Closed Bug 1267015 Opened 8 years ago Closed 8 years ago

Intermittent e10s browser_styleinspector_tooltip-multiple-background-images.js | Uncaught exception - at :0 - Error: operation not possible on dead CPOW

Categories

(DevTools :: Inspector: Rules, defect, P1)

Product:
DevTools
Component:
Inspector: Rules
Type:
defect

Tracking

(e10s+, firefox48 fixed, firefox49 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 49
Tracking Flags:
Tracking Status
e10s + ---
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: philor, Assigned: pbro)

References

(Blocks 1 open bug)

Details

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

Attachments

(2 files)

MozReview Request: Bug 1267015 - Don't access the test content process in browser_styleinspector_tooltip-multiple-background-images.js; r=bgrins
58 bytes, text/x-review-board-request
bgrins
: review+
Details
MozReview Request: Bug 1267015 - ESLint cleanup of devtools/client/styleinspector/shared/test; r=bgrins
58 bytes, text/x-review-board-request
bgrins
: review+
Details
Blocks: e10s-tests
tracking-e10s: --- → ?
Triaging (filter on CLIMBING SHOES).
Priority: -- → P3
Whiteboard: [btpp-backlog]
Bumping to P1. Failed 49 times in one week.
Priority: P3 → P1
I will remove the obvious CPOW usage in this test, hopefully this is all there is to it.
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
Comment on attachment 8752094 [details]
MozReview Request: Bug 1267015 - Don't access the test content process in browser_styleinspector_tooltip-multiple-background-images.js; r=bgrins

https://reviewboard.mozilla.org/r/52425/#review49447
Attachment #8752094 - Flags: review?(bgrinstead) → review+
Comment on attachment 8752095 [details]
MozReview Request: Bug 1267015 - ESLint cleanup of devtools/client/styleinspector/shared/test; r=bgrins

https://reviewboard.mozilla.org/r/52427/#review49451

::: devtools/client/inspector/shared/test/browser_styleinspector_output-parser.js:213
(Diff revision 1)
>        is(getUrl(fragment), "http://wow.com/cool/../../../you're(doingit)wrong");
>      }
>    },
>    {
>      name: "background-image",
> -    value: "url(../../../look/at/this/folder/structure/../../red.blue.green.svg   )",
> +    value: "url(../../../look/at/this/folder/structure/../" +

I really dislike breaking up hardcoded strings like this for the sake of lint, since it makes it very difficult to search for things.  But, so it goes

::: devtools/client/inspector/shared/test/browser_styleinspector_tooltip-longhand-fontfamily.js:112
(Diff revision 1)
>      "Tooltip contains the correct data-uri image");
>  }
>  
>  function getPropertyView(computedView, name) {
>    let propertyView = null;
> -  computedView.propertyViews.some(function(view) {
> +  computedView.propertyViews.some(function (view) {

Did we explicitly decide on the space-before-function-paren rule?  I ask because we still have way more instances of "function(" than "function (" in our code base, despite this rule, which is an argument to require the opposite.

The odd exception is that `function* () {}` seems about as popular as `function*() {}`
Attachment #8752095 - Flags: review?(bgrinstead) → review+
(In reply to Brian Grinstead [:bgrins] from comment #9)
> devtools/client/inspector/shared/test/browser_styleinspector_output-parser.
> js:213
> (Diff revision 1)
> >        is(getUrl(fragment), "http://wow.com/cool/../../../you're(doingit)wrong");
> >      }
> >    },
> >    {
> >      name: "background-image",
> > -    value: "url(../../../look/at/this/folder/structure/../../red.blue.green.svg   )",
> > +    value: "url(../../../look/at/this/folder/structure/../" +
> 
> I really dislike breaking up hardcoded strings like this for the sake of
> lint, since it makes it very difficult to search for things.  But, so it goes
Yeah, I agree. Linting should help us write readable code and prevent errors, not prevent us from writing readable code. We've discussed changing (or removing) the 80-char rule before, never really reached a consensus. At one point in the past, I spent some time fixing a lot of ESLint issues on multiple files, and it became very frustrating to have to split those long lines for no other reasons than to please ESLint.
I've sent an email to the list.

> devtools/client/inspector/shared/test/browser_styleinspector_tooltip-
> longhand-fontfamily.js:112
> (Diff revision 1)
> >      "Tooltip contains the correct data-uri image");
> >  }
> >  
> >  function getPropertyView(computedView, name) {
> >    let propertyView = null;
> > -  computedView.propertyViews.some(function(view) {
> > +  computedView.propertyViews.some(function (view) {
> 
> Did we explicitly decide on the space-before-function-paren rule?  I ask
> because we still have way more instances of "function(" than "function (" in
> our code base, despite this rule, which is an argument to require the
> opposite.
> 
> The odd exception is that `function* () {}` seems about as popular as
> `function*() {}`
We did take a decision for this. See the result and rationale in bug 1263258.
The code isn't yet consistent about this simply because we still have many ignored files.
https://hg.mozilla.org/mozilla-central/rev/2bc2f386191b
https://hg.mozilla.org/mozilla-central/rev/c4a209a8e2a8
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: