Open
Bug 1276366
Opened 9 years ago
Updated 2 years ago
Remove support for chrome -> chrome window leaks
Categories
(Core :: DOM: Core & HTML, defect, P3)
Core
DOM: Core & HTML
Tracking
()
UNCONFIRMED
People
(Reporter: mccr8, Unassigned)
References
(Depends on 2 open bugs)
Details
(Whiteboard: [MemShrink:P2] btpp-active)
Attachments
(9 files, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
automatedtester
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
erahm
:
feedback+
|
Details | Diff | Splinter Review |
It is very easy for chrome JS to leak a JS window. For our own code, we do test against this, so we usually catch these errors (but not always: see bug 1276271). For addons, of course, we have almost no testing, so there can be leaks of windows even with basic operations (see bug 873163). In theory, we could extend the hueyfix to work for reference to chrome windows also. All it will take is a hero to dig through the failures, and hope there's no systematic problems in chrome code like those that bogged down bug 1084626.
Reporter | ||
Updated•9 years ago
|
Summary: Remove support for chrome -> chrome leaks → Remove support for chrome -> chrome window leaks
Reporter | ||
Comment 1•9 years ago
|
||
Be dazzled by my coding prowess!
Here's a fresh try run that any curious people can pick through in a few hours:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=da4876482348
Reporter | ||
Comment 2•9 years ago
|
||
I was calling this the "SuperHueyfix" as a joking reference to the Super Huey helicopter, but it will probably fix less leaks than the original hueyfix so that seems inaccurate.
Reporter | ||
Comment 3•9 years ago
|
||
The change seems to break Marionette somehow, which breaks all reftests. Plain mochitests are green as you'd expect. bc tests look like there are a handful of failures per test suite, which isn't terrible.
Comment 4•9 years ago
|
||
(In reply to Andrew McCreight [:mccr8] from comment #3)
> The change seems to break Marionette somehow, which breaks all reftests.
That would be the "blooeyfix", then?
Reporter | ||
Updated•8 years ago
|
Whiteboard: [MemShrink] → [MemShrink:P2]
Updated•8 years ago
|
Whiteboard: [MemShrink:P2] → [MemShrink:P2] btpp-active
Comment 5•8 years ago
|
||
Comment 6•8 years ago
|
||
WIP: https://treeherder.mozilla.org/#/jobs?repo=try&revision=74700e1876828869d64ff970fdcaad5a80a74f6a
Marionette tests are fixed, reftests are fixed, a fair amount of e10s mochitests are fixed. Still plenty of non-e10s to work on.
Comment 7•8 years ago
|
||
Comment 8•8 years ago
|
||
This is essentially a roll-up of what I've been looking at. I'm holding off on
fixing sessionstore and devtools stuff, I think we need to redirect to domain
experts there. I'll split out patches for what I've got and get a list of still
broken tests after the next try run completes.
- mainly focused on e10s, so non-e10s might still be blowing up
- marionette should work now
- devtools was doing some shady stuff with dummy documents in deleted frames,
keeping the frame around seems to have fixed some things
- fxui tests should be working now
- SessionStore was also doing some sketchy stuff, I think I have workarounds but
- We had some issues in jsm's, so those fixes *should* cascade out and clear
out a bunch of issues.
Updated•8 years ago
|
Assignee: nobody → erahm
Status: NEW → ASSIGNED
Comment 9•8 years ago
|
||
Comment 10•8 years ago
|
||
Comment 11•8 years ago
|
||
Comment 12•8 years ago
|
||
I suspect that leaking addons would suddenly encounter exceptions or induce exceptions in browser components which don't encounter dead wrappers in vanilla FF. And maybe addon authors won't easily notice if the breakage is subtle.
Is there telemetry for dead wrapper exceptions or something like that to determine the impact?
Comment 13•8 years ago
|
||
Comment 14•8 years ago
|
||
Comment 15•8 years ago
|
||
After closing the window it will most likely become a deadwrapper. Jut handle
that gracefully.
We now expect exceptions when trying to run a script on a closed window, so
that means I can't split this change out to a blocking bug.
Attachment #8786895 -
Flags: review?(dburns)
Comment 16•8 years ago
|
||
Comment 17•8 years ago
|
||
Comment on attachment 8786895 [details] [diff] [review]
Part 1: Handle dead windows in test_execute_script
Review of attachment 8786895 [details] [diff] [review]:
-----------------------------------------------------------------
::: testing/marionette/harness/marionette/tests/unit/test_execute_script.py
@@ +248,5 @@
> self.marionette.switch_to_window("xul")
> self.assertNotEqual(self.win, self.marionette.current_window_handle)
>
> def tearDown(self):
> + try:
can you use `self.assertRaises` here instead.
Attachment #8786895 -
Flags: review?(dburns) → review+
Comment 18•8 years ago
|
||
(In reply to David Burns :automatedtester from comment #17)
> Comment on attachment 8786895 [details] [diff] [review]
> Part 1: Handle dead windows in test_execute_script
>
> Review of attachment 8786895 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> ::: testing/marionette/harness/marionette/tests/unit/test_execute_script.py
> @@ +248,5 @@
> > self.marionette.switch_to_window("xul")
> > self.assertNotEqual(self.win, self.marionette.current_window_handle)
> >
> > def tearDown(self):
> > + try:
>
> can you use `self.assertRaises` here instead.
I'm not confident that it will always raise unfortunately. I guess I can do a try push with that instead.
Comment 19•8 years ago
|
||
(In reply to Eric Rahm [:erahm] from comment #18)
> (In reply to David Burns :automatedtester from comment #17)
> > Comment on attachment 8786895 [details] [diff] [review]
> > Part 1: Handle dead windows in test_execute_script
> >
> > Review of attachment 8786895 [details] [diff] [review]:
> > -----------------------------------------------------------------
> >
> > ::: testing/marionette/harness/marionette/tests/unit/test_execute_script.py
> > @@ +248,5 @@
> > > self.marionette.switch_to_window("xul")
> > > self.assertNotEqual(self.win, self.marionette.current_window_handle)
> > >
> > > def tearDown(self):
> > > + try:
> >
> > can you use `self.assertRaises` here instead.
>
> I'm not confident that it will always raise unfortunately. I guess I can do
> a try push with that instead.
just spotted this is in teardown... it's not ideal but I guess we can leave it as you have it.
Comment 20•8 years ago
|
||
Comment on attachment 8757489 [details] [diff] [review]
Also nuke chrome window references.
billm had some opinions on this in Bug 1299589 comment 5, mainly that we might want to wait for session store messages before nuking the compartment. Bill can you elaborate?
This might help with the headache I've been having with various session store tests.
Attachment #8757489 -
Flags: feedback?(wmccloskey)
Comment 21•8 years ago
|
||
Comment 22•8 years ago
|
||
Comment 23•8 years ago
|
||
Comment 24•8 years ago
|
||
Comment 25•8 years ago
|
||
Comment 26•8 years ago
|
||
Comment 27•8 years ago
|
||
Comment 28•8 years ago
|
||
Comment 29•8 years ago
|
||
This is an attempt to handle possibly dead browser windows in |onClose|. It probably breaks things.
Comment 30•8 years ago
|
||
Comment 31•8 years ago
|
||
Okay so I split out a bunch of blockers that can be landed independently, I've split out my updated WIP patches investigating failures and started a try run after each one hoping to be able to detect if any of the fixes actually add more failures.
Next step I'll file blockers for the remaining test failures along with any debug patches I have for them.
Comment 32•8 years ago
|
||
Current issues:
bc1
===
- toolkit/mozapps/extensions/test/browser/browser_cancelCompatCheck.js fails, doesn't cleanup, breaks everything else. I'll file a bug for this, I tried to track down the issue but am at a loss
bc2
===
- session store is rather broken, looks like possible a bug in my devtools patch
bc4
===
- browser/base/content/test/urlbar/browser_urlbarAboutHomeLoading.js is timing out
cl, dt*
==
- devtools is busted (missing Cu from devtools patch)
Spoke to devtools folks, they'd prefer to avoid using Cu (they're transition their tools to content). I'll update that WIP with try/catch blocks. Hopefully we'll get a greener run as well.
Comment 33•8 years ago
|
||
Swapping out some Cu.isDeadWrapper usage with try/catch
Updated•8 years ago
|
Attachment #8787019 -
Attachment is obsolete: true
Comment 34•8 years ago
|
||
Comment 35•8 years ago
|
||
Alright, it's going to be hard to make any progress on devtools bustage until bug 1288835 (the create a dummy element in a window and then destroy that window but rely on the dummy element still being all good behavior) is fixed and that's waiting on bug 1265796 which in theory is being worked on.
Pursuing session store failures is also probably not worthwhile until we get feedback from billm.
Comment 36•8 years ago
|
||
Comment 37•8 years ago
|
||
Comment 38•8 years ago
|
||
Hi Eric,
This is the solution we discussed for session store code. Here's the process I used to validate it:
1. Force session restore to take a little while to ensure that it loses the race with window nuking. To do that, I added a delay here:
http://searchfox.org/mozilla-central/source/browser/components/sessionstore/content/content-sessionStore.js#205
You may find this useful for testing. With this change (and your patch), session store does not save sessions on normal shutdown.
2. Then I applied the patch here. It waits to nuke the window until we're done doing session store stuff. This causes session store to work again.
I think this approach should work for a variety of other frontend stuff that needs to touch the window after it's closed.
Attachment #8797291 -
Flags: feedback?(erahm)
Comment on attachment 8757489 [details] [diff] [review]
Also nuke chrome window references.
Cancelling feedback. Please needinfo me or something if you need any more information. I'm happy to review this once it's closer to passing tests.
Attachment #8757489 -
Flags: feedback?(wmccloskey)
Comment 41•8 years ago
|
||
Comment on attachment 8797291 [details] [diff] [review]
nuking blocker patch
Review of attachment 8797291 [details] [diff] [review]:
-----------------------------------------------------------------
Bill thanks this is super helpful! I have a few questions below about threading/races.
::: browser/components/sessionstore/SessionStore.jsm
@@ +1330,2 @@
> TabStateFlusher.flushWindow(aWindow).then(() => {
> + domWindowUtils.removeNukeBlocker();
Is it possible that this won't get hit? Can flushWindow fail?
::: dom/base/nsGlobalWindow.cpp
@@ +8937,5 @@
> + nsCOMPtr<nsISupportsPRUint64> wrapper =
> + do_CreateInstance(NS_SUPPORTS_PRUINT64_CONTRACTID);
> + if (wrapper) {
> + wrapper->SetData(mID);
> + observerService->NotifyObservers(wrapper, mTopic.get(), nullptr);
Is |observerService->NotifyObservers| a synchronous call? If not should be be concerned about getting down to line 8963 before the session store observer is notified?
@@ +8988,5 @@
>
> void
> +nsGlobalWindow::AddNukeBlocker()
> +{
> + MOZ_ASSERT(IsOuterWindow());
Is this always main thread only? Or more specifically do we know it's only accessed from one thread?
@@ +8995,5 @@
> +
> +void
> +nsGlobalWindow::RemoveNukeBlocker()
> +{
> + MOZ_ASSERT(IsOuterWindow());
I suppose we'd want to assert that mNukeBlockers > 0
@@ +8998,5 @@
> +{
> + MOZ_ASSERT(IsOuterWindow());
> + mNukeBlockers--;
> + if (!mNukeBlockers) {
> + nsCOMPtr<nsIRunnable> runnable = new WindowDestroyedEvent(this, mWindowID, "");
Just to be clear, the empty topic indicates means we're not going to notify observers (of which I guess session store would have been one, which is what blocked nuking in the first place). Is that right?
In that case is the flow essentially: we already notified observers, blocked nuking in our observer, and are just continuing cleanup using the logic in the |WindowDestroyedEvent|
Attachment #8797291 -
Flags: feedback?(erahm) → feedback+
Comment 42•8 years ago
|
||
ni for the is "|observerService->NotifyObservers| a synchronous call?" question.
Flags: needinfo?(wmccloskey)
(In reply to Eric Rahm [:erahm] from comment #41)
> ::: browser/components/sessionstore/SessionStore.jsm
> @@ +1330,2 @@
> > TabStateFlusher.flushWindow(aWindow).then(() => {
> > + domWindowUtils.removeNukeBlocker();
>
> Is it possible that this won't get hit? Can flushWindow fail?
It seems possible, although the consequences of it failing are kind of bad (probably worse than not nuking the window).
> ::: dom/base/nsGlobalWindow.cpp
> @@ +8937,5 @@
> > + nsCOMPtr<nsISupportsPRUint64> wrapper =
> > + do_CreateInstance(NS_SUPPORTS_PRUINT64_CONTRACTID);
> > + if (wrapper) {
> > + wrapper->SetData(mID);
> > + observerService->NotifyObservers(wrapper, mTopic.get(), nullptr);
>
> Is |observerService->NotifyObservers| a synchronous call? If not should be
> be concerned about getting down to line 8963 before the session store
> observer is notified?
It is synchronous.
> @@ +8988,5 @@
> >
> > void
> > +nsGlobalWindow::AddNukeBlocker()
> > +{
> > + MOZ_ASSERT(IsOuterWindow());
>
> Is this always main thread only? Or more specifically do we know it's only
> accessed from one thread?
I guess we can assert. Certainly the session store code never runs off the main thread, and nothing in nsIDOMWindowUtils can be used off the main threa.d
> @@ +8995,5 @@
> > +
> > +void
> > +nsGlobalWindow::RemoveNukeBlocker()
> > +{
> > + MOZ_ASSERT(IsOuterWindow());
>
> I suppose we'd want to assert that mNukeBlockers > 0
Sure.
> @@ +8998,5 @@
> > +{
> > + MOZ_ASSERT(IsOuterWindow());
> > + mNukeBlockers--;
> > + if (!mNukeBlockers) {
> > + nsCOMPtr<nsIRunnable> runnable = new WindowDestroyedEvent(this, mWindowID, "");
>
> Just to be clear, the empty topic indicates means we're not going to notify
> observers (of which I guess session store would have been one, which is what
> blocked nuking in the first place). Is that right?
Not exactly. This is notifying a different observer topic ("outer-window-destroyed" I think) than session store uses. Session store was already notified of "domwindowclose" some time ago.
We just happen to be using the same runnable for both nuking and "*-window-destroyed". It might be cleaner to split it into two runnables. I was being lazy.
> In that case is the flow essentially: we already notified observers, blocked
> nuking in our observer, and are just continuing cleanup using the logic in
> the |WindowDestroyedEvent|
The flow is like this:
1. We notify domwindowclose. At this point we block nuking.
2. Some time later we notify "outer-window-destroyed" (or maybe "inner-") and try to nuke. We don't because of the block.
3. Later on, all the session store promises get resolved and we remove the blocker.
4. That triggers another runnable, which skips notifying the observer since we already did that the first time. It does the nuking.
Flags: needinfo?(wmccloskey)
Comment 44•8 years ago
|
||
Comment 45•8 years ago
|
||
(In reply to Eric Rahm [:erahm] from comment #44)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=5942c8b40d24
This seems a bit better, there are a bunch of bc timeouts, I believe that's due to bug 1299589 not being applied (in theory attachment 8757489 [details] [diff] [review] should have made that bug not necessary). There's a few dt failures, mainly from cleanup functions trying to cleanup event handlers on windows that are already dead.
Comment 46•8 years ago
|
||
Latest push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d87a0243dfdf
With the patch from bug 1299589 and some more devtools patches we're looking good. Only two devtools failures, one is an intermittent that might have gotten worse (bug 1280726) that I've gone ahead and fixed, the other is in 'test_inspector-dead-nodes.html' [1] which is a rather odd test. I think it's checking that things in devtools don't blow up when touching dead nodes, but now the test itself blows up.
We still have a few bc1, bc3, bc6 timeouts:
- bc1:
- browser_cancelCompatCheck.js
- browser_checkAddonCompatibility.js
- bc3:
- browser_privatebrowsing_windowtitle.js
- bc6:
- browser_644409-scratchpads.js, this times out but leaves an event handler around that makes every subsequent test fail. I can't repro this locally and it doesn't repro in taskcluster so using a one-click loaner won't help
In debug there's also:
- browser_pageInfo_plugins.js
[1] https://dxr.mozilla.org/mozilla-central/rev/42c95d88aaaa7c2eca1d278399421d437441ac4d/devtools/server/tests/mochitest/test_inspector-dead-nodes.html
Comment 47•8 years ago
|
||
Comment 48•8 years ago
|
||
(In reply to Eric Rahm [:erahm] from comment #47)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=23f53fbdbde7
Disabling 'browser_644409-scratchpads.js' allowed the rest of the bc6 tests to run, we look good there.
One more failure noted in debug:
- browser_394759_behavior.js
Comment 49•8 years ago
|
||
Further triage:
- browser_privatebrowsing_windowtitle.js
- Still seems to be issues with |BrowserTestUtils.closeWindow| (what we were working on in bug 1299589)
- Seems to be known orange, bug 1290593, but is now perma-orange
- browser_cancelCompatCheck.js
- Can't repro locally, logs indicate some callbacks are invalid. It's not terribly useful as I don't know where the callbacks come from. It's possible this could be a candidate for nuke blocking:
> [task 2016-10-05T18:48:37.354451Z] 18:48:37 INFO - 490 INFO Console message: 1475693234920 addons.manager WARN AddonListener threw exception when calling onPropertyChanged: TypeError: can't access dead object (resource://gre/modules/AddonManager.jsm:1757:1) JS Stack trace: AddonManagerInternal.callAddonListeners@AddonManager.jsm:1757:1 < this.AddonManagerPrivate.callAddonListeners@AddonManager.jsm:3076:5 < this.XPIProvider.updateAddonDisabledState@XPIProvider.jsm:5026:7 < .applyCompatibilityUpdate@XPIProviderUtils.js:390:7 < UpdateChecker.prototype.onUpdateCheckComplete@XPIProvider.jsm:6857:7 < UpdateParser.prototype.onLoad@AddonUpdateChecker.jsm:684:9 < UpdateParser/<@AddonUpdateChecker.jsm:592:49 < EventListener.handleEvent*UpdateParser@AddonUpdateChecker.jsm:592:5 < this.AddonUpdateChecker.checkForUpdates@AddonUpdateChecker.jsm:931:12 < UpdateChecker@XPIProvider.jsm:6796:18 < AddonWrapper.prototype.findUpdates@XPIProvider.jsm:7683:5 < gVersionInfoPage.onPageShow<@update.js:225:7 < TaskImpl_run@Task.jsm:322:42 < Handler.prototype.process@Promise-backend.js:937:23 < this.PromiseWalker.walkerLoop@Promise-backend.js:816:7 < Promise*this.PromiseWalker.scheduleWalkerLoop@Promise-backend.js:747:11 < this.PromiseWalker.schedulePromise@Promise-backend.js:779:7 < this.PromiseWalker.completePromise@Promise-backend.js:714:7 < promise_open_compatibility_window/<@browser_cancelCompatCheck.js:175:5 < EventListener.handleEvent*EventTargetInterposition.methods.addEventListener@RemoteAddonsParent.jsm:652:5 < AddonInterpositionService.prototype.interposeProperty/desc.value@multiprocessShims.js:160:52 < promise_open_compatibility_window@browser_cancelCompatCheck.js:158:3 < cancel_during_repopulate@browser_cancelCompatCheck.js:270:28 < TaskImpl_run@Task.jsm:322:42 < Handler.prototype.process@Promise-backend.js:937:23 < this.PromiseWalker.walkerLoop@Promise-backend.js:816:7 < Promise*this.PromiseWalker.scheduleWalkerLoop@Promise-backend.js:747:11 < this.PromiseWalker.schedulePromise@Promise-backend.js:779:7 < this.PromiseWalker.completePromise@Promise-backend.js:714:7 < get _worker/worker.onmessage@PromiseWorker.jsm:231:9 < EventHandlerNonNull*get _worker@PromiseWorker.jsm:217:5 < postMessage@PromiseWorker.jsm:291:9 < TaskImpl_run@Task.jsm:322:42 < Handler.prototype.process@Promise-backend.js:937:23 < this.PromiseWalker.walkerLoop@Promise-backend.js:816:7 < Promise*this.PromiseWalker.scheduleWalkerLoop@Promise-backend.js:747:11 < this.PromiseWalker.schedulePromise@Promise-backend.js:779:7 < Promise.prototype.then@Promise-backend.js:454:5 < this.DeferredSave.prototype._deferredSave@DeferredSave.jsm:220:5 < this.DeferredSave.prototype._startTimer/<@DeferredSave.jsm:175:40 < syncLoadManifestFromFile@XPIProvider.jsm:1567:5 < addMetadata@XPIProviderUtils.js:1654:21 < processFileChanges@XPIProviderUtils.js:2018:23 < this.XPIProvider.checkForChanges@XPIProvider.jsm:3822:34 < this.XPIProvider.startup@XPIProvider.jsm:2808:25 < callProvider@AddonManager.jsm:236:12 < _startProvider@AddonManager.jsm:789:5 < AddonManagerInternal.startup@AddonManager.jsm:973:9 < this.AddonManagerPrivate.startup@AddonManager.jsm:3017:5 < amManager.prototype.observe@addonManager.js:71:9
> [task 2016-10-05T18:48:37.369973Z] 18:48:37 INFO - 491 INFO Console message: 1475693234941 addons.manager WARN AddonListener threw exception when calling onEnabling: TypeError: can't access dead object (resource://gre/modules/AddonManager.jsm:1757:1) JS Stack trace: AddonManagerInternal.callAddonListeners@AddonManager.jsm:1757:1 < this.AddonManagerPrivate.callAddonListeners@AddonManager.jsm:3076:5 < this.XPIProvider.updateAddonDisabledState@XPIProvider.jsm:5051:9 < .applyCompatibilityUpdate@XPIProviderUtils.js:390:7 < UpdateChecker.prototype.onUpdateCheckComplete@XPIProvider.jsm:6857:7 < UpdateParser.prototype.onLoad@AddonUpdateChecker.jsm:684:9 < UpdateParser/<@AddonUpdateChecker.jsm:592:49 < EventListener.handleEvent*UpdateParser@AddonUpdateChecker.jsm:592:5 < this.AddonUpdateChecker.checkForUpdates@AddonUpdateChecker.jsm:931:12 < UpdateChecker@XPIProvider.jsm:6796:18 < AddonWrapper.prototype.findUpdates@XPIProvider.jsm:7683:5 < gVersionInfoPage.onPageShow<@update.js:225:7 < TaskImpl_run@Task.jsm:322:42 < Handler.prototype.process@Promise-backend.js:937:23 < this.PromiseWalker.walkerLoop@Promise-backend.js:816:7 < Promise*this.PromiseWalker.scheduleWalkerLoop@Promise-backend.js:747:11 < this.PromiseWalker.schedulePromise@Promise-backend.js:779:7 < this.PromiseWalker.completePromise@Promise-backend.js:714:7 < promise_open_compatibility_window/<@browser_cancelCompatCheck.js:175:5 < EventListener.handleEvent*EventTargetInterposition.methods.addEventListener@RemoteAddonsParent.jsm:652:5 < AddonInterpositionService.prototype.interposeProperty/desc.value@multiprocessShims.js:160:52 < promise_open_compatibility_window@browser_cancelCompatCheck.js:158:3 < cancel_during_repopulate@browser_cancelCompatCheck.js:270:28 < TaskImpl_run@Task.jsm:322:42 < Handler.prototype.process@Promise-backend.js:937:23 < this.PromiseWalker.walkerLoop@Promise-backend.js:816:7 < Promise*this.PromiseWalker.scheduleWalkerLoop@Promise-backend.js:747:11 < this.PromiseWalker.schedulePromise@Promise-backend.js:779:7 < this.PromiseWalker.completePromise@Promise-backend.js:714:7 < get _worker/worker.onmessage@PromiseWorker.jsm:231:9 < EventHandlerNonNull*get _worker@PromiseWorker.jsm:217:5 < postMessage@PromiseWorker.jsm:291:9 < TaskImpl_run@Task.jsm:322:42 < Handler.prototype.process@Promise-backend.js:937:23 < this.PromiseWalker.walkerLoop@Promise-backend.js:816:7 < Promise*this.PromiseWalker.scheduleWalkerLoop@Promise-backend.js:747:11 < this.PromiseWalker.schedulePromise@Promise-backend.js:779:7 < Promise.prototype.then@Promise-backend.js:454:5 < this.DeferredSave.prototype._deferredSave@DeferredSave.jsm:220:5 < this.DeferredSave.prototype._startTimer/<@DeferredSave.jsm:175:40 < syncLoadManifestFromFile@XPIProvider.jsm:1567:5 < addMetadata@XPIProviderUtils.js:1654:21 < processFileChanges@XPIProviderUtils.js:2018:23 < this.XPIProvider.checkForChanges@XPIProvider.jsm:3822:34 < this.XPIProvider.startup@XPIProvider.jsm:2808:25 < callProvider@AddonManager.jsm:236:12 < _startProvider@AddonManager.jsm:789:5 < AddonManagerInternal.startup@AddonManager.jsm:973:9 < this.AddonManagerPrivate.startup@AddonManager.jsm:3017:5 < amManager.prototype.observe@addonManager.js:71:9
> [task 2016-10-05T18:48:37.392145Z] 18:48:37 INFO - 499 INFO Console message: 1475693235041 addons.manager WARN AddonListener threw exception when calling onEnabled: TypeError: can't access dead object (resource://gre/modules/AddonManager.jsm:1757:1) JS Stack trace: AddonManagerInternal.callAddonListeners@AddonManager.jsm:1757:1 < this.AddonManagerPrivate.callAddonListeners@AddonManager.jsm:3076:5 < this.XPIProvider.updateAddonDisabledState@XPIProvider.jsm:5071:11 < .applyCompatibilityUpdate@XPIProviderUtils.js:390:7 < UpdateChecker.prototype.onUpdateCheckComplete@XPIProvider.jsm:6857:7 < UpdateParser.prototype.onLoad@AddonUpdateChecker.jsm:684:9 < UpdateParser/<@AddonUpdateChecker.jsm:592:49 < EventListener.handleEvent*UpdateParser@AddonUpdateChecker.jsm:592:5 < this.AddonUpdateChecker.checkForUpdates@AddonUpdateChecker.jsm:931:12 < UpdateChecker@XPIProvider.jsm:6796:18 < AddonWrapper.prototype.findUpdates@XPIProvider.jsm:7683:5 < gVersionInfoPage.onPageShow<@update.js:225:7 < TaskImpl_run@Task.jsm:322:42 < Handler.prototype.process@Promise-backend.js:937:23 < this.PromiseWalker.walkerLoop@Promise-backend.js:816:7 < Promise*this.PromiseWalker.scheduleWalkerLoop@Promise-backend.js:747:11 < this.PromiseWalker.schedulePromise@Promise-backend.js:779:7 < this.PromiseWalker.completePromise@Promise-backend.js:714:7 < promise_open_compatibility_window/<@browser_cancelCompatCheck.js:175:5 < EventListener.handleEvent*EventTargetInterposition.methods.addEventListener@RemoteAddonsParent.jsm:652:5 < AddonInterpositionService.prototype.interposeProperty/desc.value@multiprocessShims.js:160:52 < promise_open_compatibility_window@browser_cancelCompatCheck.js:158:3 < cancel_during_repopulate@browser_cancelCompatCheck.js:270:28 < TaskImpl_run@Task.jsm:322:42 < Handler.prototype.process@Promise-backend.js:937:23 < this.PromiseWalker.walkerLoop@Promise-backend.js:816:7 < Promise*this.PromiseWalker.scheduleWalkerLoop@Promise-backend.js:747:11 < this.PromiseWalker.schedulePromise@Promise-backend.js:779:7 < this.PromiseWalker.completePromise@Promise-backend.js:714:7 < get _worker/worker.onmessage@PromiseWorker.jsm:231:9 < EventHandlerNonNull*get _worker@PromiseWorker.jsm:217:5 < postMessage@PromiseWorker.jsm:291:9 < TaskImpl_run@Task.jsm:322:42 < Handler.prototype.process@Promise-backend.js:937:23 < this.PromiseWalker.walkerLoop@Promise-backend.js:816:7 < Promise*this.PromiseWalker.scheduleWalkerLoop@Promise-backend.js:747:11 < this.PromiseWalker.schedulePromise@Promise-backend.js:779:7 < Promise.prototype.then@Promise-backend.js:454:5 < this.DeferredSave.prototype._deferredSave@DeferredSave.jsm:220:5 < this.DeferredSave.prototype._startTimer/<@DeferredSave.jsm:175:40 < syncLoadManifestFromFile@XPIProvider.jsm:1567:5 < addMetadata@XPIProviderUtils.js:1654:21 < processFileChanges@XPIProviderUtils.js:2018:23 < this.XPIProvider.checkForChanges@XPIProvider.jsm:3822:34 < this.XPIProvider.startup@XPIProvider.jsm:2808:25 < callProvider@AddonManager.jsm:236:12 < _startProvider@AddonManager.jsm:789:5 < AddonManagerInternal.startup@AddonManager.jsm:973:9 < this.AddonManagerPrivate.startup@AddonManager.jsm:3017:5 < amManager.prototype.observe@addonManager.js:71:9
- browser_checkAddonCompatibility.js
- Can't reproduce locally, this could just be fallout from browser_cancelCompatCheck.js failing
- browser_394759_behavior.js
- This is classic poking a dead object, my guess not related to that test. PageThumbs is probably just doing a periodic thing and holding a busted ref to a window:
> A promise chain failed to handle a rejection: - at resource://gre/modules/PageThumbs.jsm:421 - TypeError: can't access dead object
> INFO - Stack trace:
> INFO - PageThumbs_captureAndStoreIfStale/<@resource://gre/modules/PageThumbs.jsm:421:1
Updated•8 years ago
|
Flags: needinfo?(gijskruitbosch+bugs)
Updated•8 years ago
|
Flags: needinfo?(gijskruitbosch+bugs)
Comment 50•7 years ago
|
||
mccr8: the code in nsGlobalWindow.cpp has changed a little; there's now some add-on stuff involved. Is removing the "&& !js::IsSystemCompartment(js::GetObjectCompartment(obj))" test still the right thing to do?
There's also a comment "We only want to nuke wrappers for the chrome->content case" that looks like it should be removed or changed.
Flags: needinfo?(continuation)
Reporter | ||
Comment 51•7 years ago
|
||
(In reply to Nicholas Nethercote [:njn] from comment #50)
> mccr8: the code in nsGlobalWindow.cpp has changed a little; there's now some
> add-on stuff involved. Is removing the "&&
> !js::IsSystemCompartment(js::GetObjectCompartment(obj))" test still the
> right thing to do?
Hmm, yes, I think so.
> There's also a comment "We only want to nuke wrappers for the
> chrome->content case" that looks like it should be removed or changed.
Yeah, I think it should be more like "We don't want to nuke content->content wrappers".
Flags: needinfo?(continuation)
Comment 52•7 years ago
|
||
I haven't worked on this in a while, we think switching to web extensions only in Firefox 57 might make this a non-issue so I haven't pursued it further.
dev-tools has gone full html so that might unblock a lot of bustage. I think session restore is the largest sticking point.
Status: ASSIGNED → NEW
Updated•7 years ago
|
Assignee: erahm → nobody
Status: NEW → UNCONFIRMED
Ever confirmed: false
Updated•6 years ago
|
Priority: -- → P3
Assignee | ||
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
Comment 53•6 years ago
|
||
Is this bug still an issue?
Reporter | ||
Comment 54•6 years ago
|
||
Yes.
Comment 55•4 years ago
|
||
Should the "tracking flag" be updated?
Reporter | ||
Updated•4 years ago
|
status-firefox49:
affected → ---
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•