Closed Bug 1383328 Opened 7 years ago Closed 7 years ago

Label nsBrowserStatusFilter timer with TabGroup of tab that owns the filter

Categories

(Core :: XPCOM, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: billm, Assigned: billm)

References

Details

Attachments

(1 file)

Attached file patch
In bug 1363877, I labeled one of the runnables (a timer) for nsBrowserStatusFilter with the SystemGroup. That turned out to be incorrect, though. Some of the code that runs off of nsBrowserStatusFilter does touch tab content. Here is an example:
http://searchfox.org/mozilla-central/rev/3a3af33f513071ea829debdfbc628caebcdf6996/toolkit/content/browser-child.js#65

When I talked about this code with Felipe, I assumed that this was okay since no script actually runs. However, when I actually started using cooperative threads, I realized that the JS engine doesn't even allow us to enter a content compartment when we're in the SystemGroup. So just accessing a property on the document causes an assertion.

The patch here allows us to pass an event target to dispatch the nsBrowserStatusFilter timer to. I also added some code that allows us to get labeled event targets for each TabChild from JS, as well as get a SystemGroup event target from JS. Only TaskCategory::Other is supported, but I think that's fine.
Attachment #8888990 - Flags: review?(bugs)
Comment on attachment 8888990 [details]
patch


>+NS_IMETHODIMP
>+nsDocLoader::GetTarget(nsIEventTarget** aTarget)
>+{
>+  *aTarget = nullptr;
>+  return NS_ERROR_NOT_IMPLEMENTED;
>+}
>+
>+NS_IMETHODIMP
>+nsDocLoader::SetTarget(nsIEventTarget* aTarget)
>+{
>+  return NS_ERROR_NOT_IMPLEMENTED;
>+}

GetTarget could usually return target from the tabgroup docloader/docshell belongs to, no?
With that added, r+
Attachment #8888990 - Flags: review?(bugs) → review+
Pushed by wmccloskey@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e3e924da5d5f
Use TabGroup as event target for browser-status-filter (r=smaug)
https://hg.mozilla.org/mozilla-central/rev/e3e924da5d5f
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Backout by cbook@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/01b583d44fff
Backed out changeset e3e924da5d5f
had to back this out in https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=5845151f1a2cd00957fdd48e204542ccbdfaba1e because of the backout of bug 1351148 and in order to do a clean backout
Status: RESOLVED → REOPENED
Flags: needinfo?(wmccloskey)
Resolution: FIXED → ---
Pushed by wmccloskey@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fac66a370cf5
Use TabGroup as event target for browser-status-filter (r=smaug)
https://hg.mozilla.org/mozilla-central/rev/fac66a370cf5
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Flags: needinfo?(wmccloskey)
You need to log in before you can comment on or make changes to this bug.