mozilla :: #servo

8 Oct 2017
03:46KiChjangnox, ajeffrey: is linjs actually happening?
03:47ajeffreyKiChjang: there'll be an 0.1 release, hopefully at the end of the month.
03:47ajeffreyThere's a lot more work before we can work out how to use it in servo.
03:48ajeffreyWe have a lot of code in script!
04:13mbrubeckOMG why are these static? https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/updater/archivereader.cpp#43-44
04:14mbrubeckC++ code always has new and fascinating horrors
05:23KiChjangmbrubeck, #yolo
06:29cynicaldevilIf anyone&#39;s online right now, what&#39;s the replacement for Root<T> we&#39;re using now?
06:30KiChjangwe still have Root<T> don&#39;t we?
06:30KiChjangwe just don&#39;t have JS<T>
06:32cynicaldevilI don&#39;t think we do, I&#39;m getting a &#39;Root not found in scope&#39; error
06:32cynicaldevilhmm, the compiler suggests it could be DOmRoot
06:32KiChjangyeah, look in root.rs
06:33KiChjangit should be DomRoot<T> nowadays i think
06:33KiChjang#18663
06:33crowbotPR #18663: Make DomRoot<T> a type alias of a more general Root<T> type - https://github.com/servo/servo/pull/18663
06:34cynicaldevilKiChjang: thanks
06:35KiChjangIIRC this is what allows us to have different kinds of rooted objects, like Root<Vec<T>>
06:36KiChjangand most importantly Root<Promise<T>>
07:53noxcrowbot: Root<T> -> DomRoot<T>.
07:53noxKiChjang: Root<Promise> is still impossible. It will be Root<Box<Promise>> or something like that.
07:54KiChjangbetter than Rc<Promise>
07:54KiChjangwait, is Promise unsized?
07:56noxKiChjang: See documentation on StableTraceObject.
07:58KiChjangi.e. promises can move?
07:58noxNo they can&#39;t.
07:58noxThat&#39;s why you need the box.
07:58noxBut that&#39;s orthogonal to Sized.
07:59KiChjangokay, i&#39;m confused to why a Box<Promise> makes it ok
07:59noxKiChjang: Because a Box<T> can move.
07:59noxAs in, you can move the Box<T> but the address of the T inside will not move.
08:00KiChjangso are promises supposed to be moved?
08:00noxRoots are supposed to be able to move.
08:00KiChjangmovable*
08:02KiChjangthis is still not clear - is a promise designed in a way that it&#39;s supposed to have no stable addresses?
08:02noxKiChjang: Yes.
08:03noxA promise is just a plain old JS object.
08:03noxThe only reason Dom<T> is rootable easily is because it&#39;s a pointer to a T, and these T in our setup are all boxed.
08:03KiChjangok, so i can see that a Promise in rustland is just a JSReflector
08:03noxYes.
08:05KiChjanglemme see, so all Dom<T> are boxed
08:05KiChjangit uses NonZero internally
08:05noxIrrelevant.
08:06noxDom<T> is not boxed. Dom<T> contains an address to something that is boxed.
08:06KiChjangah, that makes more sense
08:07KiChjangbasically the NonZero is a pointer to a boxed object
08:07noxYes, that&#39;s what I&#39;ve been saying.
08:17KiChjangcynicaldevil, Root<T> -> DomRoot<T> btw
08:18cynicaldevilKiChjang: You already told me :P
08:19KiChjanggot confirmation from nox, so it must be true now
08:19cynicaldevilah
08:20* KiChjang is looking at the implementation of JS promises in rust
08:20KiChjangit&#39;s quite magical
08:20KiChjangthe do_nothing_func is what trips me the most
08:22KiChjangi get the feeling that interacting with SM is quite complicated most of the time - you need to always do something extra in order to &quot;fool&quot; it into not GC&#39;ing your values
08:22* cynicaldevil keeps forgetting that promises is a feature offered by web engines, and not built into the JS VMs
08:22KiChjangno, they should be
08:23KiChjangit&#39;s only at a later stage where browser engines embraced promises
08:24cynicaldevilreally? I thought anything to do with the event loop was outside of the VM
08:25* KiChjang tries to look for the ecmascript standard
08:25KiChjanghttps://www.ecma-international.org/ecma-262/8.0/index.html#sec-promise-constructor
08:27noxcynicaldevil: It&#39;s defined in ES.
08:27KiChjangnote that this is version 8.0, it also existed in 6.0 as evidenced here https://www.ecma-international.org/ecma-262/6.0/index.html#sec-promise-constructor
08:27cynicaldevilKiChjang: When was it introduced?
08:27KiChjang6.0, i can&#39;t find references to it in 5.1
08:30KiChjangoh, hmm, interesting... the spec only mentions an executor, but not an event loop to drive the promise to completion
08:31KiChjangwait, is that the reason why we have that do_nothing_func?
08:35KiChjangyeah, i was correct, it needs an executor, and in our implementation, it&#39;s just a blank function because all resolve/reject operations are done in rustland, not JS
08:48noxWhat?
08:49noxscript_runtime::enqueue_job
11:36emilionox: r? ^ (only last two commits)
12:20noxI think triage is done.
14:39emilioSimonSapin: if you&#39;re around, do you want to review #18782?
14:39crowbotPR #18782: style: Move OrderedMap to its own file, and add a with_capacity method. - https://github.com/servo/servo/pull/18782
14:39emilioSimonSapin: err, #18783
14:39crowbotPR #18783: style: Introduce CustomPropertiesBuilder. - https://github.com/servo/servo/pull/18783
14:40SimonSapinemilio: is this changing more than moving things around?
14:42emilioSimonSapin: if it&#39;s the OrderMap thing, the changes were mentioned in the commit message, but #18779 landed in the meantime and my idea is that that was going to be reverted, so I don&#39;t want to make that harder...
14:42crowbotPR #18779: DiagnosticHashMap - https://github.com/servo/servo/pull/18779
14:43emilioSimonSapin: if it&#39;s #18783, yes, it&#39;s just moving stuff around, no behavior changes yet
14:43crowbotPR #18783: style: Introduce CustomPropertiesBuilder. - https://github.com/servo/servo/pull/18783
14:43SimonSapinemilio: the former is closed, so I assume youre only asking for review of the latter
14:43emilioSimonSapin: yes, that&#39;s right
14:44emilioSimonSapin: the latter is just a slightly easier API, which allows me to encapsulate stuff, like whether we&#39;ve seen any reference, and if so avoid the walk over the map to remove cycles and such stuff, but that&#39;s still not implemented
14:44emilioSimonSapin: this is all because of bug 1405411
14:44firebothttps://bugzil.la/1405411 ASSIGNED, cam@mcc.id.au stylo: Loading YouTube&#39;s material design Subscriptions panel causes excessive CPU usage
14:44SimonSapinok
14:44emilioSimonSapin: (if you&#39;re curious about why the current worry about it)
14:49SimonSapinyeah, I saw that
14:50SimonSapinr+
14:53emilioSimonSapin: thanks!
15:14cynicaldevilnox: ping
15:23pumanera2ciao
15:23pumanera2!list
15:23rustbothttp://cglab.ca/~abeinges/blah/too-many-lists/book/
16:16noxcynicaldevil: Pong
16:17cynicaldevilnox: I don&#39;t understand how this condition can be true: https://github.com/servo/servo/blob/master/components/script/dom/servoparser/mod.rs#L243
16:18noxcynicaldevil: Calling write, from a script that was written by write, after another pending parsing block has already been written in the same write call.
16:19noxOr not even from a written script actually.
16:21noxcynicaldevil: <script>document.write(&#39;<script src=&quot;pending-parsing-blocking-script.js&quot;><&#39;, &#39;/script>&#39;); document.write(&#39;condition is true here&#39;);</script>
16:21noxIIRC
16:22noxcynicaldevil: Cf. step 24, bullet 3 in https://html.spec.whatwg.org/multipage/scripting.html#prepare-a-script
16:23cynicaldevilnox: got it
16:23noxcynicaldevil: That was the bit that required the whole mess that it is.
18:13KiChjangcrowbot, is ParentNode-querySelector-All.html intermittent?
18:13crowbotNo intermittent issues filed with &#39;parentnode-queryselector-all.html&#39; in the title
19:03KiChjangnox: r? #18262
19:03crowbotPR #18262: Implement value sanitization on HTMLInputElement - https://github.com/servo/servo/pull/18262
22:10* njn prepares to land a big renaming change (s/nsIAtom/nsAtom)
22:30njnthis PR ^^ is the Servo-side part of bug 1400460, which has r=froydnj. Can someone approve it for me?
22:30firebothttps://bugzil.la/1400460 ASSIGNED, n.nethercote@gmail.com Rename nsIAtom as nsAtom
22:30njngw: ^^ ?
22:31njnIt only does this:
22:31njn 12 files changed, 12752 insertions(+), 12752 deletions(-)
22:31njn:)
22:32hironjn: I did. :)
22:32njnhiro: thank you
22:51hiroxidorn: After the structs.rs merge, I get some errors on mach test-stylo. The errors: https://pastebin.mozilla.org/9069543
22:51njnhiro: can you r+ https://reviewboard.mozilla.org/r/187568/diff/1#index_header as well, so that I don&#39;t have to r+ my own patch in order to autoland it? :)
22:52xidornhiro: that doesn&#39;t seem to related to structs.rs merge
22:52hironjn: Done.
22:52njnhiro: thank you
22:52hiroxidorn: hmm, so what is the problem?
22:53hiroxidorn: I thought initially I used an old revision autoland, but it&#39;s the latest.
22:53hiroxidorn: I did pick the bindings.zip from https://treeherder.mozilla.org/#/jobs?repo=try&revision=cf1e51a50a6dd58db83f3b1c9005bf4484d6e0e6
22:54hiroxidorn: build-geckolib works fine, but test-stylo doesn&#39;t.
22:54xidornhiro: maybe someone changed the signature of one side, but forgot to update the other side
22:54xidornbuild-geckolib doesn&#39;t check anything
22:54hiroxidorn: what does the other side mean?
22:55xidornhiro: binding functions starting with Servo_ are declared in glue.rs, but we duplicate the declarations in ServoBIndingList.h for C++ to use, right?
22:55hiroxidorn: yep.
22:55xidornhiro: there is no build time check that they have the same function signature
22:55hirooh? glue.rs?
22:56xidornthat is maintained manually, so there could be inconsistency introduced without anyone noticing it
22:56xidornand it only causes error when someone tries to update bindings
22:57xidornI mean, tries to update the in-tree bindings in Servo side, because we don&#39;t run binding test until that
22:58gwnjn: sorry - missed your ping. looks like you have what you needed r+&#39;ed though?
22:58hiroxidorn: OK, thank I will find the commit.
22:59njngw: yes, thanks
22:59xidornhiro: I suspect that may be related to ServoBindings.toml
23:00xidornor maybe the addition of those functions are the problem...
23:00hiroxidorn: thanks, checking ServoBindins.toml could find the culprit.
23:00xidornhiro: I suspect those functions are new
23:05xidornhiro: I think it could be bug 1404897
23:05firebothttps://bugzil.la/1404897 FIXED, emilio@crisal.io stylo: Use stylo for Element::Matches.
23:05emiliohiro: xidorn: those look like my thing, yeh
23:05emilio*yeah
23:05hiroxidorn: yes, I am finding the PR for that bug.
23:06xidornhiro: I think we just need to update ServoBindings.toml for the new type mapping
23:06xidornsomehow
23:06emiliohiro: xidorn: I think I forgot some macro boilerplate in the Gecko side
23:06hiroxidorn, emilio: the commits for that bug were backed out, so..
23:06emiliohiro: pretty sure they landed again
23:07hirook.
23:07emiliohiro: (that is, the servo side PR wasn&#39;t backed out, only the Gecko side, and it landed later)
23:08hiroemilio: ah I see.
23:08xidornand, that&#39;s why I think we should fix bug 1405633
23:08firebothttps://bugzil.la/1405633 NEW, nobody@mozilla.org Run other stylo tests in rusttests tasks
23:08emilioxidorn: fair enough. Though I also think a lot of our macro / bindgen stuff is super confusing and error-prone
23:08emilioxidorn: (and unnecessary boilerplate in most cases)
23:09xidornemilio: if, it is possible, it would also be great to auto generate ServoBindingList.h
23:09xidornemilio: I guess it&#39;s possible... but maybe not trivial
23:09emilioxidorn: that&#39;s something like a cyclic dependency problem :/
23:09xidornemilio: would there?
23:10xidornemilio: you just read glue.rs and filter out the Servo_ declarations and generate their C++ signature?
23:10emilioxidorn: you need the header generate the bindings. You need the rust bindings for glue.rs to compile
23:10xidornemilio: yeah, but why do we need to have glue.rs compiled?
23:10emilioxidorn: oh, I guess if you do it in a hacky python way it would work...
23:10xidornemilio: something like that, yeah...
23:11xidornemilio: that may be better since it would raise any error at build time
23:11emilioxidorn: I was thinking of knowing the actual type, since foo::Bar and bar::Bar could be different, but yeah, a python hack would work I guess?
23:11xidornemilio: maybe
23:11emilioxidorn: I still think that the FooOwned stuff is not great
23:11emilioxidorn: and in general adding bindings to a new type is stupidly complicated
23:12emilioxidorn: and that the benefit we get from that is not that awesome either
23:12emilioxidorn: (from the FooOwned / FooBorrowed stuff, I mean)
23:12emilioxidorn: but that&#39;s another story I guess
23:13* emilio really really needs to get some sleep
23:13xidornemilio: just go to sleep :)
23:14xidornemilio: and yeah, I agree that it&#39;s too complicated for just adding bindings to new type
23:15njnhiro: is this an intermittent failure or otherwise not my fault? http://build.servo.org/builders/arm64/builds/9514
23:16njnxidorn: do you know what manifest_changed.sh checks?
23:16hirohiro: I have no idea, infra error?
23:16xidornnjn: I think you can assume it&#39;s an intermittent and just retry
23:16njnxidorn: @bors-servo retry ?
23:16xidornnjn: it seems to timeout rather than fail for any real reason
23:16xidornnjn: yep
23:16njnok, thanks
23:17xidorncrowbot: is manifest_changed.sh intermittent?
23:17crowbot#17450 - manifest_changed.sh intermittently returns 1 but reports no actual changes (https://github.com/servo/servo/issues/17450)
23:17xidornnjn: yours ^
23:18njnxidorn: ok
23:18njnmeanwhile I got bumped in the queue
23:18* njn sighs
23:20xidornI guess we can make homu pause for a short while before starting next task in queue if the current one fails
23:20xidornbecause there is a good chance that it is just an intermittent, and if we retry we can reuse majority of tasks from before
23:20mbrubeckYeah, that&#39;d be cool
23:20xidornwhich would save lots of machine time and wall clock time
23:21mbrubeckThough it&#39;ll require people to be very quick with their retries, if we don&#39;t want to add too much delay to the queue...
23:21mbrubeckSomewhere around 23 minutes might be workable
23:21xidornmbrubeck: yeah, I guess something between 1-5 min all works
23:22xidornI usually respond as soon as I see the email
23:22xidornoh maybe 1min is too short
23:23* xidorn is filing issue to homu
23:25gwit would probably be reasonable to scale the delay time based on the priority? e.g. p > 100 = 10 min wait, p > 50 = 5 min wait or something like that?
23:26mbrubeckThat seems like a good match for our typical work flow
23:26mbrubeckAnd an explicit &quot;r-&quot; should abort the delay and start the next PR immediately
23:29xidornmbrubeck: gw: ^
23:40jwilmDoes anyone know offhand how servo is computing ppem for fonts in Linux? Reading DPI/DPR?
23:44mbrubeckjwilm: Like, how do we detect the screen resolution?
23:48jwilmmbrubeck: yes
23:48jwilmmbrubeck: should have been more clear. I&#39;ve been down a rabbit hole of fontconfig and freetype today ;)
23:49mbrubeckjwilm: We don&#39;t have any auto-detection, as far as I know. On my high-DPI laptop, I have to pass &quot;--device-pixel-ratio 2&quot; to get things to render at a reasonable physical size.
23:50jwilmIs this scaling always done by ratio instead of the display&#39;s DPI?
23:50jwilmer, generally speaking
9 Oct 2017
No messages
   
Last message: 8 days and 13 hours ago