mozilla :: #servo

10 Aug 2017
00:00SimonSapinxidorn: also its all green when I open it in Nightly with Stylo
00:00xidornSimonSapin: that doesn't matter. any .xml file doesn't use stylo regardless of pref
00:00SimonSapinoh
00:00SimonSapinwhy?
00:00jryansManishearth: AFAIK no one has run the layout tests on windows (recently)
00:00xidornbecause it hasn't been enabled
00:00emilioManishearth: well, the alignment is the same... right? I mean, `[u64; 2]` has an alignment of `8` and size of `16`
00:00xidornSimonSapin: enabling that is tracked in bug 1370508
00:00firebothttps://bugzil.la/1370508 NEW, xidorn+moz@upsuper.org stylo: generic XML documents do not use stylo
00:00emilioManishearth: so it's what clang expects
00:01xidornSimonSapin: and that reftest failure is a new failure uncovered by that
00:01Manishearthemilio: no, if I change the [u32;3] to [u64'2] the test fails again
00:01SimonSapinxidorn: that should all be in the bug
00:02Manishearthemilio: wait no
00:02Manishearthhm
00:02xidornSimonSapin: I planned to mark it fails-if for now with a bug number
00:02Manishearthyeah the alignment is the same
00:02Manishearthhm
00:02Manishearthok i don't know what's going on here
00:02xidornSimonSapin: so I filed the bug
00:02xidornbecause the fix isn't immediately clear to me
00:03SimonSapinxidorn: I mean 1388911 itself should explain what you explained here. That stylo is not enabled etc
00:03xidornSimonSapin: oh, sorry about that
00:03SimonSapinor link to 1370508
00:04xidornSimonSapin: I'll add a comment
00:04canaltinovaemilio: I'm trying some things but still I'm unable to get StyleSet here :( How about adding PerDocumentStyleData to Context as we previously talked? I know you didn't like it but I don't see any other choice if we can't get the styleset here.
00:05emiliocanaltinova: so the question is why we have a PresContext without a PresShell at that point, right?
00:05canaltinovaemilio: yeah
00:08emiliocanaltinova: can you give me the backtrace again? It's a little unclear how are we trying to style anything before AttachShell happens
00:08emiliocanaltinova: also, does this happen on any specific test, or is the kind of "the browser can't boot" problem?
00:09mismithServo seems to be stalling on loading anything over HTTPS since sometime yesterday, is this a known issue?
00:10mismithWith RUST_LOG=debug I can see the HTTP request being sent, but no sign of a response.
00:10bradwerthemilio: urgh, my prepping Bug 1371395 gecko-side code for landing triggered another review request. These are tests you've reviewed before, unchanged. Sorry about the thrash
00:10firebothttps://bugzil.la/1371395 NEW, bwerth@mozilla.com Stylo: media emulation has no effect
00:10canaltinovaemilio: this is crashing thread: pastebin.mozilla.org/9029292 this is main thread: https://pastebin.mozilla.org/9029293 we need to have a font-variant-alternates property to go in GetFontFeatureValuesLookup method
00:11canaltinovaemilio something like this https://pastebin.mozilla.org/9029302
00:11emiliocanaltinova: oh, so the crash is during shutdown?
00:12canaltinovaemilio: oh, yeah. It looks like
00:13emiliocanaltinova: so, I'd probably just null-check it and return an empty one or something
00:13emiliocanaltinova: but that being said, it seems you're calling into that prescontext function from the parallel traversal?
00:13Manishearthjryans: yeah I'm not sure . I think you should show your errors to emilio
00:14emiliojryans: Manishearth: I'm going to sleep really soon-ish
00:14emilioManishearth: jryans: But if you ni? me and attach the bindings and a few failures I can try to take a look
00:14canaltinovaemilio: shouldn't I?
00:14jryansemilio: okay, i'll do that. thanks!
00:15emiliocanaltinova: that method is not thread-safe at all... What would happen if two threads call into that at the same time? My idea was to generate mFontFeatureValuesLookup once each time we rebuild the style set
00:16emiliocanaltinova: that would also prevent this problem, as bizarre as it is, without requiring a null-check
00:16emiliobradwerth: no worries, will stamp them real quick
00:17canaltinovaemilio: hm. I think that works too. I'll try that. Thanks!
00:17emiliocanaltinova: that will also fix the "font-feature-values" rules don't get updated, btw
00:18canaltinovaemilio: yeah, it seems so
00:43Manishearthbirtles: boris btw, the font SMIL test at anim-css-font-1.svg doesn't work and I'm not sure why -- want to have a look?
00:45birtlesManishearth: it should work as of bug 1379921
00:45firebothttps://bugzil.la/1379921 FIXED, dakatsuka@mozilla.com stylo: make font-variant-alternates animatable
00:45birtlesdid something regress it?
00:47Manishearthbirtles: it works, thanks. for some reason my script didn't catch that fix
00:47Manishearthsorry
00:50birtlesno problem
01:20localhorselarsberg: so CEF is an api that is implemented for chromium and servo?
01:21njnbholley: how does https://pastebin.mozilla.org/9029306 look?
01:21njnbholley: the new stuff is lines 28--43
01:21njnmeasuring the style structs
01:21njnI'm not measuring additional CVs yet, I started with this instead
01:28bholleynjn: that looks great!
01:28bholleynjn: I'd call it "style structs" rather than "styles"
01:28njnok
01:28njnbholley: one thing I'm unclear about
01:29njnyou want CVs in two buckets: those accessible from DOM elements, and those not
01:29njnright?
01:29bholleynjn: hm
01:29bholleynjn: I suppose that distinction could be enlightening
01:30bholleynjn: but it's not crucial
01:30njnbholley: I thought tat's what https://bugzilla.mozilla.org/show_bug.cgi?id=1387956#c0 was all about
01:30firebotBug 1387956 NEW, n.nethercote@gmail.com stylo: Need to measure ComputedValues together during memory reporting
01:30njnbholley: but if you're happy with all CVs in a single bucket, that's easier :)
01:30bholleynjn: hm yeah - I guess I was thinking that it would be useful, and the implementation strategy I was proposing gets that for free
01:30bholleynjn: unless I'm missing something?
01:31njnbholley: we could just not measure CVs off DOM elements
01:31njn(we would still measure ElementData and ElementStyles, though)
01:31bholleynjn: that would miss some stuff
01:31bholleynjn: there are some CVs accessible from DOM elements but not frames
01:32njnah, ok, I hadn't realized that
01:32bholleynjn: display:contents and display:none, for example
01:32njnso there's potentially 3 buckets: DOM-only CVs, frametree-only, both
01:32bholleynjn: right. And I would focus on the DOM, and group (1) and (3) together
01:33bholleynjn: basically, I was assuming we'd just measure the count at the moment between the DOM and frame traversals, and use that to divide the bucket
01:33njnbholley: ok, so that way we have two CVs buckets. Should the style structs hanging off those CVs also be split into two groups?
01:34bholleynjn: hmmm
01:34njnah crap
01:34bholleynjn: probably not necessary I think
01:34njnif not all CVs are accessible from the frame tree, then they're not all accessible on the C++ side
01:34njnsorry
01:34Manishearthstandups: investigate bug 1388904 (text-align-last in ruby) a bit
01:34standupsOk, submitted #49372 for https://www.standu.ps/user/Manishearth/
01:34firebothttps://bugzil.la/1388904 NEW, nobody@mozilla.org stylo: text-align-last doesn&#39;t work with <ruby> inside <p>
01:34njnnot all style structs are accessible on the C++ side
01:34Manishearthstandups: fix min font size ruby (bug 1388941)
01:34standupsOk, submitted #49373 for https://www.standu.ps/user/Manishearth/
01:34firebothttps://bugzil.la/1388941 ASSIGNED, manishearth@gmail.com stylo: min font size doesn&#39;t work within ruby
01:35bholleynjn: remember what I was saying yesterday about bouncing across FFI and back?
01:35njnsort of
01:35bholleynjn: for the DOM traversal, we _do_ need an FFI call to access the CVs out of mServoData, but it can be very lightweight
01:36bholleynjn: and it can just call back into C++ with the CV to be measured
01:36bholleynjn: you can either have an API to just extract, and then do the measurement, or you can have the API call you back
01:40Manishearthstandups: fix HASR for logical props (bug 1388943)
01:40standupsOk, submitted #49374 for https://www.standu.ps/user/Manishearth/
01:40firebothttps://bugzil.la/1388943 ASSIGNED, manishearth@gmail.com stylo: logical border properties don&#39;t force non-native appearance
01:48njnbholley: so the C++ DOM traversal code would do something like this?
01:48njn for each element:
01:48njn ServoStyleContext* context = Servo_GetComputedValues(element->mServoData);
01:48njn if (haven&#39;t measured context) { context->AddSizeOfIncludingThis(...) }
01:48bholleynjn: yes, except there are multiple CV within mServoData, because of pseudos
01:48njnyeah, I just realized that :(
01:49njngod, nothing about this is easy
01:49bholleynjn: that was why I was imagining the callback approach. But you could also just have explicit getters for each of the 4 pseudos
01:49Manishearthstandups: classify reftests https://gist.github.com/Manishearth/086118c940ff86a6cfc573f53c508279
01:49standupsOk, submitted #49375 for https://www.standu.ps/user/Manishearth/
01:51njnbholley: will EAGER_PSEUDO_COUNT always be 4?
01:51bholleynjn: it may change, but not often
01:51bholleynjn: I just landed some code to make gecko more aware of which pseudos are eager this morning
01:52bholleynjn: it sorta seems like a callback would be nice, if we could figure out how to bindgen C++/Rust closures together somehow :-)
01:52njnI guess we could have a getter that gets one pseudo, which takes an index
01:52bholleynjn: yeah
01:52njnthe callback approach would require exposing nsWindowSizes
01:52njnwhich is big and complicatd
01:53bholleynjn: but yeah, indexed getter works
01:53bholleynjn: it&#39;ll be null most of the time anyway
01:54njnthat doesn&#39;t make it any easier to write the code :P
01:57njnbholley: all this will render https://github.com/servo/servo/pull/18030 obsolete -- I just added Rust-side code to measure pseudos
01:57crowbotPR #18030: Measure ElementStyles::pseudos. - https://github.com/servo/servo/pull/18030
01:58bholleynjn: well, you still may want to measure the heap-allocated EagerPseudoStyles along with ElementData
01:58bholleynjn: just save the particular CVs for alter
01:58bholley*later
01:59njnoh, just the array
02:59larsberglocalhorse: Right! It&#39;s designed for chromium (Chromium Embedding Framework), but we have implemented it for Servo.
02:59localhorselarsberg: in webrender?
02:59larsberglocalhorse: it&#39;s a servo thing
02:59localhorseas a drop in replacement?
03:00localhorselarsberg: but which crate should i use if i want to embed it in my rust application?
03:00larsberglocalhorse: I have to go to sleep, but it won&#39;t work right now via CEF due to linker issues (https://github.com/servo/servo/issues/17663)
03:00crowbotIssue #17663: embedding: shared library no longer linkable (again) - https://github.com/servo/servo/issues/17663
03:01larsberglocalhorse: but if you&#39;re just coming from rust, just use the &#39;servo&#39; crate - https://github.com/servo/servo/blob/master/components/servo/lib.rs
03:02localhorselarsberg: is it possible to create a fullscreen window that renders a given html website from disk?
03:02localhorsewithout browser controls
03:02localhorsejust as a viewer
03:02localhorsecontrolled by the code only
03:28njnbholley: I&#39;m trying to work out the signature of this Servo_Element_GetComputedValues() function
03:28njnesp. what the return type is
03:29njnoh, Servo_StyleSet_GetBaseComputedValuesForElement() might be a useful guide
03:31njnor this:
03:32njnpub extern &quot;C&quot; fn Servo_ResolveStyleAllowStale(element: RawGeckoElementBorrowed)
03:32njn -> ServoStyleContextStrong
03:38njnexcept it should actually be Option<ServoStyleContextStrong> returned, hmm
04:16bzWhoa
04:16bzstylo-failures.md is no more?
04:16bzIs that because we&#39;re passing all enabled mochitests?
04:52bholleybz: yep
04:54bzbholley: sweet
04:54bzbholley: btw, I have patches up for the anon box bits
04:54bholleybz: great!
04:57bzbholley: https://public.etherpad-mozilla.org/p/anon-box-stylo has status
04:59bzbholley: I think that&#39;s all the stylo stuff on my plate for the moment; I&#39;m going to focus on wrapping up reviews and whatnot in the two days I have left before vacation
04:59bzbholley: But if there&#39;s something else that is urgent, please let me know
04:59* bz has this cellmap thing on his mind too
05:02paulajeffrey: how many tabs can we have per top level browsing context? Only one?
05:04bholleybz: cellmap?
05:05bholleybz: (potentially those remaining first-line failures, but maybe they can wait til you&#39;re back)
05:07bzbholley: I looked at the first-line failures 20 mins ago
05:08bzhttps://bugzilla.mozilla.org/show_bug.cgi?id=1388877#c1
05:08firebotBug 1388877 NEW, nobody@mozilla.org stylo: Some failing first-line reftests
05:10bzbholley: I mean, I&#39;ll do the RecoverLetterFrames thing for sure
05:11bzbholley: cellmap, see https://bugzilla.mozilla.org/show_bug.cgi?id=1384602#c20 and https://bugzilla.mozilla.org/show_bug.cgi?id=1384602#c23
05:11firebotBug 1384602 ASSIGNED, bzbarsky@mit.edu stylo: Squashed boxes in YouTrack Agile Boards view
05:11bzbholley: On the one hand, messing with the cellmap is scary
05:11bzbholley: on the other hand, it&#39;s possible that the workarounds in bug 1384602 won&#39;t apply to other situations where stylo ends up with different insertion patterns than Gecko, so might be good to fix anyway
05:24ajeffreypaul: well &quot;tab&quot; isn&#39;t really a concept in the spec, but each tlbc only has one active document, so I&#39;d say yes, tabs are 1:1 with tlbcs.
05:25paulajeffrey: alright - so it&#39;s not possible to have multiple top level active documents loading
05:25paulajeffrey: so I&#39;m confused about the counter thing you mentioned
05:29ajeffreypaul: if the events are
05:30ajeffrey> Load A
05:30ajeffrey> Load B
05:31ajeffrey< LoadStart A
05:31ajeffrey< LoadComplete A
05:31ajeffrey< LoadStart B
05:31paulWe start spinning on LoadStart A, not on Load A.
05:31ajeffrey< LoadComplete B
05:32ajeffreypaul: really?
05:32paulyes.
05:33ajeffreySo the embedder is expected to wait for the LoadStart before spinning?
05:33paulyes.
05:33ajeffreyEven so, in this example the spinner will stop even though there&#39;s an outstanding load.
05:34paulIt will spin, stop spinning, spin, stop spinning
05:34paulso it&#39;s fine.
05:35ajeffreypaul: okay, if we can live with transitory states like this, sure.
05:35paulIt&#39;s not a compromise, it&#39;s really the behavior we want.
05:36ajeffreyOkay, I&#39;m not the UI person :)
05:36paulFirst, Load X is not sure to load X, maybe servo will refuse to load (if AllowNavigation returns false for example). Also, we get LoadStarts if the user navigates, so the embedder want to rely on LoadStart.
05:37paulSo yes, LoadStart is really the event the embedder wants to rely on.
05:40paulif at the constellation level, indeed, A finishes loading before B starts loading, the spinner should reflect that. I have some scenarios in mind where Load A should not trigger the spinner right away. Like, if the constellation need to ask the embedder to allow or not the navigation, and if the embedder uses a modal dialog to notify the user about
05:40paulnavigating away, then the spinner should start only when the user clicks yes, not when the navigation is requested
05:40paulajeffrey: so if you&#39;re fine with this current behavior, I will make the change I mentioned earlier in the comments, and re-ask for a review
05:41ajeffreypaul: sounds good!
05:41paulajeffrey: thank you :)
06:23gwwow, who knew cache invalidation was hard
06:25njngw: I prefer to call it &quot;cache reconsequencing&quot;
06:33Manishearthxidorn: good catch
06:34xidornManishearth: :)
06:36xidornah...
07:20xidorncrowbot: is attachment-local-clipping-image intermittent?
07:20crowbot#17885 - Intermittent failure in /css-backgrounds-3_dev/html4/attachment-local-clipping-image-1.htm, /css-backgrounds-3_dev/html4/attachment-local-clipping-image-4.htm, /css-backgrounds-3_dev/html4/attachment-local-positioning-5.htm, /css-backgrounds-3_dev/html4/attachment-local-clipping-image-3.htm (https://github.com/servo/servo/issues/17885)
07:55Manishearthmorning emilio
08:01emilioManishearth: hey
08:03borisnox: are you planning to move all the tranform related code into another place? There are so many in animated_properties.mako.rs
08:04noxboris: Yes.
08:04noxboris: Remove some, too.
08:04noxPretty sure there are many refactorings doable there.
08:04borisnox: cool!
08:19noxdaisuke: Seems similar to what I did, but with more code in mako things and less code reuse.
08:19noxUnfortunate that this was a P3 blocking a P2 without stating it...
08:33daisukenox: thank you very much, I had commented on the bugzilla.
08:36noxdaisuke: AFAIK for the serialisation Servo is correct and Gecko isn&#39;t.
08:37cynicaldevilnox: was tree_builder/actions.rs moved into mod.rs?
08:37noxcynicaldevil: There were changes yeah. We removed some traits. Not sure whether that&#39;s the move you talk about.
08:38noxdaisuke: I didn&#39;t implement most compute_distance methods given that was the other ticket that&#39;s blocked by yours,
08:39noxnical: I hate the fact that you made Euclid use Copy.
08:39noxnical: Now any trait impl that uses Euclid types need to have + Copy and it&#39;s a huge pain.
08:40cynicaldevilnox: hmm that&#39;s the one, just checked the PR
08:44noxemilio: Around?
08:44nicalnox: can you show me an example? The point of using copy was that we could not get around having at least Clone, and euclid code was just filled with `.clone()` aal over the place where realistically it is only gonna be used with copy types
08:44noxWe could have get around having just Clone.
08:45noxnical: I&#39;m implementing Animatable for BorderCornerRadius,
08:45noxwhich uses Size2D,
08:45noxthe impl for Size2D needs Copy to call Size2D::new,
08:45noxso the impl for BorderCornerRadius needs it,
08:45noxso the impl for BorderRadius needs it,
08:45emilionox: yes
08:45nicalBorderCornerRadius is generic over the scalar type ?
08:45noxso the impl for insetRect needs it,
08:45noxso the impl for BasicShape needs it,
08:46noxso the impl for ShapeSource needs it.
08:46noxnical: Yes, most CSS types are generics to abstract over specified vs computed vs animated values.
08:46noxThat being said, we could also stop using Euclid in CSS types.
08:46nicalnox: wouldn&#39;t it make sense to use the Unit for this ?
08:47noxHow is that related in any way?
08:47noxnical: Size2D<LengthOrPercentage>, what unit?
08:47nicalthe Unit is there to have some strong tying if you want to differentiate between say computed and animated values
08:47nicalah ok that sounds different
08:48emilioheycam: ping? What&#39;s the AutoClearStaleData stuff?
08:48noxnical: We are differentiating them not to do fancy things with the type system, but because they are different, that&#39;s all.
08:48noxemilio: I see we implement HVP for ShapeSource,
08:48noxjust returning false,
08:48noxemilio: but can&#39;t basic shapes contain viewport percentages?
08:48nicalnox: would it help if Thing::new didn&#39;t require copy but the other methods did ?
08:48emilionox: presumably, yeah
08:49noxnical: Meh, at that point I think we should just stop using euclid in these types. :)
08:49heycamemilio: that&#39;s the thing that clears out ElementData from the tree of a document that&#39;s gone into and come back out of the bfcache
08:49noxnical: Cause other issues like not being able to derive ToCss.
08:50nicalnox: and having to specify Clone for all of these types is different/better than copy ?
08:50noxnical: Ah I see what you mean.
08:50noxnical: I&#39;m 100% of the opinion that needless bounds shouldn&#39;t be there.
08:50emilioheycam: I see... So I&#39;m failing the !e->HasServoData() assertions with my patches for bug 1381071, but that&#39;s because presumably now we _do_ have a bfcache entry or a shell, and we leave them there
08:50firebothttps://bugzil.la/1381071 NEW, emilio+bugs@crisal.io stylo: Huge hang on Facebook, spending most of my time in style recalculations
08:50nox::new definitely doesn&#39;t need any of these bounds indeed.
08:50emilioheycam: so I think the assertion is now valid
08:50emilio*invalid
08:51emilioheycam: Going to relax it a bit...
08:52* heycam looks
08:53heycamemilio: can you explain what your patches do?
08:53heycamstyle un-pres-shelled documents?
08:53noxdaisuke: What&#39;s box_type?
08:54nicalnox: only Foo::new() is needlessly requiring Copy. The rest needs at least Clone and since we aren&#39;t going to use euclid for anything that isn&#39;t Copy, might as well use Copy (it made euclid&#39;s code a gazillion times easier to read/maintain)
08:55noxnical: Why &quot;might as well use Copy&quot;?
08:55emilioheycam: nope, just store on the element data the styles from getComputedStyle when called on display: none subtrees
08:55noxI am not arguing against using Copy anymore in euclid, but if a method doesn&#39;t need a bound, I don&#39;t see why it should have the bound,
08:56noxthat being said, I&#39;ll file an issue to remove use of Size2D and whatnot with LoP types.
08:56emilioheycam: if the rule inclusion is All, and we have a presShell/bfCache entry
08:56daisukenox: GeometryBox for clip-path and ShapeBox for shape-outside
08:57nicalnox: for methods that needed at least clone, because &quot;clone()&quot; wast stomped several times per line all over the code. for Foo::new() it was just laziness and I can change it today if it gets in the way
08:57noxnical: I know, I remember how it was with Clone. :)
08:57noxdaisuke: Ah I see.
08:57noxOf course I need to rebase...
08:57noxdaisuke: Pushing.
08:58heycamemilio: can you limit the assertion to just whatever you call into for getComputedStyle?
08:58noxWtf is UrlShapeSource?
08:58emilioheycam: sure, that&#39;s my plan, relaxing it a bit, not removing it
08:58heycamemilio: sounds good :-)
08:58noxOh, just Url with a bad name.
09:10heycamif I have an iterator of iterators. is there any easy way to fold/chain them up? I tried passing in iter::empty() as the first fold argument, but the types don&#39;t seem to agree
09:12Manishearthheycam: .flat_map?
09:13Manishearthplaybot: [[1,2,3],[4,5,6],7,8,9].iter().flat_map(|x| x.iter()).collect::<Vec<_>>()
09:13Manishearthplaybot: [[1,2,3],[4,5,6],[7,8,9]].iter().flat_map(|x| x.iter()).collect::<Vec<_>>()
09:13Manishearth^^
09:14noxdaisuke: What&#39;s your GH handle?
09:14daisukenox: dadaa
09:14Manishearthnox: you can animate shapes omg
09:15noxemilio: Doing a geckotry, but my m-c clone is 2 weeks old. -.-
09:15noxManishearth: What about it?
09:15noxManishearth: Only similar shapes, it&#39;s not Adobe Flash.
09:15Manishearthnox: no it&#39;s just something i didn&#39;t know you could do
09:15Manishearthtfw shape tweeeeen
09:16heycamManishearth: thanks!
09:17noxgit fetch autoland after two weeks takes ages.
09:20noxOH RIGHT
09:20noxWhat&#39;s the new command for mach try?
09:23Manishearthnox: mach try
09:23noxNo.
09:23noxThe params.
09:23Manishearth./mach try -b d -p linux64 -u all -t none
09:23Manishearth-b do if you want opt too
09:24noxCan&#39;t run just stylo stuff anymore or whatever?
09:24noxWhat&#39;s slower between -b o and -b d?
09:24Manishearthnox: you could probably pin it down in the -u param
09:24Manishearthidk how
09:25Manishearthnox: *shrug* i usually do -b d since it has asserts
09:25noxMy code never asserts out.
09:25* nox hides.
09:43emilioheycam: what about adding a PerOrigin<T>
09:43emilioheycam: to avoid the iter_origins, etc duplication?
09:44emilioheycam: maybe worth to do in a followup
09:47heycamemilio: yeah, just slightly couldn&#39;t be bothered ;)
09:48noxemilio: I feel like you really really want to review my PR today.
09:48nox:P
10:11emilionox: which one?
10:12noxemilio: https://github.com/servo/servo/pull/18035
10:12crowbotPR #18035: Animate basic shapes - https://github.com/servo/servo/pull/18035
10:12emilionox: oh, sure, will get to it
10:13emilionox: actually, it&#39;s rather straight-forward
10:13noxYes it is.
10:15noxemilio: derive(Animatable) is the plan at some point, yes.
10:21noxdaisuke: Still around here?
10:21noxdaisuke: My patch doesn&#39;t cause unexpected failures.
10:21noxOh wait it&#39;s not done trying yet.
10:30noxdaisuke: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9e42a940030271a5c4e5dc4a55a117f330ab9c26&selectedJob=122231759 Same failures for you?
10:35noxWait, shape-outside isn&#39;t animated in Gecko or what?
10:42noxemilio: Around?
10:43emilionox: yup
10:43noxI have absolutely no clue what the failures I just linked are.
10:43* emilio looks
10:43noxWould help if daisuke could reply if he got the same ones, but he doesn&#39;t seem around
10:45emilionox: seems like it&#39;s testing that transitions are _not_ supported for a given property
10:45emilionox: which looks silly
10:45noxYeah I don&#39;t really understand that part.
10:46noxemilio: Am also confused by the sudden -moz-outline-radius-bottomleft failures.
10:47noxWait this is even worse than I thought...
10:47noxhttps://github.com/servo/servo/commit/b37f270c650e193e5cc2a6dbde346c0ffc877111 Why is there a computed type in this module?! Cc boris
10:48noxOh I see.
10:48emilionox: that seems for all radius, so yeah, seems like your PR is breaking that?
10:49noxShouldn&#39;t.
10:49emilionox: apart from that, it&#39;d be a matter of either write or skip the test for shape-outside
10:49noxIt makes no sense.
10:50noxemilio: I changed the BorderCornerRadius impl of Animatable, could you check in the PR that they are the same for the add_weighted method and that I&#39;m not insane?
10:52emilionox: yeah, those look the same to me, but maybe it&#39;s the compute_distance stuff?
10:52noxIf that&#39;s it I&#39;m angry.
10:52noxThese aren&#39;t used on the Web, and that test sounds like it&#39;s about the Web.
10:53noxBut anyway, I need to implement ToAnimatedValue for all the types that transitively use BorderCornerRadius,
10:53noxfun times...
10:53emilionox: *shrug*. The compute distance seems different to me
10:53noxYes it is.
10:54noxI guess I&#39;ll clean the animated/* modules once I&#39;m done.
10:55noxemilio: Wait nope, my impl is the same. :(
10:56travis-ciServo failed to build with Rust nightly: https://travis-ci.org/servo/servo-with-rust-nightly/builds/263033485 CC nox, SimonSapin
10:56noxAh no ok, finally understood the problem.
10:57noxIf you squint hard enough, you notice that add_weighted reuses the impl of Animatable for Size2D<T>,
10:57noxbut compute_distance does the computation itself, and cannot reuse the impl because the one for Size2D doesn&#39;t even define compute_distance.
10:57noxGreat.
11:03noxemilio: I&#39;ll just land my patch as is without the proper unclamped intermediate values in basic shapes.
11:03noxemilio: To fix that properly, I first need to clean up all the NonNegative types and whatnot. :(
11:06noxboris: Ping.
11:10xidornemilio: I just did a profiling on test_value_computation.html (with profiler of visual studio), and saw something weird. the profiler says we spend 29% of cpu time running Stylist::rebuild, but as far as I can see, in majority of time, that test should just set property and get computed style. it doesn&#39;t seem to make sense that it would trigger
11:10xidornStylist:rebuild a lot. is that expected? known issue?
11:16xidornI guess I may know why...
11:22xidornyep... I know why... this test can be significantly optimized...
11:23xidornbut we still spend 30% of time doing Stylist::rebuild :/
12:02emilioxidorn: the patches heycam is working on should help a lot with that
12:03xidornemilio: bug number?
12:04emilioxidorn: https://bugzilla.mozilla.org/show_bug.cgi?id=1382925
12:04firebotBug 1382925 ASSIGNED, cam@mcc.id.au stylo: Keep the UA parts of the stylist across changes to document sheets
12:04xidornemilio: doesn&#39;t sound like it will help a lot...
12:04emilioxidorn: oh, I thought it was adding stylesheets and what not
12:05emilioxidorn: but it seems it&#39;s adding <iframe>s
12:05emilioxidorn: yeah, then it won&#39;t help that much
12:05xidornemilio: it currently is, but I have a patch in bug 1389041 which would make it no longer do that
12:05firebothttps://bugzil.la/1389041 NEW, xidorn+moz@upsuper.org A simple optimization to test_value_computation.html
12:05emilioxidorn: with that, do we still spend 30% of the time in stylist::rebuild?
12:06xidornemilio: yes
12:06emilioxidorn: wat
12:06emilioxidorn: where are we rebuilding?
12:06xidornI want to know as well :/
12:06xidornsounds like it is not a known issue, then
12:07xidornemilio: oh, are you asking where?
12:07emilioxidorn: yeah, what is causing us to rebuild?
12:08xidornemilio: flush pending notifications...
12:08xidornwell, that&#39;s a reasonable source, because we are changing properties and querying computed style... but that doesn&#39;t sound like a good reason to rebuild stylist anyway
12:08emilioxidorn: seems like that testcase does setProperty on the stylesheets
12:08emilioxidorn: on the CSS rules
12:08emilioxidorn: not on the style attribute
12:09emilioxidorn: so we do definitely rebuild stylesheets
12:09emilioxidorn: (and gecko does too)
12:09xidornhmmm
12:09emilioxidorn: and heycam&#39;s patches _will_ help a lot there
12:09xidornemilio: because we would not rebuild the ua part?
12:09emilioxidorn: right, we&#39;ll only rebuild the document part, which are three or four rules
12:09xidornthat sounds promising then
12:09xidornthat can also explain why gecko is much faster on this
12:10emilioxidorn: right, it&#39;s pretty much bug 1382927
12:10firebothttps://bugzil.la/1382927 NEW, nobody@mozilla.org stylo: very slow handling of modification of stylesheets
12:11emilioxidorn: we could optimize it even more I guess, by avoiding the rebuild and just doing a recascade of the whole document
12:11xidornemilio: yeah, that&#39;s what I was thinking
12:11xidornmaybe it isn&#39;t worth, though
12:11emilioxidorn: but that needs to wait for bug 1384802 at least, I think, otherwise we may miss empty rules
12:11firebothttps://bugzil.la/1384802 NEW, kuoe0@mozilla.com Stylo: Empty rules should still be returned by inIDOMUtils.getCSSStyleRules
12:12xidornemilio: so that we build rule node for empty rules and then we can just recascade on top of that
12:12xidornsounds good
12:13emilioxidorn: right... hmm... I guess that&#39;s not enough
12:13xidornanyway, I guess it&#39;s not a worthy optimization for real websites
12:13emilioxidorn: if you insert a non-important rule in an important declaration block we&#39;d need to create another rule node
12:13xidornalthough it may be valuable for many of our tests
12:13emilioxidorn: yeah, that&#39;s fair
12:34noxemilio: https://treeherder.mozilla.org/#/jobs?repo=try&revision=22905db42de44a6fdfca4d543876d349b5bbe127&selectedJob=122260524
12:34noxemilio: Should I just not animate shape-outside for now?
12:34noxSeems like it causes some issues or that Gecko itself says it shouldn&#39;t be animated.
12:39noxdaisuke: An opinion? ^
13:42jdmcrowbot: what&#39;s new?
13:42crowbotHuh! Chromium totally crushed Servo in HTTP throughput!
13:42jdmtrue
13:43gsneddersOH NO
13:44noxThat&#39;s because we don&#39;t use tokio yet.
13:45fwiwum... is that just random words or based on something?
13:47noxfwiw: Based on random words.
13:48jdmcrowbot: what does the web need?
13:48crowbotjdm: servo could take the lead on the browser testing and tools working group (https://www.w3.org/Consortium/activities#Browser_Testing_and_Tools_Working_Group)
13:48jdmthat is... more accurate than usual.
13:48noxfwiw: https://github.com/servo/crowbot/blob/master/server.js#L355
13:49noxcrowbot: What&#39;s new?
13:49crowbotPalemoon beat Servo in experimental crypto implementation :<
13:50martyixcrowbot: What&#39;s new?
13:50crowboti heard that SeaMonkey outperformed Servo&#39;s FTP support by an underwhelming amount
13:50martyixcrowbot: What&#39;s new?
13:50crowbotChrome outdid Servo&#39;s process separation usage...
13:58fwiwlol
13:59fwiwnox: honestly, can&#39;t tell from truth
14:46cynicaldevilnox, jdm: Does the solution I sent via mail today look good?
14:46jdmcrowbot: yeah, I think it&#39;s worth giving a shot
14:47cynicaldevilcrowbot will definitely try it then
14:47jdmer, cynicaldevil
14:47jdmha
14:48* froydnj wonders what jdm just approved crowbot to do
14:49avadacatavrafroydnj: start the bot uprising
15:03borisnox: yeah, feel free to clean NonNegative up if you have a better solution :)
15:04borisnox: I remember shape-out is non-animatable in Gecko now, so it would be fine to make it non-animatable for now.
15:25noxboris: clip-path is animatable though, right?
15:25borisnox: yes. and I think we have some tests for clip-path in Gecko
15:25noxOk.
15:46bojancrowbot: What&#39;s new?
15:46crowbotServo absolutely destroyed Palemoon&#39;s parallel IndexedDB benchmarks :)
15:46est31crowbot: explain netwerk
15:46crowbotest31 http://i.imgur.com/hEsWQDN.png is old, but it&#39;s better than nothing
15:49bojancrowbot: easy bug
15:49crowbotbojan: Try working on issue #17181 - Do not add spaces when serializing operators in attribute selectors - https://github.com/servo/servo/issues/17181
15:52bojanWell I would gladly work on this.. if I knew Rust :D
15:53fwiwcrowbot: explain rust
15:53crowbotfwiw here you go - http://i.imgur.com/T28NoeY.png
15:56bojanloving the crowbot source: graphs.randomGraph(...) :D
16:03pcwaltonstandups: Finished the FreeType-based font loader yesterday. Now working on the Web service component of the demo, which will convert glyphs in fonts into meshes.
16:03standupsOk, submitted #49400 for https://www.standu.ps/user/pcwalton/
16:08avadacatavrahttps://irccloud.mozilla.com/pastebin/v3AwH0H8/
16:08avadacatavrawhere is that trait defined?
16:08jdmavadacatavra: /Users/ddh/mozilla/servo/servo-0/target/debug/build/script-3c766708dacc98e6/out/Bindings/DocumentBinding.rs
16:08jdmavadacatavra: it&#39;s generated from the webidl file
16:09avadacatavrajdm: can i change it?
16:09jdmavadacatavra: nope
16:09avadacatavradamn
16:09jdmavadacatavra: our DOM objects need interior mutability
16:10avadacatavrajdm: i&#39;m trying to set navigation start for the interactive metrics
16:10avadacatavrahttps://github.com/servo/servo/blob/master/components/script/dom/document.rs#L3668
16:10avadacatavrathere&#39;s also https://github.com/servo/servo/blob/master/components/script/dom/document.rs#L3744
16:10avadacatavrathat might be useful
16:11avadacatavracurrently document has an InteractiveMetrics object
16:11avadacatavrabut maybe i need to change that if i can&#39;t make it mutable there
16:12jdmavadacatavra: use Cell/DOMRefCell like all the other things that require mutability
16:12* avadacatavra doh
16:12avadacatavrajdm: can i use the excuse that i&#39;ve been in a cave of C++ lately?
16:13jdmheh
16:13* avadacatavra takes an afternoon break
16:14jdmc &quot;const is constant until const_cast&quot; ++
16:14avadacatavrajdm: not sure if const?
16:41Jackneillhttps://github.com/servo/servo/blob/master/components/script/dom/document.rs#L3744 is this file created before rustfmt and still using that coding style?
16:41jdmJackneill: yes
18:04bholleyemilio: do you think we need to tick animations in reconstruct traversals?
18:05bzbholley: Do you have a sec?
18:05bholleybz: kinda
18:05bholleybz: what&#39;s up?
18:05bzbholley: Do you know where the rust/jemalloc integration is?
18:05bholleybz: what about it?
18:05bzbholley: That is, when servo bits of stylo need to allocate memory, how do they do it?
18:05bholleybz: oh, like where in the code?
18:05bzbholley: As in &quot;do they call moz_xmalloc?&quot;
18:06bholleybz: I believe froydnj set that up
18:06bzok
18:06bholleybz: that is my understanding of how it works, at least - froydnj can confirm
18:06bzI&#39;ll check with him
18:06bzSo we don&#39;t get a rust panic, we get a whatever malloc abort?
18:06bzanyway, I&#39;ll check
18:08mbrubeckI believe Rust&#39;s standard allocators also abort rather than panic on failure...
18:08bzyes, but the question is whether instrumenting moz_xmalloc will catch allocations rust does
18:08bzfor a patch that&#39;s trying to measure things about allocations
18:08bzor whether it&#39;s calling malloc and then panicking
18:25jntrnrSimonSapin: just saw they fixed https://github.com/rust-lang/rust/issues/39280
18:25crowbotIssue #39280: trans/LLVM: Don&#39;t keep all LLVM modules in memory at the same time - https://github.com/rust-lang/rust/issues/39280
18:26SimonSapinjntrnr: yeah, Ive left that email notification unread as a reminder to do some more testing of incremental compilation at some point
18:27SimonSapinjntrnr: except we cant upgrade rust-in-servo at the moment, were blocked on https://github.com/rtbo/rust-xcb/pull/41
18:27crowbotPR #41: Use more lifetimes - https://github.com/rtbo/rust-xcb/pull/41
18:28bzheycam is on Taipei time, right?
18:28SimonSapinbz: yes
18:28bzThanks
18:28SimonSapinjntrnr: (the compiler started rejecting code that it maybe should have rejected all along, but we depend on some such code thats maintained by someone who hasnt responded yet)
18:29jntrnrSimonSapin: got it. hrm, blocking on a dep that might be abandoned sounds bad :-/
18:30SimonSapinjntrnr: well, we dont know if