mozilla :: #content

19 May 2017
00:12* bkelly actually goes to try this "exercise" thing...
00:41qDotbkelly: overrated.
01:41bkellyqDot: it wasn't too bad
01:46qDotI should actually do some of that soon.
02:15bkellyqDot: I think one of the hardest part about being a remotee is that its so easy to get no activity... when you go into the office you have to walk around at least a little bit
02:17qDotbkelly: I did walk way more when I went to the office, yeah. But I'm close to 2 nice climbing gyms, used to have a daily stretching/core workout routine, etc. Just need to whip myself back into it. -.-
02:18bkellyqDot: you could climb to the office
02:19bkellyI guess you'd have to live in the sewers, then
02:20qDotWell my desk in SF is on 7.
02:20bkellygood point
02:51bzqdot: did you get your test_interfaces questions answered?
02:52qDotbz: If you'd take a look, it'd be appreciated.
02:52qDotThe logic for exposure there is a little confusing.
02:52bzThis is the nonReleaseWindows: false thing?
02:52qDotYeah.
02:53bzk
02:54bzSo this test is saying that if !isWindows
02:55* bz thinks it through
02:58bzSo I'm not happy with the naming
02:58bzThe actual effect here seems to be to enable on all channels on windows
02:58bzand on non-release channels on non-windows
02:58qDotThe naming plus the negations happening everywhere made it super confusing. I think they were trying to follow the nonReleaseAndroid naming? And yeah, that's what I thought they were going for but I couldn't confirm it.
02:59bzYes, but the "nonReleaseAndroid" thing means "android and not release channel"
02:59bzWhereas their "nonReleaseWindows" thing means "windows or not release channel"
02:59bzSo modeling naming of one on the other is pretty bad
03:01qDot'k. I'll have them rename it.
03:01bzIt's more like releaseNonWindows
03:02qDotYeah, combination of naming and following the logic chains just had me staring at the monitor for a bit heh.
03:02bzbut also...
03:02bzNow that aurora is gone, we only have two channels, right?
03:02bznightly and release
03:03bzI guess the point is still that this stuff can't express "enabled on X or Y"
03:03bzok
03:03bzSo they do need a new primitive
03:04qDot(brb)
03:34qDotOk back.
03:51bkellybz: this try build makes us much better at that servo tiles demo https://treeherder.mozilla.org/#/jobs?repo=try&revision=b4f72aac2d49ef43c10c5fc5d8c7dc6e82d252a8
03:51bkellyhttps://mozdevs.github.io/servo-experiments/experiments/tiles/
03:52bkellystill not as good as safari
03:52bkellychrome seems jankier to me
04:12bzbkelly: nice
04:12bzbkelly: the other thing with Safari is it manages it with pretty low CPU usage
09:47annevkhsinyi: shawnjohnjr: I can probably do the work for https://bugzilla.mozilla.org/show_bug.cgi?id=1364598
09:47firebotBug 1364598 NEW, nobody@mozilla.org Access-Control-Expose-Headers does not handle stray spaces as Chrome does, breaking FCC pagination.
09:48annevkhsinyi: shawnjohnjr: just wanted to confirm that's an okay approach
09:49hsinyiannevk: cool, that sounds good to me!
12:16bzannevk: ping
12:16annevkbz: hey
12:20* annevk is reminded of http://www.nohello.com/
12:34bzannevk: Sorry
12:34bzannevk: Something came up; back now
12:34bzannevk: do you want to talk sync about https://github.com/whatwg/html/issues/2667 ?
12:35annevkbz: I think I got all the information now, I guess I should file bugs
12:35bzOK, great.
12:35annevkbz: I was wondering about a similar thing for <a> and friends
12:35bz<a> explicitly does the navigation off a task
12:35annevkbz: yeah, but why?
12:36annevkbz: activation behavior itself is already a task
12:36bzhttps://html.spec.whatwg.org/multipage/semantics.html#following-hyperlinks-2it is?
12:36bzer...
12:36bzir is?
12:36bzbah
12:36annevkyes
12:36annevkwhich is invoked from <a>&#39;s &quot;click handler&quot;
12:37annevkit seems weirdly inconsistent
12:37bzno...
12:37bzActivation behavior happens sync from https://dom.spec.whatwg.org/#concept-event-dispatch
12:37bzIn step 18.1, rigt?
12:37bzer, right?
12:38annevkbz: sure, you could trigger it sync with click()
12:38bzexactly
12:38annevkbz: just like you can with setting src here
12:38annevkbz: but in the general case a user click would be a task
12:38bzSure
12:38annevkI dunno, seemed weird that we&#39;d sometimes invoke navigate from a task and sometimes not
12:39bzWell
12:40annevkMaybe this <iframe> case is just a bug, even window.location uses a task
12:40bzThis is totally observable, right?
12:40bzin terms of how things behave?
12:40bz(and not just with beforeunload)
12:40annevkSure, either way it&#39;s observable
12:40bzI&#39;d really like to understand exactly what other UAs are doing here
12:41bzBecause making changes to this sort of thing sets off my web compat alarm bells
12:42annevkbz: if I do setTimeout(..., 0) just before .src and they&#39;d queue a task the timeout would run first, right?
12:42annevkalthough maybe that&#39;s not guaranteed if they&#39;re not the same task source...
12:42annevkgood times
12:43annevksince nobody implements task source maybe I should just assume that though
12:43bzright
12:43bzsafari does
12:43annevkoh cool
12:44bzIn that in safari the ordering of settimeout or message events with other stuff may not be &quot;in the order things are queued&quot;
12:44bzI have never investigated the details
12:47annevkbz: FWIW, next week somewhere my break until SF starts (Mark&#39;s email about not taking breaks came way late) so unlikely to make much more progress, but I do plan to continue to figure out all things navigation / document / browsing context/ etc. after that
12:49annevkIt&#39;s not going as fast as I&#39;d like, only modest refactoring after 3 weeks, but there&#39;s progress which I didn&#39;t really see happening before
12:51bzannevk: Awesome, thank you
13:21bkellybz: well, safari does a lot more graphics acceleration than we do on mac... we went back to software rendering there AFAIK
13:39farresmaug: ping
13:41smaugMobilefarre: pong
13:41smaugMobile(running tests on the other machine)
13:41farresmaugMobile: ah, you&#39;re on your phone. I thought I&#39;d ask you if you could fast-track the review of bug 1364858, but I can ask someone else
13:41firebothttps://bugzil.la/1364858 NEW, afarre@mozilla.com The mThrottleTrackingTimeoutsTimer timer is still present when calling ~TimeoutManager
13:42farrebkelly perhaps?
13:42smaugMobilefarre: I can review in couple of minutes
13:42farregreat! it&#39;s a three-liner, and it blocks landing the bg tracking throttling
13:42smaugMobileJust can&#39;t disturb performance test run
13:43smaugfarre: which is now done
13:43farreexcellent
13:46bkellyI love how touching setTImeout() brings all the race conditions to the yard
13:46farrebkelly: how&#39;s the single nsITimer ... wow, that timing was scary
13:47bkellyfarre: its looking really good... but because setTimeout(f, 0) runs faster some tests are failing
13:47bkellyfarre: current try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b4f72aac2d49ef43c10c5fc5d8c7dc6e82d252a8
13:47farrebkeyll: I saw you mentioned that. reducing the amount of timers we use got me not liking how I did bug 1358476
13:48firebothttps://bugzil.la/1358476 NEW, afarre@mozilla.com Add nsThread::idleDispatch(nsIRunnable, uint32_t aTimeout)
13:48farreas in, what&#39;s up for feedback atm. it&#39;s just lots of timers again
13:48smaugfarre: so inner objects were always freed
13:48smaugbut something...
13:48smaugsomething still called MaybeStartThrottleTrackingTimout
13:48bkellythe good news is that most of these tests are reproduceable locally now
13:49smaugoh, http://searchfox.org/mozilla-central/source/dom/base/nsGlobalWindow.cpp#3969 could definitely call the method
13:49farrebkelly: do you think it would be a hassle to implement nsThread::DelayedDispatch using something similar? i.e. a list of runnables and just one nsITimer
13:49farresmaug: exactly
13:50smaugfarre: is mWindow guaranteed to be non-null ?
13:50bkellyfarre: I&#39;m not sure... I would be more in favor of removing nsThread::DelayedDispatch()... its confusing to have multiple ways of doing the same thing
13:50farreyes, because nsGlobalWindow uniquely owns TimeoutManager
13:50smaugah, &
13:50smaugfarre: done
13:50farresmaug: thanks
13:55farrebkelly: do you mean similar to SetTimeout?
13:56bkellyfarre: what is the use case for nsThread::DelayedDispatch() compared to nsITimer?
13:57farrebkelly: ah, you mean like that. well, it would basically be the same, and yes you&#39;d pretty much duplicate the list of something that is callable as with the list on the timer thread.
13:58bkellyfarre: in theory nsThread::DelayedDispatch() could be implemented as a binary heap data structure and with the timing controlled by the thread&#39;s event loop... if no other runnables, wait for next soonest delayed deadline or check for deadlines after each runnable execution
13:58farrebkelly: it&#39;s just that the helpers that are in place for creating runnables and dispatching to threads are so much nicer than timers.
13:58bkellyfarre: if we had that, then we could make nsITimer check to see if its target support delayed dispatch... if it does, use that instead of nsITimer
13:58farrebkelly: yes, that was kept me awake last night
13:58bkellyinstead of TimerThread
13:59bkellyfarre: we can make nicer helpers for nsITimer
13:59bkellyfarre: froydnj|away pointed me to SimpleTimer in media code: http://searchfox.org/mozilla-central/source/dom/media/VideoUtils.h#281
13:59bkellyfarre: we could move something like to xpcom
14:00bkellySimpleTimer::Create(NewRunnableMethod([] { /* my lambda */ }));
14:48bkellysmaug: is the painting here guaranteed to complete within one event loop turn (implied by setTimeout(f, 0) here)? https://dxr.mozilla.org/mozilla-central/source/layout/reftests/bugs/703186-1.html#10
14:48bkellymy changes to speed up setTimeout() cause this reftest to fail sometimes...
14:48bkellyits unclear to me why the setTimeout(f, 0) should always fall after the painting... so I think its a bug in the test... but I&#39;m unsure
14:50smaugbkelly: looking
14:50bkellysorry for the random question... but you reviewed the test :-)
14:51smaugbkelly: trying to recall when reftest takes the screenshot
14:51smaugI thought the next paint after removing reftest-wait
14:52smaug(dxr is so painful)
14:52smaugbkelly: I guess you could try to use rAF there and see whether it passes
14:56bkellysmaug: hmm, ok... or maybe the reftest harness is using setTimeout(f, 0) somewhere and its the one running faster
14:56smaugdbaron might recall the reftest setup better
15:48sheppyHaving a little battle with Intersection Observer. Reading the spec, it seems that each time a target element&#39;s intersection ratio passes one of the entries in the observer&#39;s thresholds, the callback should be called. But it appears that we only call the callback the first time isIntersecting changes and the ratio exceeds at least one of the entries in
15:48sheppythresholds.
15:49sheppyOr it gets called when isIntersecting changes, then a second time when the first threshold is passed. Then it doesn&#39;t get called again until isIntersecting flips again. Pretty sure that&#39;s not what&#39;s supposed to happen, is it?
15:49sheppy(preparing to file a bug but I like to ask about these things here too)
15:50smaugsheppy: IntersectionObserver is really a layout thing
15:50smaugnot DOM
15:50smaugsheppy: you may want to ping tobytailor
15:51sheppysmaug: I always have a hard time finding the border between these areas :)
15:52smaugsheppy: yeah, this one is tricky.
15:52smaugbut it has been implemented by layout team
16:22dbaronbkelly: IIRC, reftest does a setTimeout(0), after calling the onload handler, before capturing the screenshot so that tests have the chance to do a setTimeout(0) in onload and still have that happen before the screenshot. Or something like that.
16:22dbarons/reftest/the reftest harness/
16:23bkellydbaron: ok... I&#39;m making setTimeout(0) faster and seeing a number of reftest failures... I might need to tweak some of these if they are racing things that are not actually in the next event loop turn
16:23bkellymy changes make setTimeout(0) effectively do NS_DispatchToCurrentThread(runnable) instead of nsITimer->Init(0)
16:24* bkelly writes bad pseudocode...
16:24dbaronbkelly: it&#39;s possible that the reftest *harness* would need to be changed to use something slower...
16:24bkellyyea, thats what I&#39;m going to look at
16:25dbaronbkelly: since what it does may well have been intended to be at least as slow as some set of things that we want to allow tests to do in onload
16:25bkellydbaron: maybe a requestAnimationFrame()?
16:25dbaronbkelly: requestAnimationFrame might have other side-effects that we don&#39;t want...
16:25bkellyor two rAFs if we want to force a paint
16:25bkellyok
16:25bkellyI&#39;ve been using setTimeout(5) in other tests with a similar problem
16:29bkellyits weird that I can&#39;t seem to run reftests locally using xvfb-run
16:29* bkelly goes to lunch while that runs...
17:06* smaug is adding quite some hacks to ensure we flush layout less when calling focus()
17:16* jdm tries to figure out how to VNC into a ubuntu box on his desk
18:23mystorUgh, why does libxul take 3 minutes to link on windows :&#39;(
18:24mystorIt takes me 5 minutes to recompile after a single cpp file change
18:30bkellymystor: yea... its a known problem, but not sure there is a solution yet... something to do with incremental linking being broken
18:30bkellyits a major pain
18:31bkellyI wonder if we could have a &quot;build to DLLs&quot; to mode for development to avoid the &quot;link the world&quot; step
18:33bkellymystor: you are using ./mach build binaries, right? that at least avoids some of the other slow windows stuff
18:53mystorbkelly: Sorry was afk for a bit
18:53mystorbkelly: Yeah, I&#39;m using mach build binaries
18:53mystorbkelly: A dll mode would probably be nice but *shrug*
19:05bkellyk
19:32smaugehsan: by any chance, would you be interested in to review bug 1366250 ? I think Enn is away.
19:32firebothttps://bugzil.la/1366250 NEW, bugs@pettay.fi Flushing in nsFocusManager::CheckIfFocusable shows up significantly in Speedometer.
19:32smaugif not, I think I&#39;ll ask, hmm, masayuki
19:32ehsansmaug, of course
19:33smaugehsan: the flushing stuff is needed especially for tests which do something like foo.focus(); testingOnlyMethodWhichSendTrustedKeyEvents();
19:34ehsansmaug, I see... there shouldn&#39;t be any scriptable APIs visible to pages which can do something like that right?
19:34ehsanso it should be a testing only scenario
19:38smaugehsan: well, I think we do have currently some issue if one changes input&#39;s type after focusing it
19:38smaugand there is key events pending
19:38smaugit might lead to unexpected results
19:39smaugsince the layout of input element isn&#39;t flushed when handling key event
19:39smaugthe patch should actually fix that
19:39ehsanyes, it will
19:39ehsanafter your patch the key event will be processed with the input having the correct frame type
19:39ehsanwhich is nice ;)
19:41* smaug reruns speedometer with the patch. afk for couple of minutes
19:43bkellywhere is the speedometer v2 test?
19:44ehsanbkelly, https://sm.duct.io/Full.html
19:45billmbholley: ping
19:46bkellywow, we have a lot of pauses on that thing
19:49bholleybillm: (about to grab dinner, but what&#39;s up?)
19:49smaugbkelly: pauses?
19:50* smaug continues running tests
19:50ehsansmaug, r+
19:50bkellyehsan: are we aware that link has some documents getting pulled over network every time?
19:50smaugehsan: thanks
19:50bkellyindex.html
19:50smaugI&#39;m surprised this wasn&#39;t filed before
19:50ehsanbkelly, say more
19:50smaugperhaps the tests have changed a bit
19:50bkellyehsan: run the test with network panel open
19:50smaugsince I don&#39;t recall seeing this issue before
19:50smaugbkelly: don&#39;t run any tests with devtools open
19:50billmbholley: just wanted to see if you could give an opinion in bug 1366072. I was hoping to start implementing JW&#39;s suggestions, but I don&#39;t want to waste time if you&#39;d like it a different way.
19:50firebothttps://bugzil.la/1366072 NEW, wmccloskey@mozilla.com Allow MozPromise to take an nsIEventTarget as well as an AbstractThread
19:51bkellysmaug: well sure... but we shouldn&#39;t have a client side test depending on network conditions
19:51* ehsan fights the network panel on a running test page
19:51smaugbkelly: I mean activating devtools tend to do bad things
19:51bkellysmaug: I only opened it because I saw the &quot;load from https://sm.duct.io&quot; status box popping up a lot
19:51bkellysorry... &quot;waiting for https://sm.duct.io&quot;
19:52bholleybillm: so nsIEventTarget doesn&#39;t work because it doesn&#39;t guarantee that stuff doesn&#39;t run simultaneously
19:52ehsanbkelly, do you realize that it&#39;s loading different iframes?
19:52ehsannot same ones?
19:52bholleybillm: nsIThreadPool is the canonical example
19:52billmbholley: but there&#39;s no reason AbstractThread shouldn&#39;t be an nsIEventTarget
19:52bkellyehsan: thats fine... but what are we testing if it repeatedly loads over the network with no http cache?
19:52* ehsan doesn&#39;t see anything suspicious going on...
19:52ehsanbkelly, a real browser :)
19:53bholleybillm: AbstractThread can be an nsIEventTarget, but APIs that use AbstractThread need that and not nsIEventTarget
19:53bkellyehsan: our network connectioN?
19:53bholleybillm: now, we can certainly add nsIAbstractThread
19:53ehsansure
19:53ehsanthat&#39;s not where the tests are bottlenecked though
19:53bkellythats bogus for a benchmark IMO
19:53billmbholley: so you&#39;re saying MozPromise shouldn&#39;t take nsIEventTarget?
19:53bholleybillm: it should not
19:53billmbholley: why?
19:53bholleybillm: because it would be wrong to pass it a thread pool
19:53bholleybillm: nsIThreadPool
19:53billmbholley: not for people who don&#39;t care :-)
19:54bholleybillm: there are lots of assumptions in MozPromise about ordering
19:54bkellyehsan: I see &quot;waiting&quot; for 300+ms for a single document... thats not going to affect test results?
19:54ehsanbkelly, afaik the benchmark doesn&#39;t measure those wait times
19:54bholleybillm: I don&#39;t think consumers can adequately make that call
19:54ehsanbkelly, I&#39;m confused what you&#39;re objecting to?
19:54bkellyehsan: how can we compare test runs if network conditions change over time
19:55* ehsan isn&#39;t sure how that matters
19:55bkellymaybe these waiting times are our main thread jank
19:55billmbholley: ok, what if nsIEventTarget exposed a method saying whether it ever dispatches runnables to different threads, and we would assert that that&#39;s false in the MozPromise code?
19:55bkellyehsan: seriously? if you have a test that depends on network... and network changes... how can you verify code changes are having an impact? this seems like testing 101 to me
19:55ehsanbkelly, the parts where the tests measured have been profiled
19:56ehsanand afaik so far nobody has seen idle waits under the profiles
19:56bholleybillm: how about we add a new interface?
19:56bholleybillm: that&#39;s the obvious thing to do
19:57bholleybillm: and that&#39;s what AbstractThread was, except I didn&#39;t make it an XPCOM interface because it was all just media code back then
19:57bholleybillm: and it mattered less
19:57billmbholley: but then everyone would have to use that interface instead
19:57ehsanbkelly, that&#39;s not evident when you run the benchmark from this UI, of course...
19:57bkellyehsan: if nothing else, it should enable http caching so that the test runs faster
19:57bholleybillm: I mean, we&#39;d have nsIThread which inherits nsIAbstractThread which inherits nsIEventTarget
19:57bholleybillm: so people using nsIThread could use it just fine
19:57bkellythe old speedometer v1 did not have this problem
19:58billmbholley: all right. people store nsIEventTarget all over the place, but I guess we can change them as needed.
19:58ehsanbkelly, when browserbench.org updates they&#39;ll probably just run under the same server side config
19:59bholleybillm: I mean, we can also just QI
19:59billmbholley: in MozPromise?
19:59bholleybillm: no, callsites
19:59bholleybillm: so, a few difficulties to sort out with this
19:59billmbholley: oh. that seems worse to me.
20:00bholleybillm: callers want to use MozPromise, callers can store the appropriate type, no?
20:00bholleybillm: it doesn&#39;t seem that hard
20:00bholleybillm: and I&#39;d rather not QI on _every_ call to MozPromise
20:00smaugehsan: tested again. Without patch 90, with the patch 115
20:00ehsansmaug, sweet!!!
20:00ehsanSHIP IT!
20:00billmbholley: I&#39;ve already conceded that point
20:00ehsansmaug, btw
20:01ehsansmaug, have you looked at the timeout issue jandem mentioned?
20:01smaugehsan: I haven&#39;t
20:01ehsansmaug, let&#39;s do it next week?
20:01smaugyup
20:01ehsansounds great
20:01bholleybillm: so, a few other difficulties to sort out: (1) how to keep the API &quot;nice&quot; (infallible, taking already_addRefed, etc), and (2) Being careful with the stuff the we currently assume about AbstractThred
20:02ehsansmaug, is there anything else in speedometer dom wise we should worry about?
20:02smaugehsan: that and I&#39;m planning to look at also input.value some more, unless makoto does it
20:02smaugI&#39;m not aware of other things
20:02ehsansmaug, there has been some traffic on https://bugzilla.mozilla.org/show_bug.cgi?id=1358025
20:02smaugehsan: but I&#39;m not sure we have profiled the very latest version too much
20:02firebotBug 1358025 NEW, m_kato@ga2.so-net.ne.jp Don&#39;t create transaction when script sets input.value
20:02bholleybillm: maybe (b) is fine as long as we implement the stuff at http://searchfox.org/mozilla-central/rev/24c443a440104dabd9447608bd41b8766e8fc2f5/xpcom/threads/AbstractThread.h#81 to return the right thing
20:02ehsansmaug, yeah...
20:02smaugah, I wasn&#39;t CCed to that bug
20:03ehsansmaug, I&#39;m planning to do more profiling over the weekend
20:03bholleybillm: ok, anything else before I head to dinner?
20:03ehsansmaug, oh, thought you were... sorry about that
20:03billmbholley: what&#39;s (b)?
20:03ehsansmaug, I think he is on top of the input.value issue, I wouldn&#39;t worry about that one
20:03smaugok
20:03bholleybillm: err, (2)
20:03smauggood
20:04billmbholley: well, I don&#39;t intend to do tail dispatching at all if you pass an nsIEventTarget
20:04billmbholley: one that&#39;s not an AbstractThread that is
20:04billmbholley: I hope MozPromise doesn&#39;t somehow rely on tail dispatching?
20:04bholleybillm: my suggestion is to replace AbstractThread with nsIAbstractThread
20:05billmbholley: well, I really was just going to make an interface that&#39;s exactly nsIEventTarget, except that it would only be implemented by people who dispatch one runnable at a time
20:06billmbholley: I think that&#39;s all MozPromise needs
20:06billmit never uses the tail dispatcher
20:06bholleybillm: But if your objection is to the proliferation of AbstractThread, then why not eliminate it and unify the stuff?
20:07billmbholley: the proliferation is almost entirely due to MozPromise. so if we can avoid using it there, then almost the entire problem goes away.
20:07billmbholley: after that point, maybe we could tackle the rest. but I don&#39;t think I have time for that now.
20:09bholleybillm: it seems like it would be pretty straightforward
20:10bholleybillm: the only code changes I can think of would be the renaming and making TaskQueue do its refcounting the XPCOM-y way instead of inline
20:10bholleybillm: and then we&#39;d avoid parameterizing promise on the dispatch type
20:10bholleybillm: which is exactly what I introduced MozPromise to avoid
20:10bholleybillm: er
20:10bholleybillm: AbstractThread
20:11billmbholley: I guess I don&#39;t understand. what would do tail dispatching?
20:11billmbholley: would that move to nsThread?
20:12bholleybillm: well, that logic is currently virtual
20:12bholleybillm: and lives in TaskQueue and XPCOMThreadWrapper
20:12bholleybillm: the XPCOMThreadWrapper bit would go into nsThread, and be conditional on being the main thread
20:12bholleybillm: and then we&#39;d get rid of the wrapper and the pointer-identity problems along with it
20:13bholleybillm: (needs to be the main thread because it relies on the stable state machinery)
20:14billmbholley: I just don&#39;t think I have time to do that. I can make an interface and have MozPromise use it. the rest of what you&#39;re saying could be follow-on work.
20:14billmbholley: I think they would fit well together
20:15bholleybillm: I&#39;m just a bit unhappy about re-adding the templated dispatch target stuff to MozPromise after I did all that work to remove it
20:15bholleybillm: can you see if jw or bkelly have time?
20:15billmbholley: if we made AbstractThread inherit from this interface, then there wouldn&#39;t be any templating
20:16bkellyI don&#39;t... and I don&#39;t see why we can&#39;t take an incremental approach
20:16bholleybillm: but that&#39;s exactly the same amount of work!
20:16bkellyadding nsIEventTarget to MozPromise would move us in the right direction and it wouldn&#39;t risk regressing a bunch of media code/tests
20:17bholleybillm: because you then need to XPCOMify TaskQueue to make the refcounts work right
20:17billmbholley: the interface would only have the nsIEventTarget methods
20:17bholleybillm: and also nsISupports, right?
20:17bholleybillm: I&#39;m assuming this interface will inherit from nsIEventTarget
20:17billmbholley: ugh, I forgot about that
20:18billmbholley: so we&#39;d just have to change the refcounting, right? that doesn&#39;t seem too hard.
20:18billmbholley: I really want to avoid touching the tail dispatching code
20:19bholleybillm: I don&#39;t think you need to, you just move it to nsThread and make it conditional on being main thread
20:20billmbholley: so it would assert in other situations?
20:20bholleybillm: right - and the &quot;is tail dispatch supported&quot; stuff would return false
20:20billmbholley: we also would have to fix up the MessageLoop variant
20:20bholleybillm: MessageLoop implements AbstractThread?
20:21billmbholley: http://searchfox.org/mozilla-central/source/ipc/glue/MessageLoopAbstractThreadWrapper.h
20:21bholleybillm: does MessageLoop implement nsIEventTarget?
20:22bholleybillm: looks like that wrapper doesn&#39;t do anything with tail dispatch
20:22billmbholley: no. I guess that was another reason JW wanted to use templates.
20:22ehsanbkelly, https://bugzilla.mozilla.org/attachment.cgi?id=8868735 is so smart
20:22ehsanI like it
20:22ehsantook me a while to understand it at first...
20:23billmbholley: can I just use templates? I&#39;m afraid I&#39;m going to get sucked into a whole project here.
20:23bholleybillm: can you at least ask jw if he&#39;s willing to do it?
20:24bholleybillm: I&#39;m not steadfastly opposed multiple dispatch targets, I just think it&#39;s less elegant and more maintenance burden and I haven&#39;t yet heard a reason why it&#39;s more than an hour or two of work
20:24billmbholley: I&#39;ll talk to him about it. I know he&#39;s doing some sort of refactoring of this code.
20:24ehsanbkelly, I actually like searching through mFiringIdStack
20:25bholleybillm: ok. If he isn&#39;t willing to do it either then we can templatize, but we do need the new interface for correctness
20:25billmbholley: he might be willing to pick it up. to do it right, though, I think we need to stop using MessageLoop so much.
20:25bholleybillm: and we might as well call it nsIAbstractThread
20:25ehsanbkelly, since I think the super nested case is the pathological case we shouldn&#39;t be optimizing for here...
20:25bholleybillm: presumably the messageloopwrapper can stay
20:25billmbholley: I&#39;d rather give it a name that fits with our existing abstractions. nsISingleThreadedEventTarget, say.
20:25bholleybillm: it would just inherit nsIAbstractThread
20:26bholleybillm: I think it does fit
20:26bholleybillm: because it&#39;s like nsIThread
20:26bholleybillm: except not guaranteed to run on the same OS thread
20:26bholleybillm: so nsIThread, nsIAbstractThread, and nsIEventTarget
20:26bholleybillm: seems reasonable to me
20:26billmbholley: but it&#39;s not necessarily a thread. it&#39;s weird that TaskQueue is a &quot;thread&quot;.
20:26billmbholley: it doesn&#39;t process events. it just dispatches them.
20:27bholleybillm: that&#39;s what the &#39;abstract&#39; means - I don&#39;t see how calling it &quot;SingleThreaded&quot; is any better :-)
20:27bholleybillm: it&#39;s a logical thread
20:27bholleybillm: from the perspective of the caller
20:27billmbholley: well, I&#39;ll ask nathan. I think it&#39;s his call.
20:27bholleybillm: ok - but at least present my reason along with yours :-)
20:27billmbholley: will do
20:28billmbholley: thanks
20:28bholleybillm: np - sorry to be difficult. Dinner for real now
20:28billmbholley: have a good time
20:32bkellyehsan: thanks!
20:57smaugbkelly: curious, have you run speedometer with your setTimeout changes?
20:57bkellysmaug: I tried earlier, but I need to replicate it locally because the network delays were annoying me
20:57bkellyand I don&#39;t trust it with network delays in the loop
20:58bkellysmaug: try build here if you want to test it on your setup: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dc573d3a4617d6479a74453f75a61dc51e501660&selectedJob=100551929
20:58smaugbkelly: I haven&#39;t seen differences in speedometer results even if loading some page has taken time
20:58smaugand I&#39;m now on rather unreliable over-the-forest-under-the-lake connection
20:59smaugbkelly: too many patches to import :)
20:59bkellysmaug: well, you can download the build
20:59bkellynot familiar with that kind of network connection
20:59smaugwell, I just pushed a patch which changes speedometer number significantly
20:59bkellycool
20:59smaug90 -> 115
20:59smaughopefully it sticks
21:00bkellysmaug: the v1 or v2 version of the test?
21:00smaugv2
21:01smaughttps://sm.duct.io/Full.html is the one I run
21:05bkellyI&#39;m cloning the webkit repo to access the test locally
21:06smaugbkelly: I think that repo might have some changes
21:06smaughmm, wasn&#39;t there some comment it has currently some bug
21:07bkellyI dunno... today was the first time I ever saw a link to the v2
21:07smaug&quot;The current version of Speedometer on webkit tip is broken and stops
21:07smaugat test 32: it tries to load
21:07smaug/resources/todomvc/architecture-examples/preact/build/index.html which
21:07smaugdoes not exist.&quot;
21:07smaughttps://bugs.webkit.org/show_bug.cgi?id=172371
21:29bkellythanks
21:31bkellywhat is responsible for drawing the dotted box around an element when its focused?
21:31bkellyis that chrome script or a native thing?
22:09billmbkelly: pretty sure it&#39;s a platform thing. something in layout.
22:11gandalfsmaug: ping
22:11smauggandalf: pong
22:11gandalfsmaug: are you going to the QF work week?
22:11smaugyes
22:13gandalfso, I know it&#39;s post 57, but I&#39;m trying to use the time to line things up so that when we&#39;ll start transitioning to l20n, the platform is as ready as it can be - is there anything I can do to help you take l20n DOM use into account when planning DOM optimizations next week?
22:13gandalfor do you think it&#39;s out of scope?
22:14smaugthinking
22:15smauggandalf: in general figuring out how to do it all in native code
22:15smaugusing fast code paths
22:15gandalfbug 1347799 has the current DOM bindings patch, and bug 1363862 has your C++ patch
22:15firebothttps://bugzil.la/1347799 NEW Create Localization DOM bindings module
22:15firebothttps://bugzil.la/1363862 NEW Add a chrome-only DOM API for localization
22:15smaugright
22:15smaugI wonder if even more could be done natively
22:15gandalfit seems to me like we&#39;re in a good place
22:16gandalfI&#39;m at the point where getting all the startup code (getting to the point where a single element in browser.xul is translated using l20n) costs <1% of ts_paint/tpaint. I didn&#39;t test scaling it to 100/200 elements, but I have your patch which should ensure we stay within 1%
22:16gandalfI believe that eventually, we&#39;ll want to move the whole DOM bindings to C++
22:16smauggandalf: I wouldn&#39;t mind profiling couple of try builds with l20n stuff enable, or just some patches
22:16gandalfbut it&#39;s easier to prototype in JS and in particular with your team busy on Quantum :)
22:17gandalfhmm, but would you like to profile the bootstrap piece (everthing until we have a single localized element) or scaling (200 elements on browser.xul) ?
22:17smauggandalf: btw, have you discussed with florian and others who have been improving startup time?
22:17gandalfcause I have the former for you, but not the latter (because I didn&#39;t want to maintain a fork of broser.xul file which is changing now)
22:18gandalfnot yet. I&#39;ve been working with :kmag and others on Gecko startup time and language negotiation, picking up langpacks during startup etc. But not on per-document localization yet.
22:18gandalfbecause it seems that we look good in talos for now
22:19smauggandalf: remind me at which point l20n kicks in
22:19smaugaround DOMContentLoaded?
22:19smaugor was it that new layout event?
22:19gandalfso, currently it kicks in at DOMContentLoaded of browser.xul
22:19smaugBeforeLayout or so
22:19gandalfbut I&#39;d like to move it to MozBeforeLayout
22:19gandalfyeah
22:20gandalfthen it does XHR (sync for now) for IO (we&#39;ll want to move it to Fetch API and async)
22:20gandalfand then all the DOM operations like querySelectorAll([&#39;data-l10n-id&#39;]);
22:20gandalfand element.textContent=msg.value;for (attr of msg.attrs)element.setAttribute(attr.id, attr.value);
22:21gandalffor each localizable element
22:21smaugcould we load the data from xul cache ?
22:22floriangandalf: rather than maintaining a fork of browser.xul, have you considered maintaining instead a script doing the changes you care about automatically? (so that updating your patch to a newer source tree is just a matter of re-running a script)
22:23gandalfflorian: yeah, I used to have a script to do that, but lost is somewhere on the way. I may rewrite it soon, since we&#39;re getting close to landing of the bits :)
22:23gandalfsmaug: I don&#39;t know!
22:23smauggandalf: the data is json
22:23smaugright?
22:23gandalfnope. The data is in l10n format called FTL (like CSS for style)
22:23smaugoh, it was its own
22:23smauggandalf: and you parse it using JS?
22:23gandalfbut we can cache the AST JSON that we parse to in a JSM singleton
22:23gandalfyeah
22:24gandalfin the future we&#39;d like to parse it using Rust
22:24gandalfwe have a Rust parser
22:24gandalfbut Rust->JS for JSON is not easy yet
22:24gandalfand as I said, so far the talos profile looks good :)
22:24gandalfsmaug: if you want to profile the bootstrap, I can give you a patch now (m-c vs. browser.xul with a single element localized using l20n)
22:24smaugsure, but we should _improve_ things, not make things slower ;)
22:25gandalfif you want to profile a more realistic localization, I&#39;ll try to prepare a browser.xul with 200 elements localized using L20n for you
22:25smaugxul cache might make sense
22:25gandalfyeah, that&#39;s true
22:25smaugin preparsed format
22:25smaugsimilar to xul itself
22:25smaugwhich is then fast to load from xul cache without parsing
22:25smaugjust reading it as bytecode
22:26smauggandalf: do you know how much time parsing l20n takes?
22:26gandalfand if locale changed, invalidate cache and translate again?
22:26smaugright
22:26gandalfit used to take less than 5ms, and it should get faster with all the work in SpiderMonkey, but I didn&#39;t profile lately
22:26smauglocale changing should happen extremely rarely
22:26smaugok
22:27gandalfas I said, we used to have 20-40ms performance loss compared to m-c, now we have 3-5ms total.
22:27gandalfcorrect
22:27smaugI still wonder what fixed the issue you had
22:27gandalfyeah, I don&#39;t know
22:27gandalfbut I&#39;ll profile it soon :)
22:27smaugI&#39;m not aware node wrapping becoming any faster
22:28gandalfso, here&#39;s the thing. Half a year ago I had two profiles - for a single element, and for 200 elements (whole menubar).
22:28smaugdo we just create the wrappers before l20n touches them these days
22:28gandalffor single element we were paying ~20ms, for 200 elements ~40ms.
22:28gandalfnow we pay ~3-5ms in the single element scenario
22:28gandalfwe still may have 20ms cost for 200 elements
22:28gandalfI just don&#39;t have a patch to test that yet
22:29gandalfso I expect that your C++ patch will come useful
22:29gandalfbut the &quot;bootstrapping&quot; of L20n got definitely much faster
22:29gandalfsmaug: to wrap up. Would you like me to prepare a patch with 200 elements to profile?
22:29gandalfwould you have time/interest in doing that during your work week?
22:30smauggandalf: I could profile some cases
22:30gandalfI&#39;ll have only one case - browser.xul with 200 elements using data-l10n-id to localize :)
22:30smaugjust tell