Closed Bug 1283704 Opened 4 years ago Closed 4 years ago

Intermittent browser/base/content/test/plugins/browser_bug812562.js | Uncaught exception - at :0 - Error: operation not possible on dead CPOW

Categories

(Core :: Plug-ins, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox49 --- fixed
firefox50 --- fixed
firefox51 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: kevchan85, Mentored)

Details

(Keywords: good-first-bug, intermittent-failure)

Attachments

(2 files)

mconley, can you give me some hints about what CPOW is actually involved here? The failure is somewhere between

http://searchfox.org/mozilla-central/rev/a7c8e9f3cc323fd707659175a46826ad12899cd1/browser/base/content/test/plugins/browser_bug812562.js#54 and #70 

here's the relevant bits of the log:
21:57:42     INFO -  60 INFO TEST-PASS | browser/base/content/test/plugins/browser_bug812562.js | test part 2: Should not have a click-to-play notification -
21:57:42     INFO -  61 INFO Console message: [JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "data:text/html,<html></html>" line: 0}]
21:57:42     INFO -  62 INFO TEST-PASS | browser/base/content/test/plugins/browser_bug812562.js | test part 2: plugin should not be activated - true == true -
21:57:42     INFO -  63 INFO TEST-UNEXPECTED-FAIL | browser/base/content/test/plugins/browser_bug812562.js | Uncaught exception - at :0 - Error: operation not possible on dead CPOW
21:57:42     INFO -  Stack trace:
21:57:42     INFO -      Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:937:23
21:57:42     INFO -      this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:816:7
21:57:42     INFO -      Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:747:11
21:57:42     INFO -      this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:779:7
21:57:42     INFO -      this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:714:7
21:57:42     INFO -      Tester_execTest@chrome://mochikit/content/browser-test.js:791:9
21:57:42     INFO -      Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:711:7
21:57:42     INFO -      SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:741:59
21:57:42     INFO -  64 INFO Leaving test bound
21:57:42     INFO -  65 INFO Entering test bound
21:57:42     INFO -  Not taking screenshot here: see the one that was previously logged
21:57:42     INFO -  66 INFO TEST-UNEXPECTED-FAIL | browser/base/content/test/plugins/browser_bug812562.js | test part 3: Should have a click-to-play notification -
21:57:42     INFO -  Stack trace:
21:57:42     INFO -      chrome://mochitests/content/browser/browser/base/content/test/plugins/browser_bug812562.js:null:71
21:57:42     INFO -      Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:940:21
21:57:42     INFO -      this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:816:7
21:57:42     INFO -      Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:747:11
21:57:42     INFO -      this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:779:7
21:57:42     INFO -      this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:714:7
21:57:42     INFO -      receiveMessage@resource://testing-common/ContentTask.jsm:113:9
Flags: needinfo?(mconley)
Probably here:

http://searchfox.org/mozilla-central/rev/a7c8e9f3cc323fd707659175a46826ad12899cd1/browser/base/content/test/plugins/browser_bug812562.js#62

You can probably fix this by changing that to gTestBrowser.goBack();
Flags: needinfo?(mconley)
Indeed! This could make a pretty nice good-first-bug.
Mentor: mconley
Keywords: good-first-bug
Priority: -- → P3
Can I have information on how to test this particular module?
I can absolutely help you get started. First of all, do you have a build of Firefox ready to go? If not, let me know and I can guide you through that.

If so, you can run the test in question using this command from within the repository directory:

./mach mochitest browser/base/content/test/plugins/browser_bug812562.js

That should, after a few seconds, open up an instance of Firefox, run the test, and (if all goes well) close it all down. There will be a bunch of stuff spewed to the console, as well.

This bug is about an intermittent failure, meaning that _sometimes_ (due to a timing issue) the test will fail, and sometimes it won't. Sometimes those failures will only occur on our test machines in automation, which is pretty frustrating if you're trying to fix the failure locally.

So check to see if the test passes for you locally. If it doesn't - great! We can work with that. If it does, that's still okay - we can try the fix I suggest in the bug and push it to automation to see if it fixes it.

The next step is to open the test file at browser/base/content/test/plugins/browser_bug812562.js, at the line I point at in the bug, and change the line I mentioned to just:

gTestBrowser.goBack();

If you're interested in why this fixes the bug, it's because the old line:

gTestBrowser.contentWindow.history.back();

is accessing the "contentWindow" property, which (when running with multi-process Firefox) uses our "CPOW"[1] layer to synchronously message the content that's running in the other process. This usually works, but because CPOWs don't hold strong references to the things in the other processes, it means that if the content window in the other process disappears, then accessing gTestBrowser.contentWindow.anything will result in a "dead CPOW" error, meaning that the item in the other process has disappeared. Using gTestBrowser.goBack() avoids the CPOW, and uses a "message" instead to send the browser back, which should be safer.

Give that a shot, and see if the test still passes for you locally. If it does, great! Attach your patch (which I can guide you through) to the bug (or use MozReview[2] to post it), and we'll send it to automation to see if the intermittent has gone.

I hope that helps! Let me know. Feel free to use the "needinfo" flag (towards the bottom of this page) on "mentor" to get my attention.

[1]: mikeconley.ca/blog/2015/02/17/on-unsafe-cpow-usage-in-firefox-desktop-and-why-is-my-nightly-so-sluggish-with-e10s-enabled/
[2]: https://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview-user.html
Attachment #8784668 - Flags: review?(mconley)
Hello! 
Thanks for the tips. Did I attach the patch correctly?
(In reply to kevin from comment #7)
> Hello! 
> Thanks for the tips. Did I attach the patch correctly?

You did indeed! I'll push this patch to try to see if the error has gone away. If so, I think we've got a winner.
Comment on attachment 8784668 [details] [diff] [review]
bug1283704_intermittent_browser.diff

I did a bunch of retriggers, and while I see some oranges, I don't see this one.

Unfortunately, this one was rather rare, so I can't be absolutely sure we fixed it, but you _have_ removed a CPOW here, so I have pretty high confidence.

Great work!
Attachment #8784668 - Flags: review?(mconley) → review+
Author: kevin <kevchan85@yahoo.com>
Bug number: 1283704
Commit message: Bug 1283704 - Remove a CPOW from browser_bug812562.js to avoid a dead CPOW intermittent. r=mconley
Keywords: checkin-needed
Hey kevin, great job on this one? Want another? Feel like tackling bug 1134307?
Assignee: nobody → kevchan85
Flags: needinfo?(kevchan85)
Heh, that should have been "great job on this one!", with the exclamation, and not the question mark. :)
awesome I was about to ask for a different one to work on today.
Flags: needinfo?(kevchan85)
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4406e97ee4e5
Use .goBack() instead of contentWindow.history.back() to avoid CPOW usage in browser_bug812562.js. r=mconley
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4406e97ee4e5
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.