Closed
Bug 1399453
Opened 7 years ago
Closed 7 years ago
Annotate crash reports with compositor thread ownership in parent process
Categories
(Core :: Graphics, defect, P1)
Core
Graphics
Tracking
()
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox56 | --- | wontfix |
firefox57 | --- | fix-optional |
firefox58 | --- | fixed |
People
(Reporter: aosmond, Assigned: aosmond)
References
Details
(Whiteboard: [gfx-noted])
Attachments
(1 file, 2 obsolete files)
11.33 KB,
patch
|
aosmond
:
review+
|
Details | Diff | Splinter Review |
See bug 1389021. Annotate the crash reports with compositor thread owners to find out who is keeping the thread alive.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → aosmond
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•7 years ago
|
||
Assignee | ||
Comment 2•7 years ago
|
||
Comment on attachment 8907560 [details] [diff] [review] 0001-Bug-XXX-Annotate-crash-reports-with-compositor-threa.patch try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e67515f0fb0de9f159e99bc17ea8fbe9e413110b Plan is to land it for a build or two, and then back out.
Attachment #8907560 -
Flags: review?(dvander)
Comment on attachment 8907560 [details] [diff] [review] 0001-Bug-XXX-Annotate-crash-reports-with-compositor-threa.patch Review of attachment 8907560 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/ipc/CompositorThread.cpp @@ +43,5 @@ > +CompositorThreadHolderDebug::~CompositorThreadHolderDebug() > +{ > +#ifdef MOZ_CRASHREPORTER > + if (XRE_IsParentProcess()) { > + CrashReporter::RemoveCrashReportAnnotation(mId); You might want to have this change the annotation to "0" instead, that way we know *something* happened.
Attachment #8907560 -
Flags: review?(dvander) → review+
Assignee | ||
Comment 4•7 years ago
|
||
try that works: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7d2103975f9babf15929220bdfa2c0baa2c2f2f7 (In reply to David Anderson [:dvander] from comment #3) > Comment on attachment 8907560 [details] [diff] [review] > 0001-Bug-XXX-Annotate-crash-reports-with-compositor-threa.patch > > Review of attachment 8907560 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: gfx/layers/ipc/CompositorThread.cpp > @@ +43,5 @@ > > +CompositorThreadHolderDebug::~CompositorThreadHolderDebug() > > +{ > > +#ifdef MOZ_CRASHREPORTER > > + if (XRE_IsParentProcess()) { > > + CrashReporter::RemoveCrashReportAnnotation(mId); > > You might want to have this change the annotation to "0" instead, that way > we know *something* happened. Good point. I was concerned about adding a lot of entries, but we shouldn't create too many content processes, so the number should be limited. I updated the patch to do this.
Attachment #8907560 -
Attachment is obsolete: true
Attachment #8907603 -
Flags: review+
Assignee | ||
Comment 5•7 years ago
|
||
Fix missing explicit keyword (dammit, this really should fail in my local build...) -- https://treeherder.mozilla.org/#/jobs?repo=try&revision=f25aef4e2ea9f3d2b7117a63fa7aa0293581c74c
Updated•7 years ago
|
Pushed by aosmond@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/727f0d09e299 Temporarily annotate crash reports with compositor thread owners. r=dvander
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/727f0d09e299
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Backout by aosmond@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/1796dd9bccfb Backed out changeset 727f0d09e299 because the annotations are no longer needed.
Assignee | ||
Comment 9•7 years ago
|
||
Reopening as I want to reland this to help diagnose bug 1408532.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Updated•7 years ago
|
status-firefox57:
fixed → ---
Target Milestone: mozilla57 → mozilla58
Assignee | ||
Updated•7 years ago
|
Attachment #8907603 -
Attachment is obsolete: true
Comment 11•7 years ago
|
||
Pushed by aosmond@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/dc9d9c01080a Temporarily annotate crash reports with compositor thread owners. r=dvander
Updated•7 years ago
|
status-firefox56:
--- → wontfix
status-firefox57:
--- → fix-optional
status-firefox58:
--- → fix-optional
Comment 12•7 years ago
|
||
Backed out for static bustage at layers/CompositorThread.h:68: bad implicit conversion constructor for 'CompositorThreadHolderDebug': https://hg.mozilla.org/integration/mozilla-inbound/rev/91f976f5e4c5b59db6a8b1eb6bc39c3ec992774d Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=dc9d9c01080a3a9d36af97706a0ec612663c48b0&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry Build log: https://treeherder.mozilla.org/logviewer.html#?job_id=138925228&repo=mozilla-inbound [task 2017-10-23T16:02:19.453Z] 16:02:19 INFO - In file included from /builds/worker/workspace/build/src/gfx/ipc/GPUParent.cpp:28: [task 2017-10-23T16:02:19.453Z] 16:02:19 INFO - /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/layers/CompositorThread.h:68:3: error: bad implicit conversion constructor for 'CompositorThreadHolderDebug' [task 2017-10-23T16:02:19.454Z] 16:02:19 INFO - CompositorThreadHolderDebug(const char* aName); [task 2017-10-23T16:02:19.455Z] 16:02:19 INFO - ^
Flags: needinfo?(aosmond)
Comment 13•7 years ago
|
||
Pushed by aosmond@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d84ea38d6fd0 Temporarily annotate crash reports with compositor thread owners. r=dvander
Comment 14•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d84ea38d6fd0
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(aosmond)
Comment 15•7 years ago
|
||
Backout by aosmond@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/7a22351172c8 Backed out changeset d84ea38d6fd0 because the annotations are no longer needed.
Comment 16•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7a22351172c8 - landed 15 hours ago
You need to log in
before you can comment on or make changes to this bug.
Description
•