Closed
Bug 1378281
Opened 7 years ago
Closed 7 years ago
Crash in mozilla::dom::PBrowserChild::SendUpdateContentCache
Categories
(Core :: Widget, defect)
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox54 | --- | unaffected |
firefox55 | --- | unaffected |
firefox56 | --- | fixed |
firefox57 | --- | fixed |
People
(Reporter: ting, Assigned: nika)
References
Details
(Keywords: crash)
Crash Data
Attachments
(2 files)
1011 bytes,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
887 bytes,
patch
|
catalinb
:
review+
gchang
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is report bp-dea5840b-95db-4146-b62e-c48870170704. ============================================================= Top #21 of Nightly 20170702030204 on Windows, bp-dea5840b-95db-4146-b62e-c48870170704 shows PuppetWidget::mTabChild is null.
Comment 1•7 years ago
|
||
Top #16 of Nightly 20170710030203 on Windows. Regression range: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9af23c413a1f8d337b19b4f8450e241e91b71136&tochange=d9a144b7b6d994fc9a497c53b13f51a2a654d85e I guess it's bug 1343728. Masayuki, Michael, it looks like after the change PuppetWidget::mTabChild maybe null when we do FakeShow.
Flags: needinfo?(michael)
Flags: needinfo?(masayuki)
Assignee | ||
Comment 2•7 years ago
|
||
MozReview-Commit-ID: ECTVHnGF8UI
Attachment #8885731 -
Flags: review?(bugs)
Assignee | ||
Comment 3•7 years ago
|
||
If we don't have PuppetWidget::mTabChild anymore, I think that means that we've torn down our IPC actor. So I think we can just check newChild->IPCOpen() and abort if it fails.
Flags: needinfo?(michael)
Updated•7 years ago
|
Attachment #8885731 -
Flags: review?(bugs) → review+
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → michael
Pushed by michael@thelayzells.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/703825995ef4 Check IPCOpen and abort if the TabChild has been destroyed, r=smaug
Updated•7 years ago
|
Flags: needinfo?(masayuki)
Comment 5•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/703825995ef4
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Updated•7 years ago
|
status-firefox54:
--- → unaffected
status-firefox55:
--- → unaffected
status-firefox-esr52:
--- → unaffected
Comment 6•7 years ago
|
||
the signature is still present in current 56.0b builds - https://crash-stats.mozilla.com/signature/?product=Firefox&signature=mozilla%3A%3Adom%3A%3APBrowserChild%3A%3ASendUpdateContentCache should we reopen this bug or file a new one about the continuing crash?
Flags: needinfo?(michael)
Assignee | ||
Comment 7•7 years ago
|
||
It looks to me like I was not doing a good enough check in my code. Namely, I was checking mIPCOpen, which is cleared in ActorDestroy, but we actually also clean up the PuppetWidget (thus breaking the backlink and allowing this null dereference to occur) in TabChild::RecvDestroy. That method sets mDestroyed when it is called. I figure checking both mIPCOpen and mDestroyed should probably do it.
Status: RESOLVED → REOPENED
Flags: needinfo?(michael)
Resolution: FIXED → ---
Assignee | ||
Comment 8•7 years ago
|
||
Attachment #8903651 -
Flags: review?(catalin.badea392)
Updated•7 years ago
|
Attachment #8903651 -
Flags: review?(catalin.badea392) → review+
Pushed by michael@thelayzells.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/dbda696c8bcb Take 2: Also check if the child is destroyed during ProvideWindowCommon, r=catalinb
Comment 10•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/dbda696c8bcb
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Assignee | ||
Comment 12•7 years ago
|
||
Comment on attachment 8903651 [details] [diff] [review] Take 2: Also check if the child is destroyed during ProvideWindowCommon Approval Request Comment [Feature/Bug causing the regression]: bug 1343728 [User impact if declined]: occasional crashes [Is this code covered by automated tests?]: no [Has the fix been verified in Nightly?]: yes [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: adding an extra check before dereferencing a pointer to make sure it isn't invalid. [String changes made/needed]: None
Flags: needinfo?(michael)
Attachment #8903651 -
Flags: approval-mozilla-beta?
Comment 13•7 years ago
|
||
Comment on attachment 8903651 [details] [diff] [review] Take 2: Also check if the child is destroyed during ProvideWindowCommon Fix a crash. Beta56+.
Attachment #8903651 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 14•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/73592705a693
You need to log in
before you can comment on or make changes to this bug.
Description
•