Closed Bug 1239566 Opened 4 years ago Closed 4 years ago

Intermittent e10s browser_webconsole_view_source.js | Uncaught exception - at :0 - Error: operation not possible on dead CPOW

Categories

(DevTools :: Console, defect, P3)

defect

Tracking

(e10s+, firefox47 fixed, firefox48 fixed)

RESOLVED FIXED
Firefox 48
Tracking Status
e10s + ---
firefox47 --- fixed
firefox48 --- fixed

People

(Reporter: philor, Assigned: bgrins)

References

(Blocks 1 open bug)

Details

(Keywords: intermittent-failure, Whiteboard: [btpp-backlog])

Attachments

(1 file)

Blocks: e10s-tests
tracking-e10s: --- → +
1 failure in the past month.
Priority: -- → P3
Whiteboard: [btpp-backlog]
Bumped into this on a random try push, looks easy fix so took it.  Here's an updated push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=db8c7812ef05
Comment on attachment 8740934 [details]
MozReview Request: Bug 1239566 - Get rid of CPOW in browser_webconsole_view_source.js;r=linclark

https://reviewboard.mozilla.org/r/46089/#review42585

::: devtools/client/webconsole/test/browser_webconsole_view_source.js:22
(Diff revision 1)
> -  let button = content.document.querySelector("button");
> -  ok(button, "we have the button on the page");
>  
>    // On e10s, the exception is triggered in child process
>    // and is ignored by test harness
>    if (!Services.appinfo.browserTabsRemoteAutostart) {

Would this block, catching the uncaught exception, apply to the code that was moved? TBH, I've seen these in a number of tests and wasn't quite sure under what conditions they were necessary.
Attachment #8740934 - Flags: review?(lclark)
(In reply to Lin Clark [:linclark] from comment #6)
> Comment on attachment 8740934 [details]
> MozReview Request: Bug 1239566 - Get rid of CPOW in
> browser_webconsole_view_source.js;r=linclark
> 
> https://reviewboard.mozilla.org/r/46089/#review42585
> 
> ::: devtools/client/webconsole/test/browser_webconsole_view_source.js:22
> (Diff revision 1)
> > -  let button = content.document.querySelector("button");
> > -  ok(button, "we have the button on the page");
> >  
> >    // On e10s, the exception is triggered in child process
> >    // and is ignored by test harness
> >    if (!Services.appinfo.browserTabsRemoteAutostart) {
> 
> Would this block, catching the uncaught exception, apply to the code that
> was moved? TBH, I've seen these in a number of tests and wasn't quite sure
> under what conditions they were necessary.

That block doesn't catch an exception, it tells the test harness to not fail when it receives one.  By default tests fail if any exception happens when they are running.  And the exception that is triggered when pressing the button is visible by the test harness in non-e10s (although doesn't need to be caught for the same reason we don't need to catch page errors from interrupting chrome code).

The block was there because the exception happens on the next line, but there's no reason it can't be moved earlier in the task.  Does that make sense?
Comment on attachment 8740934 [details]
MozReview Request: Bug 1239566 - Get rid of CPOW in browser_webconsole_view_source.js;r=linclark

Re-flagging based on Comment 7
Attachment #8740934 - Flags: review?(lclark)
Comment on attachment 8740934 [details]
MozReview Request: Bug 1239566 - Get rid of CPOW in browser_webconsole_view_source.js;r=linclark

https://reviewboard.mozilla.org/r/46089/#review42627

Based on the convo, this looks good to me.
Attachment #8740934 - Flags: review?(lclark) → review+
Try push is looking fine, marking checkin-needed
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/67ac22444604
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.