Closed Bug 1320765 Opened 3 years ago Closed 3 years ago

Crash in nsFrameLoader::SwapWithOtherRemoteLoader

Categories

(Core :: DOM: Core & HTML, defect, P1, critical)

52 Branch
x86
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox50 --- wontfix
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: marcia, Assigned: jryans)

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-ecf8f445-718d-43ab-92d3-fc0e32161128.
=============================================================

Seen while looking at Aurora crash stats: http://bit.ly/2gARqoj. This crash affects FF 52 and 50 only according to the summary.

ni on [:jryans] to see if he has some ideas
Flags: needinfo?(jryans)
If the "line" property is right, then the crashing line for this is:
   aOther->mRemoteBrowser->IsIsolatedMozBrowserElement()

If aOther was null, then I believe that the function would crash earlier (as it is used earlier), which makes me think that aOther->mRemoteBrowser is null.

It also looks like in the location where mRemoteBrowser is set, we do try to handle the mRemoteBrowser is null when in a remote frame case. This makes me think that SwapWithOtherRemoteLoader should be updated to gracefully fail in that situation.
:mystor's theory seems reasonable to me, I'll try adding a guard for this.  I am little unsure what's caused this to show up just now, as `aOther->mRemoteBrowser` is referenced in lots of long standing parts of the swap path.  Perhaps new RDM (which has been enabled in 52) is sending more people down this path during some kind of race...?

Anyway, let's try a guard.
Assignee: nobody → jryans
Status: NEW → ASSIGNED
Flags: needinfo?(jryans)
Given only a single crash in 50, I would assume we won't make a change there.
Comment on attachment 8815431 [details]
Bug 1320765 - Test mRemoteBrowser to avoid crash during swapping frame loaders.

https://reviewboard.mozilla.org/r/96330/#review96558
Attachment #8815431 - Flags: review?(michael) → review+
Pushed by jryans@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/12db8198fa96
Test mRemoteBrowser to avoid crash during swapping frame loaders. r=mystor
https://hg.mozilla.org/mozilla-central/rev/12db8198fa96
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Comment on attachment 8815431 [details]
Bug 1320765 - Test mRemoteBrowser to avoid crash during swapping frame loaders.

Approval Request Comment
[Feature/Bug causing the regression]: Cause is not clear yet, seems related to Responsive Design Mode which was enabled in 52
[User impact if declined]: If declined, Firefox will likely continue to crash for this set of users
[Is this code covered by automated tests?]: No tests for this specific change, but others cover RDM more generally
[Has the fix been verified in Nightly?]: It was tested locally and landed in m-c
[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?]: Adds a guard to avoid a crash
[String changes made/needed]: None
Attachment #8815431 - Flags: approval-mozilla-aurora?
Comment on attachment 8815431 [details]
Bug 1320765 - Test mRemoteBrowser to avoid crash during swapping frame loaders.

crash fix, for aurora52
Attachment #8815431 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.