mozilla :: #content

11 Sep 2017
08:15annevkIRCCloud failing in Nightly known?
08:15annevkHmm maybe it was some kind of network issue
08:16annevkYeah, looks like it was, but IRCCloud reports it as your browser having WebSocket problems
08:16annevkThat's not great
08:42pauljtannevk: its affecting release and ircloud on IOS too
08:42pauljtat least, for me
08:43annevkpauljt: yeah, it also failing on my phone was how I noticed
10:51emiliosmaug: if you're around and can talk about bug 1397983 I'd appreciate it. My idea was that FlattenedTreeIterator didn't make sense in detached subtrees, but icbw.
10:51firebot NEW, stylo: Assertion failure: !mServoRestyleRoot || mServoRestyleRoot == aRoot || nsContentUtils::Conten
10:51smaugemilio: pong
10:52smaugemilio: why wouldn't FlattenedTreeIterator make sense in not-in-composed-doc trees?
10:52smaugI'd expect it to work always
10:52emiliosmaug: well, in that case my assertions make no sense / probably should be moved to the frame constructor
10:53emiliosmaug: will remove them then, I was just surprised that the only usage of those was from chrome / devtools APIs
10:53smaugcreating frames sure shouldn't deal with !IsInComposedDoc elements, so sounds reasonable
10:54emiliosmaug: does the `InstallAnonymousContent` bits in that patch make sense to you? I'm still somewhat confused about the FORCE_XBL_BINDINGS stuff
10:54smaugemilio: which patch should I look at?
10:54emiliosmaug: last one (first one was removed, second one goes away)
10:55emiliosmaug: s/removed/reviewed
10:57smaugwhat is !GetShadowRoot( about?
10:58smaugInstallAnonymousContent call sounds wrong there, if we have already installed anon content
10:58smaug(even though the actual thing which InstallAnonymousContent does might be right)
10:58emiliosmaug: it's basically the same check we do to unbind the stuff
11:00emiliosmaug: actually it sounded wrong to do the `RemoveSubtreeFromDocument` bits on unbind and not the `AddSubtreeToDocument` on bind
11:01smaughow so?
11:01smaugbut yeah, Install and Uninstall do similar things
11:01smaugthey should be called something else
11:02smaugwould it be large change to rename them?
11:02emiliosmaug: probably not, they only have a handful of callers
11:02smaugBindAnonymousContent and UnbindAnonymousContent
11:02smaugor some such
11:02emiliosmaug: sounds good
11:03emiliosmaug: will submit and updated patch
12:19gsneddershsivonen: FWIW, I definitely remember finding some depth limit in Chrome previously and getting to silly-deep things with Edge before it eventually hung. If Chrome's changed behaviour, that might be worth looking into.
12:19gsneddershsivonen: either that or my memory is wrong :)
12:23gsneddershsivonen: ah, is what I'm thinking of. Chrome limits the tree at 513 nodes deep (where the root is 1 deep)
12:25gsneddershsivonen: will send email after I get back in an hour or so
13:17smaugmystor: does the bhr data for child process look right to you?
13:17smaugor do we just have less hangs on child process now?
13:18smaugthe timeline seems to be empty
13:18smaugbut stack trace area shows something
13:25smaugat least the hangs look different now
13:29smaugmystor: how would one check whether the overall number of hangs has changed?
13:29* smaug thinks the UI has some bug.
14:21mystorsmaug: sorry I missed this
14:22mystorsmaug: the problem there if I had to guess is that before the most recent run the units for hangs/hr were actually not hangs/hr, they were some arbitrary other unit because the new bhr ping didn't have uptime. I think that dthayer recently reran with actual support for hangs/hr.
14:23bkellyI wish we had documentation on how to submit a patch that didn't involve mozreview
14:23emiliosmaug: do you know when I should use BindingManager::GetBindingWithContent(this) vs. this->GetXBLBinding()? In particular, we use the former on bind and the later on unbind, and I'm not sure why.
14:24mystorsmaug: the last one probably was actually hangs/day if I had to guess (dthayer would know for sure), while the new one is more representative.
15:08smaugmystor: FWIW, I did land some patch(es?) last week which might help with hangs
15:08smaugemilio: I would need to look at the code
15:08mystorsmaug: awesome :-)
15:09emiliosmaug: got it figured out, np, just waiting for try to confirm
15:09smaugmystor: in other words, less likely to synchronously run GC
15:09mystorsmaug: \o/
15:09mystora true hero
18:22bkellymccr8: is there a bug for enabling jsloader.shareGlobal?
18:22firebotBug 1381961 NEW, Enabled shared global for JSMs pref
18:22mccr8bkelly: it seems to mostly work now if you want to try it yourself. :)
18:23mccr8Just make sure you update to the latest Nightly, as in the 9-9 Nightly there was a problem with cached scripts...
18:25bkellymccr8: were the talos improvements expected? I guess I thought it was a memory improvement
18:26mccr8bkelly: the goal was a memory improvement, but it turns out that getting rid of tens of megabytes of cross compartment wrappers also improves speed. ;)
18:26mccr8though there was some concern that it might cause a regression because it can't be Ion compiled or something. But it looks like an improvement over all.
18:45mystorsmaug: ping
18:51smaugmystor: pong
18:52mystorsmaug: So, somewhat recently I did a small change to DataTransfer to bring it more in line with Chrome's implementation and the spec, and it appears that that change was somewhat web-incompatible - as in it at the very least seems to have broken dropping images on irccloud (bug 1398883).
18:52firebot NEW, Drag & drop image upload broken on
18:53smaugI assume dnd works in Chrome
18:53mystorsmaug: Yeah, I think so. I'll check 1 sec
18:53smaugmccr8: would you object decreasing
18:53mccr8smaug: by how much? :)
18:54smaugin bhr reports BuildGraph shows up in cases there is lots of JS to trace
18:54smaugmccr8: I was thinking to 100 ?
18:54smaugthis is hard to evaluate
18:54smaugthough, I guess I could profile speedometer at least
18:54mystorsmaug: Yeah, it works fine in chrome
18:54mccr8smaug: maybe try 500 first and see if it affects anything?
18:55smaugfine, I can start with that
18:55* smaug files a bug
18:55mystorsmaug: gimme a minute - I should double-check that the problem is what I think it is.
18:56smaugmccr8: hmm, should we check budget more often when the budget is small
18:57smaugoh well, I'll try with 500 first
19:05mystorsmaug: Ok - so chrome's doing something a bit weird
19:06mystorsmaug: Basically it appears that when chrome cleans up its DataTransfer after a drop event, it doesn't clean up its `.files` list properly, so irccloud is able to read that after the drag event has ended, and we clean it up correctly after my patch
19:08smaugwhat is the reason to clean up DataTransfer after drop?
19:09mystorsmaug: To try to match the spec and behave in a way which matches other browsers.
19:09mystorsmaug: technically there are theoretical security concerns depending on how its modeled internally
19:10smaugI wonder how other browsers work
19:10mystorsmaug: The spec defines it this way because in the spec the data isn't in the DataTransfer, but in the Drag Data Store which is defined as a persistent background object, so you could theoretically read data you shouldn't be able to if it isn't disconnected - for us that hasn't been a problem because of our model for Drag and Drop operations
19:10smaugdo they have similar behavior as Chrome
19:10smaugand is the spec just buggy
19:10mystornobody acts identically
19:11mystorsmaug: edge never really cleans up after itself and actually allows you to modify the original DataTransfer after the dragstart event has fired
19:11smaugCould we just backout the patch from FF57?
19:11mystorsmaug: I don't quite remember what safari does...
19:11mystorsmaug: Yeah, it wouldn't be too hard to turn it off - I could put it behind a pref
19:12smaugjust thinking about the risks here
19:12smaugdnd issues probably aren't such which get filed too soon
19:12mystorsmaug: Yeah, I could also do what chrome does and just not kill the files array, and I think irccloud would work again, but I'm worried a bit about other cases
19:12smaugand there aren't too good tests
19:13mystorsmaug: So turn it off for 57 and we can take another look in 58 or beyond perhaps?
19:13smaugyeah, I would do that, unless there is some good reason to have it in FF57
19:13mystorsmaug: Sounds good - thanks :-)
20:03billmfroydnj: ping
20:03froydnjbillm: pong
20:04billmfroydnj: hey, have you had a chance to look at the new patches in bug 1398423?
20:04firebot NEW, Fix some LabeledEventQueue bugs
20:05froydnjbillm: was just working through the review queue, was planning on getting to them by the end of day
20:05billmfroydnj: great, thanks
20:05billmfroydnj: I'll try to get to your patches in the next couple days
20:06froydnjbillm: great, thank you!
20:39mccr8smaug: another thing we could think about would be bumping the counter inside the NoteXPCOM/JSChild callbacks instead of at the top of the loop.
20:40mccr8smaug: it might be tricky to set up, but then we'd avoid the problem of objects with a gazillion fields taking a long time.
20:40mccr8smaug: the GC does something akin to that to handle scanning large arrays.
20:40mccr8eg globals have a ton of fields, and we may end up tracing a lot of globals at the start of a CC, so maybe we end up with longer CCs because of that...
20:41smaugyeah, I think I was looking into that at some point
20:41mccr8You'd just have to stick a timestamp on the nsCC object itself.
20:41mccr8err slicebudget
21:14bkellymy new coding music...
21:15smauggood one
21:28bkellysmaug: ok... I only lasted 15 minutes before I had to stop it
12 Sep 2017
No messages
Last message: 10 days and 3 hours ago