mozilla :: #content

9 Oct 2017
05:46casperlWhich class property store anonymous tree in dom node?
10:10smaughsivonen: ping
10:11smaugcasperlei: which anon tree you mean?
10:14casperleismaug: like <progress> have anon tree which is<div>
10:16smaugcasperlei: that case is basically controlled by nsIFrame tree. And the div is created in
10:18casperleismaug: I see <div> is maintained by frame tree I guess. Is all anon tree maintained by frame tree, not dom tree?
10:24smaugcasperlei: XBL (and Shadow DOM which is being developed) are maintained differently
10:25smaugcasperlei: curious, why you ask ?
10:26smauglike, are you perhaps fixing some bug :)
10:26casperleismaug: I am digging into firefox. And hope I can find a bug :)
11:49noxHello, is per event loop or per global?
14:03smaugcatalinb: ping
14:03catalinbsmaug: pong
14:03smaugcatalinb: do you know how we end up having range in two lists?
14:05smaugI mean, MOZ_DIAGNOSTIC_ASSERT(!isInList()); and the if (isInList()) feels wrong. The assertion is probably fine, but we should then rely on it to never fire and then the &#39;if&#39; should be needed.
14:06catalinbsmaug: so this could have been caused by bug 1399091
14:06firebotBug is not accessible
14:06smaugsure, but do you know how it is possible that range is in the list?
14:06catalinbsmaug: but I&#39;m not certain and without a test case there&#39;s not much I can do
14:06smaugwell, code inspection?
14:08catalinbsmaug: registerCommonAncestor() without a prior UnregisterCommonAncestor() can cause this
14:10catalinbsmaug: and also the test case in bug 1399091 can result in this infinite loop
14:10smaugcatalinb: ok, sounds like a fix should be something else then, right?
14:10catalinbsmaug: yes
14:12catalinbsmaug: would you be ok with landing just the MOZ_DIAGNOSTIC_ASSERT?
14:13smaugcatalinb: for now, and you&#39;ll fix the issue in a separate patch?
14:14catalinbyes, I&#39;ll keep the bug open
14:14smaugfeel free to land the assertion
14:14smaugthis is a bug we need to get fixed in FF57 too, right?
14:16smaugin which case there isn&#39;t much time to look at crash stat stacks
14:19catalinbsmaug: it&#39;s not. I&#39;ll try to go through the code and find the cause.
15:32zombieso, our code seems to suggest our service workers don&#39;t support foreign fetch
15:33bkellyzombie: foreign fetch has been removed from the spec for right now
15:33zombieand yet, we have wpt intermittents
15:34bkellyzombie: yes, those tests should be removed upstream
15:34bkellyzombie: we can disable them locally if necessary
15:34zombieemphasis on &quot;intermittents&quot; ;)
15:34bkellyzombie: they are marked expected fail... sometimes they timeout
16:03zombiebkelly: for my curiosity, is there a tldr link on why it got removed?
16:03zombie(i&#39;m guessing too much security exceptions to cover)
16:04zombiei couldn&#39;t easily find it (and don&#39;t know exactly where to look)
17:37annevkbz: sorry about all the regressions
17:37annevkbz: though even with them it seems your issue still stands :/
17:52bzannevk: yeah
17:52bzannevk: Really, my issue is &quot;implementations suck&quot;
17:52bzannevk: and &quot;mkwst is hard to get hold of&quot;
17:52bzannevk: Obviously I can unilaterally change gecko to the behavior I think is right here, and I&#39;ve been considering just doing that
17:52bzannevk: but if Chrome doesn&#39;t follow, no one will end up using this window feature anyway
17:53bzannevk: since it totally breaks the UI in Chrome. :(
17:53bzannevk: The larger issue of lack of spec around window features is there, of course, but it&#39;s not really noopener-specific
18:30bkellybz: which feature is this?
18:31* bkelly is happy he didn&#39;t get backed out while getting lunch.
18:35bzbkelly: noopener
18:36bkellyI thought chrome cared about that
18:38* bz shrugs
18:38bzI did too
18:38bzThe link rel value works fine
18:38bzThe feature is totally borked
18:39bzNot usable by websites in practice
18:40bkellybz: I would still like to get clients.openWindow() exposed on main thread... async API, noopener behavior... it would be better for most sites I think
18:49annevkbz: I can ping mkwst tomorrow
18:49bzannevk: Thanks!
19:03bkellysmaug: do we care about sites creating infinite microtask loops hanging things?
19:04smaugmy patches, which I&#39;m again just atm updating, will deal with that also in Promise case
19:04bkellysmaug: oh, how do you handle it?
19:04smaugjust like we already do with microtasks
19:04smaugwith MutationObserver
19:05smaugbkelly: see mozilla::AutoSlowOperation usage
19:06bkellysmaug: ah, so it allows interruption and slow script dialog?
19:06smaugI don&#39;t think we can do anything else
19:06smaugPromise usage is occasionally silly
19:06smaugchaining lots of promises and janking badly
19:06bkellysmaug: I had wondered if it would be worth converting a microtask to a task if we detected a problem situation... like X consecutive microtasks and time >Y
19:06smaugbut not sure what we can do to that
19:07smaugthat would be bad
19:07smaugvery much breaking change
19:07bkellysmaug: what if we did it for things that aren&#39;t mutation observer?
19:07smauglike for promises?
19:08smaugwell, promises and mutation observer should use the same queue
19:08bkellysmaug: or for this thing Domenic is tryign to get implementor interest in:
19:09bkellysmaug: I guess I view it as similar to throttling timers or other tasks in problem situations... if we detect a microtask loop we force a yield at some point... like >50 microtasks and 4+ ms or something
19:09bkellyanyway, it was just a thought
19:10smaugit is very different from throttling
19:10smaugI mean, microtasks are sync operations from browser point of view
19:10smaugand the whole point of them from JS point of view is that microtasks check point happens before the next task
19:10bkellysmaug: maybe I am thinking mostly of promises where they can fire non-sync
19:11smaugright, when some API resolves promises
19:11bkellysmaug: what do you think of Domenic&#39;s proposal?
19:12smaugbkelly: I&#39;m a bit worried
19:12smaugI see it so that this makes it more likely to cause janks
19:12bkellyah, sorry I missed you commented on it
19:13bkellysmaug: I guess I was thinking about setImmediate() recently and if we could implement it with throttling protections like we did for setTimeout()
19:15bkelly&quot;This HashSet is shared between the StorageProcess and the WebProcesses via shared memory&quot; impressive goal there:
19:15smaugI think I don&#39;t have anything against setImmediate. It is after all quite similar to onmessage + postMessage
19:15bkellysmaug: yea, I was thinking of doing the same throttling mitigation for postMessage() as well
19:49jibsmaug: Is setImmediate effectively a JS api for &quot;queue a task&quot;?
19:50jibif so that seems different from onmessage + postMessage, which I couldn&#39;t get to work right:
19:50jibworks right in Chrome fwiw
19:55John-Galtbz: ping
20:05bzJohn-Galt: ack
20:05bkellyjib: that is because our promises don&#39;t do real microtasks yet
20:05bkellyI bet
20:06jibbkelly: when I run that fiddle the console lines from promises are where I&#39;d expect them. The odd one out is the onmessage one which comes *after*setTimeout in FF
20:07jibi.e. I get 0 1 2 3 4 6 5 in firefox, 0 1 2 3 4 5 6 in Chrome
20:07John-Galtbz: Is there any reason to try to maintain backwards compatibility in nsIContentPolicy at this point?
20:07bzJohn-Galt: no
20:08bzJohn-Galt: I mean, we have to not break whatever impls are in-tree
20:08bkellyjib: why would setTimeout(f, 0) fire after postMessage()?
20:08bkellythere is no clamping going on
20:08bzJohn-Galt: And ideally either not break comm-central or given them a migration path....
20:08bkellyjib: chrome has an open bug about clamping setTimeout(f, 0) to 1ms always
20:08bzJohn-Galt: (I say this as a Thunderbird user)
20:08jibbkelly: you tell me. Does it work for you?
20:08John-Galtbz: We&#39;re pretty inconsistent about what we pass for the loading principal, and pretty unclear about it what it means. I&#39;d like to explicitly pass the triggering principal and loading principal instead. Or possibly just the load info.
20:09bkellyjib: I&#39;m telling you your expectations are wrong, IMO
20:09bzJohn-Galt: worksforme!
20:09bzJohn-Galt: And yes, this was definitely a thing we wanted to change
20:09jibbkelly: ah, ok. I thought setTimeout(, 0) could take many milliseconds, i.e. be throttled out quite a bit
20:10John-Galtbz: OK. Well, if we have to worry about breaking Thunderbird, I think I&#39;ll do it in a follow-up bug... But for the moment, I just need access to the triggering principal, rather than the loading principal we&#39;re passing in some cases.
20:10John-GaltWhich seems to be fine, since whenever we actually want the loading principal, we get it from the context.
20:10bzJohn-Galt: Adding an arg is not a big deal
20:10bzJohn-Galt: in terms of tbird fallout
20:10bkellyjib: here is the chromium bug:
20:10bzJohn-Galt: I mean, they can just add it to their impls and ignore it
20:11bzJohn-Galt: Changing the semantics of the existing arg might need a bit more thought on their end..
20:11bkellyjib: setTimeout(f, 0) can be throttled, but it can also run as fast a normal task
20:11jibbkelly: And I thought postmessage+onmessage and setImmediate were attempts at what specs call &quot;queue a task&quot;, for which I&#39;ve been told setTimeout(,0) is way too late for (and to use
20:12John-Galtbz: I think just changing the request principal to always be the triggering principal should be fine, at least as far as the ways we&#39;re currently using it (which is to say, we&#39;re mostly not). We don&#39;t really clearly define what principal is used there, and a lot of the time it already *is* the triggering principal, just not always.
20:12bkellyjib: setTimeout(f, 0) clamps to 4ms if you nest it more than 4 times... setImmediate in theory would not do that
20:12John-GaltIf I&#39;m going to break Tb, I&#39;d rather just do it once.
20:12* John-Galt will do a quick comm-central check just to be sure
20:12bzJohn-Galt: Fair
20:12jibbkelly: why are you not surprised onmessage postMessage is slower than setTimeout(,0)?
20:12bzJohn-Galt: I know they have some conpol impls for sure
20:13bzJohn-Galt: How much they care about the principal past system-or-not I&#39;d have to look.
20:13John-Galtbz: I&#39;d be willing to bet most of them haven&#39;t changed since before we started passing that arg ;)
20:13John-GaltAnyway, I&#39;m checking now.
20:13* bz is pretty sure christoph broke some of their stuff
20:13bzand then we either fixed on the gecko side by reverting his changes or they updated or both...
20:13* bz can&#39;t recall details but can look them up
20:14bkellyjib: &quot;slower&quot; is not accurate... they are both runnables queued to the main event loop... you are queuing the setTimeout(f,0) first, so its running first
20:14jibah thanks for that info
20:14bzNote that per spec they can run in whatever order, btw
20:14bkellyyea, they are separate task sources
20:15jibbkelly: I&#39;ve verified that switching those lines worked
20:15John-GaltLooks like they only have one content policy, and it uses the principal arg the same way m-c does.
20:15bzjohn-galt: I think I was thinking about
20:15firebotBug 1347598 FIXED, Images sent from Hotmail/Outlook aren&#39;t displayed [embedded (&quot;cid:&quot;) image with attribute crossorigi
20:17jibsetImmediate() is a poor name though imho
20:18jibeveryone will s/setTimeout(f,0)/setImmediate(f)/ and then what was the point
20:18bkellyjib: the difference is that we now have the ability to implement setImmediate() safely without 4ms clamping
20:19bkellywell &quot;safely&quot; is my subjective opinion for forcing it to yield between excessive numbers of setImmediate() runnables
20:19jibok and we can&#39;t change setTimeout(,0) for backwards compatibility?
20:19bkellyprobably correct
20:20jibseems odd
20:20* jib crawls back under a rock
20:20bkellyjib: also, setTimeout(f,0) has other behavior that setImmediate() does not... setTimeout() combines callbacks in the same runnable where possible
20:20bkellyAIUI setImmediate() doesn&#39;t do that
20:21jibMight have been better to call that QueueATask() or something
20:21bzAny API that makes people have to spell queue, esp non-English speakers, is just a bad API
20:22jibok fair enough
20:22jibLineUpATask() ;)
20:23jibok setImmediate() doesn&#39;t seem so bad now
20:23bkellybut I don&#39;t really care about the name
20:24* jib loves the name setImmediate() suddenly
20:24bzAh, you must be worried about the caching behavior, then. ;)
20:29jibATisketATasket() is lossy
20:29bzLousy too
21:15qDotbz: Would it be worth it to make a C++ accessible enum for consts from WebIDL bindings? Have to redefine these in the implementation seems redundant.
21:18bzqdot: The theory is that they&#39;re legacy and there should be no more....
21:18bzqdot: That said,
21:18firebotBug 792059 NEW, Expose WebIDL constants to C++
21:18bzqdot: If we restrict it to just numeric constants, might be ok....
21:19bzqdot: in terms of complexity
21:20asuthugh, apparently if you open the editor in 2 browser windows or accidentally ctrl-w then ctrl-shift-t it, its etherpad-style-logic goes nuts and can corrupt everything even though it looks fine in the window
21:20bzasuth: you say it&#39;s glitchy?
21:21asuthbadum bum
21:21qDotbz: Hmm, ok, that makes sense. I&#39;m just starting to remove XPCOM interfaces with embedded consts and was trying to decide if it was worth the work to generate them.
21:21asuthluckily I was able to ctrl-c copy/paste the apparent state from the window and now all is well
21:22bzqdot: it probably is
21:22bzqdot: keeping things in sync kinda sucks
21:22qDotbz: Agreed. I&#39;ll probably go ahead and take it. Time to get the yak shaver sharpened.
21:29qDotEvery week where I remove more code than I add is a good week though.
10 Oct 2017
No messages
Last message: 7 days and 16 hours ago