Intermittent dom/manifest/test/browser_fire_install_event.js | Uncaught exception - at :0 - Error: operation not possible on dead CPOW

RESOLVED FIXED in Firefox 50

Status

()

defect
P3
normal
RESOLVED FIXED
3 years ago
4 months ago

People

(Reporter: intermittent-bug-filer, Assigned: marcosc)

Tracking

({intermittent-failure})

unspecified
mozilla52
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox49 wontfix, firefox50 fixed, firefox51 fixed, firefox52 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Bulk assigning P3 to all open intermittent bugs without a priority set in Firefox components per bug 1298978.
Priority: -- → P3
This is one of the more common dead CPOW oranges being hit in automation. Can you please take a look, Marcos?

TEST-UNEXPECTED-FAIL | dom/manifest/test/browser_fire_install_event.js | Uncaught exception - at resource://gre/modules/RemoteAddonsParent.jsm:880 - Error: operation not possible on dead CPOW
Stack trace:
    getContentDocument@resource://gre/modules/RemoteAddonsParent.jsm:880:3
    RemoteBrowserElementInterposition.getters.contentDocument@resource://gre/modules/RemoteAddonsParent.jsm:897:10
    AddonInterpositionService.prototype.interposeProperty/desc.get@resource://gre/components/multiprocessShims.js:165:38
    theTest/responsePromise<@chrome://mochitests/content/browser/dom/manifest/test/browser_fire_install_event.js:26:5
    theTest@chrome://mochitests/content/browser/dom/manifest/test/browser_fire_install_event.js:25:27
    @chrome://mochitests/content/browser/dom/manifest/test/browser_fire_install_event.js:43:9
    @chrome://mochitests/content/browser/dom/manifest/test/browser_fire_install_event.js:43:9
    Tester_execTest@chrome://mochikit/content/browser-test.js:784:9
    Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:704:7
    SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:741:59
    Tester_execTest@chrome://mochikit/content/browser-test.js:784:9
    Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:704:7
    SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:741:59

Looks like this is coming from line 26 maybe?
> aBrowser.contentDocument.addEventListener("dom.manifest.oninstall", resolve);
Flags: needinfo?(mcaceres)
Assignee: nobody → mcaceres
Flags: needinfo?(mcaceres)
Posted patch Use message instead (obsolete) — Splinter Review
Mike, wdyt? I saw other tests doing this, um, "hack" with frame scripts as data URLs. Is that ok?
Attachment #8791066 - Flags: review?(mconley)
Comment on attachment 8791066 [details] [diff] [review]
Use message instead

Review of attachment 8791066 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks again, Marcos! Glad to see us de-CPOW'ing our tests. Thankfully, I think there's a simpler mechanism we can use for messaging between the parent and content. See below.

::: dom/manifest/test/browser_fire_install_event.js
@@ +31,2 @@
>    // Resolves when we get a custom event with the correct name
> +  mm.loadFrameScript("data:," + frameScript, true);

This is another place where ContentTask.spawn is useful.

I believe you can do:

const responsePromise = ContentTask.spawn(aBrowser, null, function*() {
  return new Promise((resolve) => {
    addEventListener("install", function onInstall() {
      removeEventListener("install", onInstall);
      resolve();
    });
  });
});

and that way, you don't need to hook up your own framescript or do your own messaging - ContentTask will take care of it for you.
Attachment #8791066 - Flags: review?(mconley) → review-
Posted patch Use Mike's suggestions (obsolete) — Splinter Review
Awesome suggestions, thanks Mike!
Attachment #8791066 - Attachment is obsolete: true
Attachment #8791895 - Flags: review?(mconley)
Comment on attachment 8791895 [details] [diff] [review]
Use Mike's suggestions

Review of attachment 8791895 [details] [diff] [review]:
-----------------------------------------------------------------

This looks mostly fine - just clearing review request until I hear more about this event name change.

::: dom/manifest/test/browser_fire_install_event.js
@@ +21,5 @@
>  // This cause file_reg_install_event.html to be dynamically change.
>  function* theTest(aBrowser) {
>    aBrowser.allowEvents = true;
> +  let waitForInstall = ContentTask.spawn(aBrowser, null, function*() {
> +    yield ContentTaskUtils.waitForEvent(content.window, "install");

Did this event name change? It was "dom.manifest.oninstall" before, and it's "install" now. Are they synonymous?

@@ +30,5 @@
> +  try {
> +    yield waitForInstall;
> +    ok(true, "Install event fired");
> +  } catch (err) {
> +    ok(false, "Install event didn't fired: " + err.message);

Typo: "fired" -> "fire"
Attachment #8791895 - Flags: review?(mconley)
(In reply to Mike Conley (:mconley) - (needinfo me!) from comment #7)
> Comment on attachment 8791895 [details] [diff] [review]
> Use Mike's suggestions
> 
> Review of attachment 8791895 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This looks mostly fine - just clearing review request until I hear more
> about this event name change.
> 
> ::: dom/manifest/test/browser_fire_install_event.js
> @@ +21,5 @@
> >  // This cause file_reg_install_event.html to be dynamically change.
> >  function* theTest(aBrowser) {
> >    aBrowser.allowEvents = true;
> > +  let waitForInstall = ContentTask.spawn(aBrowser, null, function*() {
> > +    yield ContentTaskUtils.waitForEvent(content.window, "install");
> 
> Did this event name change? It was "dom.manifest.oninstall" before, and it's
> "install" now. Are they synonymous?

Sorry about the confusion. The "dom.manifest.oninstall" was the message name that triggers the event to fire in the frame script, not the event. The event is "oninstall" on the window object. 

It is defined here:
https://www.w3.org/TR/appmanifest/#oninstall-attribute

 
> @@ +30,5 @@
> > +  try {
> > +    yield waitForInstall;
> > +    ok(true, "Install event fired");
> > +  } catch (err) {
> > +    ok(false, "Install event didn't fired: " + err.message);
> 
> Typo: "fired" -> "fire"

D'oh. Fixed!
Posted patch Fix typoSplinter Review
Attachment #8791895 - Attachment is obsolete: true
Attachment #8792386 - Flags: review?(mconley)
Comment on attachment 8792386 [details] [diff] [review]
Fix typo

Review of attachment 8792386 [details] [diff] [review]:
-----------------------------------------------------------------

Okie dokie, let's do it. Thanks MarcosC!
Attachment #8792386 - Flags: review?(mconley) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/073e1d56b6cd
Fix intermittent e10s issue with oninstall. r=mconley
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/073e1d56b6cd
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.