Closed Bug 1401840 Opened 6 years ago Closed 6 years ago

Crash in nsFrameManager::CaptureFrameStateFor when opening Presenter View in Google Slides

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Version:
57 Branch
Platform:
Unspecified
macOS
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla57
Tracking Flags:
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- unaffected
firefox57 + fixed
firefox58 --- fixed

People

(Reporter: timdream, Assigned: bzbarsky)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression, Whiteboard: [clouseau])

Crash Data

Attachments

(2 files)

Minimal-ish testcase (will crash!)
180 bytes, text/html
Details
Make sure to not end up with infinite recursion when reframing a <body> that has a table pseudo parent coming from the root <html>
3.32 KB, patch
dholbert
: review+
Sylvestre
: approval-mozilla-beta+
Details | Diff | Splinter Review 
This bug was filed from the Socorro interface and is 
report bp-2a898450-ac50-4b21-8849-69e640170921.
=============================================================

STR:

1. Go to a Google Slides
2. Hit the little arrow besides the [Present] button on the top right.
3. Hit [Presenter view]

Expected:

1. Two popups opened and they work.

Actual:

1. Two popups opened and they all crash.
For some reasons it only happens on my MBP ... how do I debug further?
I was able to reproduce this issue too. All the tabs related to the presentation crash.

Tested with:
Browser / Version: Firefox Mobile Nightly 57.0a1 (2017-09-20)
Operating System: Windows 10 Pro
There are 48 crashes in nightly 57 starting with buildid 20170919220202. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1400599.
:bz, could you investigate please ?

[1] https://hg.mozilla.org/mozilla-central/rev?node=13f651129c38f79e0b7efa67892dcb296e41fe1a
Blocks: 1400599, clouseau
status-firefox55: --- → unaffected
status-firefox56: --- → unaffected
status-firefox57: --- → affected
status-firefox-esr52: --- → unaffected
Flags: needinfo?(bzbarsky)
Keywords: regression
Whiteboard: [clouseau]
Component: General → DOM
Product: Firefox → Core
Target Milestone: --- → mozilla57
Version: unspecified → 57 Branch
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #1)
> For some reasons it only happens on my MBP ... how do I debug further?
You are not alone.. I hit this crash quite often in these two days.
The testcase is:

  <style>
    html { display: table; }
    body { overflow: scroll; }
  </style>
  <script>
    onload = function() {
      document.body.style.display = 'inline';
    }
  </script>

What happens is that we go to reframe the <body>.  That calls ContentRemoved on the body, which calls MaybeRecreateContainerForFrameRemoval.  That detects that the body has a table pseudo parent and goes to recreate the <html>.  That computes the "new" scrollbar override element, which is just the <body> (because it's not gone from the DOM or anything).  Since the thing being removed is not the <body> itself, we RecreateFramesForContent(newOverrideElement) and now we're doing infinite recursion and eventually we run out of stack.

Fix coming up.  This is definitely a regression from bug 1400599.
Flags: needinfo?(bzbarsky)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Calixte, thanks for pinning down the problem!
Comment on attachment 8911385 [details] [diff] [review]
Make sure to not end up with infinite recursion when reframing a <body> that has a table pseudo parent coming from the root <html>

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1400599
[User impact if declined]: Crashes on trying to present Google slides.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Locally, yes...
[Needs manual test from QE? If yes, steps to reproduce]: Probably not, but
    the steps are in comment 0 anyway.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]:  I think at this point we've caught all
   the edge cases in this stuff...
[String changes made/needed]: None.
Attachment #8911385 - Flags: approval-mozilla-beta?
Comment on attachment 8911385 [details] [diff] [review]
Make sure to not end up with infinite recursion when reframing a <body> that has a table pseudo parent coming from the root <html>

Review of attachment 8911385 [details] [diff] [review]:
-----------------------------------------------------------------

r=me
Attachment #8911385 - Flags: review?(dholbert) → review+
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d26a0adf4c2e
Make sure to not end up with infinite recursion when reframing a <body> that has a table pseudo parent coming from the root <html>.  r=dholbert
https://hg.mozilla.org/mozilla-central/rev/d26a0adf4c2e
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Comment on attachment 8911385 [details] [diff] [review]
Make sure to not end up with infinite recursion when reframing a <body> that has a table pseudo parent coming from the root <html>

Fix a crash, taking it. Should be in 57b3
Attachment #8911385 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.