mozilla :: #content

7 Aug 2017
09:42heycamannevk: can't load whatwg.org, known?
09:44annevkheycam|away: yeah, ta
09:44annevkheycam|away: top people are looking into it (not really unfortunately, waiting for DreamHost)
09:46heycamcool
16:56smaugmstange: I feel stupid. Could you explain https://bugzilla.mozilla.org/show_bug.cgi?id=1386495#c8 a bit
16:56firebotBug 1386495 NEW, nobody@mozilla.org Include event timeStamp with DOMEvent marker
16:56smaugI'm having hard to time to understand the patch
17:08smaugmstange: nm
17:31qDotbz: What's your take on the 57 addons stuff? I'm not feeling like that dev-platform thread has a good yes/no answer, mostly a "mostly no but also still yes until we can flip the pref"? Trying to decide whether I should block reviews until that pref is set.
17:46bzqDot: I think the intent is that we should be breaking things
17:47bzqdot: I personally feel like doing that until we have disabled those add-ons is not very nice
17:47bzqdot: What it means in terms of reviews... I dunno
17:47bzqdot: I'd probably be ok with landing things that are just not known to not break addons
17:47bzqdot: And less ok with landing things that are known to break them....
17:47bzqdot: But maybe I'm overthinking this
17:49qDotbz: Well we're both on the same page at least. Some sort of message to provide actual closure on this would be nice, versus "well we're gonna flip a pref then it will be obvious".
17:49qDotSince it's mostly been "we will shut off addons" not "we have shut off addons."
17:57mccr8also of note, there are patches up for removing jetpack: 1350646 Or some bug chunks of it at least.
18:05qDotbaku|away: ping
18:11smaugisn't baku|away still on vacation
18:22qDotsmaug: Oh. Yeah. Thanks.
18:52gandalfsmaug: so the try results for the tpaint change look ok. Do you think it's ok for us to land this change without understanding why timeToFirstNonBlankPaint is 20-40ms earlier than MozAfterPaint?
18:53smauggandalf: someone should look at the code again and think about why there could be that difference
18:53gandalfsmaug: any nominees? :)
18:53smauggandalf: you ! :)
18:53smauggandalf: I'm in middle of reviewing, but I could take a look
18:54gandalfI don't understand paint/layout/rendering and even DOM code enough to make any informative statement :(
18:55gandalfthank you! I'll put a NI on you for that. I don't think it's worth spending too much time, but I think it would be good to understand if we should land this patch (a'ka, does it measure better what we're trying to measure) or not
18:56* smaug kicks MozReview when patch removes files
18:56smaugdownloading the raw patch again
19:29gandalfsmaug: thanks! Does it seem reasonable that composition on a trivial HTML document takes 11% of the whole window opening?
19:30gandalfbecause that's the win from using fnbp over mozafterpaint
19:32smaugit is not 11%
19:33smauggandalf: composite happens on a different thread or process
19:33gandalfwell, my patch affects tpaint by that much - https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=1ff17e717191&newProject=try&newRevision=547f1c7f332653d5b63b803acfc854d5f5a7fa6b&framework=1&showOnlyImportant=0
19:33smaugso there is ping-pong to that thread-process and while that is happening the main process/thread is doing other stuff
19:36smauggandalf: perhaps we should have some other timestamp, which is from gpu thread/process
19:36smaugtelling when something has been updated to screen
19:37smaugor maybe we have something like that? graphics folks would know
19:37gandalfsmaug: I posted in the bug a snippet from the spec proposal for the paint timing API. I believe we should get firstNonBlankPaint to align with that, and measure it for our tpaint/ts_paint/tp6 benchmarks. But I don't know what it means in terms of where this should be marked in Gecko
19:38smaugI have no idea what the spec proposal says
19:38gandalfafter or before reading my comment?
19:38smaug(I guess it is from web perf WG, so I assume it is highly inaccurate or vague spec proposal ;) )
19:39smauggandalf: I mean, I haven't read that proposal, I think
19:39smaugah, you linked to the spec
19:40smauggandalf: and yes, rather vague to me. what does "first rendered" mean?
19:40smaugaha, The browser has performed a "paint" or "render" when it has converted the render tree to pixels on the screen.
19:40gandalfyeah
19:40smaugok, to MozAfterPaint is closer to right, but that is reporting too late timestamps I think
19:41gandalfso, we can move timeToFirstNonBlankPaint to better align with first-contentful-paint ?
19:41smauger, then it refers to HTML spec...
19:42smaugand that all is vague
19:42smaugsince it happens possibly async
19:42smaugin practice it is async
19:49gandalfsmaug: notice the images in the https://github.com/WICG/paint-timing doc
19:49gandalfthe background color change is first-paint, only the first DOM elements painted are first-contentful-paint
19:49gandalfI believe we want to measure the first-contentful-paint
19:50smaugwhatever that means
19:50smaugbut sure
19:51smaugwe do get a timestamp for composite
19:53smaugaha, and that is passed to the event at least in this case http://searchfox.org/mozilla-central/rev/b52285fffc13f36eca6b47de735d4e4403b3859e/view/nsView.cpp#1089
19:55smauggandalf: are we using event.timeStamp or event.paintTimeStamp ?
19:55smaugevent.paintTimeStamp should be probably right
19:55gandalfwithout my patch we're using performance.now() in the callback to MozAfterPaint
19:56smaugoh, that is wrong
19:56gandalfwith my patch we're using performance.timing.timeToNonBlankPaint
19:56smaugin cases paintTimeStamp is actually updated properly, it is probably the right thing
19:56gandalfso, what's the relation between performance.timing.timeToNonBlankPaint and event.paintTimeStamp? :)
19:57smaugtimeToNonBlankPaint seems to be when we do "paint"
19:57smaugand "paintTimeStamp" is more like end-of-composite, if I read the code right
19:58smaugso paintTimeStamp is closer to "The browser has performed a "paint" or "render" when it has converted the render tree to pixels on the screen."
19:58smaugpaintTimeStamp should have happened before performance.now() in the event listener
19:59gandalfso, does it sound like we should switch timeToNonBlankPaint to register at paintTimeStamp?
20:00gandalfinstead of http://searchfox.org/mozilla-central/source/dom/base/nsDOMNavigationTiming.cpp#259 ?
20:01gandalfumm, sorry, or is it http://searchfox.org/mozilla-central/source/layout/base/nsPresContext.cpp#3045 ?
20:05smaugmaybe
20:05smaugdepends on what we want to measure
20:07gandalfwe want to measure what the spec proposal is calling first-contentful-paint
20:07gandalfso, the first paint that contains DOM
20:08gandalfcurrently we measure it here: http://searchfox.org/mozilla-central/source/layout/painting/nsDisplayList.cpp#1251
20:08gandalfand from what I understand, you say we should take the timestamp from "event.paintTimeStamp"?
20:08gandalfinstead of this
20:09gandalfis that correct?
20:18smauggandalf: for our measurements for tpaint and such we should use event.paintTimeStamp yes
20:18gandalfwhat event is the prop on?
20:18gandalfMozAfterPaint?
20:19smaugor change timeToNonBlankPaint to be updated when first contentful composition has been updated
20:19smauggandalf: yes
20:19gandalfok
20:19gandalfthanks!
20:22gandalfmstange: you were the original author of the timeToNonBlankPaint (bug 1307242) - does :smaug's suggestion sound good to you as well?
20:22firebothttps://bugzil.la/1307242 FIXED, mstange@themasta.com Implement Time to non-blank Paint (TFP) for Telemetry/Profiler
20:25mstangegandalf: I'll reply tomorrow (it's a holiday in Canada today)
20:25gandalfmstange: sure! I'll file bugs in the meantime :)
20:25gandalfmstange: have a happy holiday!
20:25mstangesounds good
20:25mstangethanks
21:21billmmstange: ping
21:21gandalfbillm: he's on holiday today
21:21billmoh, ok
21:21billmgandalf: thanks
21:23gandalfsmaug: I filed bug 1388159 which switches tpaint to use event.paintTimeStamp, and we'll switch it to timeToNonBlank once bug 1388157 is resolved
21:23firebothttps://bugzil.la/1388159 ASSIGNED, gandalf Improve the measurement in tpaint
21:23gandalfthanks for help!
21:23firebothttps://bugzil.la/1388157 NEW Time to non-blank Paint should be aligned with the proposed "first-contentful-paint"
21:26smauggandalf: worth to test paintTimeStamp. I think MozAfterPaint can be fired also with paintTimeStamp 0
21:26gandalfoh, really
21:26gandalfok, thanks!
21:26smauggandalf: I'm just reading the code, not testing anything ;)
21:27gandalfsure, that's fully complementary to what I'm doing ;)
22:04mrbkapsmaug: dumb question: if I get linknode.href and want to turn it into an nsIURI, do I need to pass the linknode's document's charset to the URI constructor?
22:05gandalfsmaug: I'm getting evt.paintTimeStamp === undefined. Is it possible that paintTimeStamp is chrome-only as per http://searchfox.org/mozilla-central/source/dom/webidl/NotifyPaintEvent.webidl#36 ?
22:05smaugyes
22:05smaugMozAfterPaint itself should be chromeonly, I hope
22:06gandalfyou can turn on mozafterpaint to work in content
22:06gandalfthat's what we use in tests
22:06smaugmove the relevant code to chrome?
22:07gandalf"dom.send_after_paint_to_content"
22:07gandalfhmmm, how? I'm opening a child window and I need to measure it's mozafterpaint
22:07smaugor wrap the event so that you can access chromeonly properties
22:08gandalfI believe both, the window that opens new windows, and the child windows are content
22:08smaugSpecialPowers.wrap
22:08smaugassuming you have specialpowers there
22:09smauggandalf: you could run chrome JS as a frame script
8 Aug 2017
No messages
   
Last message: 13 days and 22 hours ago