Closed Bug 1274046 Opened 3 years ago Closed 3 years ago

Add FailureId to gfxConfig (FeatureState)

Categories

(Core :: Graphics, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: BenWa, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: feature, Whiteboard: [gfx-noted])

Attachments

(1 file)

We need a clear failureId to report failures to telemetry. We should have an explicit field for it.
Keywords: feature
OS: Unspecified → All
Hardware: Unspecified → All
Whiteboard: [gfx-noted]
Can we this to Instance instead of FeatureState? In theory failure IDs could get attached at multiple levels (blocklist fail, force-enable, runtime fail).

Are you think that only the most recent failure ID matters? If so, that seems okay, though it'd be nice if we could hide that and add a failure ID parameter to the relevant state functions (like SetFailed, Disable) anyway.
Flags: needinfo?(bgirard)
Yea I was on the fence about it. (On the side it's not immediately obvious how Failed/Disable match with each Instance, I think we could pick clearer names.) Right now I only want to report the most relevant failureId and I don't see how we would deal with multiple failure ids in the future so I opted for this way.

It's not a bad idea to set the API properly however. I'll take a look.
Flags: needinfo?(bgirard)
Comment on attachment 8754075 [details]
Bug 1274046 - Add FailureId to gfxConfig (FeatureState).

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53706/diff/1-2/
Attachment #8754075 - Attachment description: MozReview Request: Bug 1274046 - Add FailureId to gfxConfig (FeatureState). r=dvander → MozReview Request: Bug 1274046 - Add FailureId to gfxConfig (FeatureState). WIP
Blocks: 1274012
Comment on attachment 8754075 [details]
Bug 1274046 - Add FailureId to gfxConfig (FeatureState).

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53706/diff/2-3/
Comment on attachment 8754075 [details]
Bug 1274046 - Add FailureId to gfxConfig (FeatureState).

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53706/diff/3-4/
Attachment #8754075 - Attachment description: MozReview Request: Bug 1274046 - Add FailureId to gfxConfig (FeatureState). WIP → MozReview Request: Bug 1274046 - Add FailureId to gfxConfig (FeatureState). r=dvander
Attachment #8754075 - Flags: review?(dvander) → review+
This patch is adding a string to a static type which our leak checker hates:
16 bytes leaked (nsStringBuffer)
Comment on attachment 8754075 [details]
Bug 1274046 - Add FailureId to gfxConfig (FeatureState).

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53706/diff/4-5/
Attachment #8754075 - Attachment description: MozReview Request: Bug 1274046 - Add FailureId to gfxConfig (FeatureState). r=dvander → MozReview Request: Bug 1274046 - Add FailureId to gfxConfig (FeatureState). WIP
Comment on attachment 8754075 [details]
Bug 1274046 - Add FailureId to gfxConfig (FeatureState).

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53706/diff/5-6/
Attachment #8754075 - Attachment description: MozReview Request: Bug 1274046 - Add FailureId to gfxConfig (FeatureState). WIP → MozReview Request: Bug 1274046 - Add FailureId to gfxConfig (FeatureState). r=dvander
Comment on attachment 8754075 [details]
Bug 1274046 - Add FailureId to gfxConfig (FeatureState).

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53706/diff/6-7/
Comment on attachment 8754075 [details]
Bug 1274046 - Add FailureId to gfxConfig (FeatureState).

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53706/diff/7-8/
Flags: needinfo?(bgirard)
Comment on attachment 8754075 [details]
Bug 1274046 - Add FailureId to gfxConfig (FeatureState).

I had to change how gfxConfig was init because otherwise nsCString was reported as being leaked.
Flags: needinfo?(bgirard)
Attachment #8754075 - Flags: review+ → review?(dvander)
Pushed by b56girard@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d9111a0d9a44
Add FailureId to gfxConfig (FeatureState). r=dvander
this landing causes bustage.  But taskcluster has problem now, so treeherder won't be able to get error log.


 0:06.07 gfxWindowsPlatform.cpp
 0:06.07 c:/Development/hg.mozilla.org/mozilla-win64/gfx/thebes/gfxWindowsPlatform.cpp(1942): error C2660: 'mozilla::gfx::FeatureState::DisableByDefault': function does not take 2 arguments
 0:06.07 c:/Development/hg.mozilla.org/mozilla-win64/gfx/thebes/gfxWindowsPlatform.cpp(1949): error C2660: 'IsGfxInfoStatusOkay': function does not take 2 arguments
 0:06.07 c:/Development/hg.mozilla.org/mozilla-win64/gfx/thebes/gfxWindowsPlatform.cpp(1950): error C2660: 'mozilla::gfx::FeatureState::Disable': function does not take 2 arguments
ni? for re-landing reminder
Flags: needinfo?(bgirard)
Comment on attachment 8754075 [details]
Bug 1274046 - Add FailureId to gfxConfig (FeatureState).

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53706/diff/8-9/
Attachment #8754075 - Attachment description: MozReview Request: Bug 1274046 - Add FailureId to gfxConfig (FeatureState). r=dvander → Bug 1274046 - Add FailureId to gfxConfig (FeatureState).
Pushed by b56girard@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f382e0ae3c2b
Add FailureId to gfxConfig (FeatureState). r=dvander
Blocks: 1259536
Flags: needinfo?(bgirard)
https://hg.mozilla.org/mozilla-central/rev/f382e0ae3c2b
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.