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

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[Treeherder Bug Filer]

Filed by: rvandermeulen [at] mozilla.com

https://treeherder.mozilla.org/logviewer.html#?job_id=3175081&repo=mozilla-aurora

http://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-aurora-linux64-asan/1470066888/mozilla-aurora_ubuntu64-asan_vm_test-mochitest-e10s-browser-chrome-3-bm121-tests1-linux64-build9.txt.gz

Comment 1

[BMO Automation]

Bulk assigning P3 to all open intermittent bugs without a priority set in Firefox components per bug 1298978.

Comment 2

[Ryan VanderMeulen [:RyanVM]]

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);

Comment 3

[Marcos Caceres [:marcosc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8c4a868eacca

Comment 4

[Marcos Caceres [:marcosc]]

Attachment: Use message instead

Mike, wdyt? I saw other tests doing this, um, "hack" with frame scripts as data URLs. Is that ok?

Comment 5

[Mike Conley (: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.

Comment 6

[Marcos Caceres [:marcosc]]

Attachment: Use Mike's suggestions

Awesome suggestions, thanks Mike!

Comment 7

[Mike Conley (: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"

Comment 8

[Marcos Caceres [:marcosc]]

(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!

Comment 9

[Marcos Caceres [:marcosc]]

Attachment: Fix typo

Comment 10

[Mike Conley (: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!

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/073e1d56b6cd
Fix intermittent e10s issue with oninstall. r=mconley

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/073e1d56b6cd

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1bd1cb922892

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/35d7d3c4d627