Changelog includes only one CL related to fullscreen: https://chromium.googlesource.com/chromium/src/+/81e33d3b1082212c0cc3732719167004dcfb63fc

Likely a regression from that CL.

In a debug build, this manifests as a failing DCHECK(GetDocument().IsActive()) in StyleEngine::UpdateActiveStyle() and can be reproduced by putting the attached file in LayoutTest/fullscreen/ and running as any LayoutTest:

[18678:18715:0802/165013.474543:194565524468:FATAL:StyleEngine.cpp(389)] Check failed: GetDocument().IsActive(). 
#0 0x7fb5b3bf78cd base::debug::StackTrace::StackTrace()
#1 0x7fb5b3bf5c9c base::debug::StackTrace::StackTrace()
#2 0x7fb5b3c863ba logging::LogMessage::~LogMessage()
#3 0x7fb5ab9f78ae blink::StyleEngine::UpdateActiveStyle()
#4 0x7fb5ab9fd421 blink::StyleEngine::EnsureUAStyleForFullscreen()
#5 0x7fb5ab8ef927 blink::Element::SetContainsFullScreenElement()
#6 0x7fb5abdfaeb0 blink::Fullscreen::FullscreenElementChanged()
#7 0x7fb5abdfa586 blink::Fullscreen::PopFullscreenElementStack()
#8 0x7fb5abdfa225 blink::Fullscreen::ContinueExitFullscreen()
#9 0x7fb5abdfa292 blink::Fullscreen::DidExitFullscreen()
#10 0x7fb5abcd4a29 blink::FullscreenController::DidExitFullscreen()
#11 0x7fb5abc885f4 blink::WebViewImpl::DidExitFullscreen()
#12 0x7fb5abdbf3c2 blink::WebViewFrameWidget::DidExitFullscreen()
#13 0x7fb5b6f004fb content::RenderWidget::DidToggleFullscreen()
#14 0x7fb5b6f00367 content::RenderWidget::Resize()
#15 0x7fb5b6efc0ef content::RenderWidget::OnResize()
#16 0x7fb5b6ee8a84 content::RenderViewImpl::OnResize()

The adoptNode() call in the test causes the fullscreen element to be removed from the original document and get an inactive document as its node document, even though it isn't inserted into that document.

Fullscreen::ElementRemoved() calls ExitFullscreen() but leaves the element on the fullscreen element stack in the interim, so in Fullscreen::PopFullscreenElementStack() the old_element's document isn't the same as as the document that has the fullscreen supplement. The IsActive checks sprinkled about are for the wrong document in this case.

These facts aren't new, but most likely a timing change has made this possible to hit now where it wasn't before.

Contemplating fix.

Deleted: fuzz-http-45.html 782 bytes

fuzz-http-45.html 782 bytes View Download

Review at https://chromium-review.googlesource.com/c/602272

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a67e690ff85360414ce2db240ee5969b19595f67

commit a67e690ff85360414ce2db240ee5969b19595f67
Author: Philip Jägenstedt <foolip@chromium.org>
Date: Mon Aug 07 15:49:36 2017

Avoid calling EnsureUAStyleForFullscreen() when exiting fullscreen

Because the document may be inactive, and when it isn't the UA style
sheet is not needed any longer, and should already be loaded.

The added test triggered the same DCHECK as discussed in the bug.

Bug:  747827 
Change-Id: I36e862ff3a8086315d8be47169b5d5a2a9033a0f
Reviewed-on: https://chromium-review.googlesource.com/602272
Reviewed-by: Rune Lillesveen <rune@opera.com>
Commit-Queue: Philip Jägenstedt <foolip@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492328}
[add] https://crrev.com/a67e690ff85360414ce2db240ee5969b19595f67/third_party/WebKit/LayoutTests/external/wpt/fullscreen/model/move-to-inactive-document-manual.html
[modify] https://crrev.com/a67e690ff85360414ce2db240ee5969b19595f67/third_party/WebKit/Source/core/dom/Element.cpp

ClusterFuzz has detected this issue as fixed in range 492325:492338.

Detailed report: https://clusterfuzz.com/testcase?key=6488681547563008

Fuzzer: inferno_twister
Job Type: mac_asan_content_shell
Platform Id: mac

Crash Type: Null-dereference READ
Crash Address: 0x0000000000e8
Crash State:
  blink::StyleEngine::EnsureUAStyleForFullscreen
  blink::Element::SetContainsFullScreenElement
  blink::Fullscreen::FullscreenElementChanged
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=mac_asan_content_shell&range=480824:480870
Fixed: https://clusterfuzz.com/revisions?job=mac_asan_content_shell&range=492325:492338

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6488681547563008


Additional requirements: Requires HTTP

See https://github.com/google/clusterfuzz-tools for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

ClusterFuzz testcase 6488681547563008 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

This was caused by https://chromium.googlesource.com/chromium/src/+/81e33d3b1082212c0cc3732719167004dcfb63fc which was in M61, so requesting merge. ClusterFuzz verified this as fixed in #7.

Removing Merge-Request-61 label. This was in 62.0.3179.0, waiting until that's reached canary.

This has now reached canary, requesting merge.

This bug requires manual review: M61 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Before we approve merge to M61, please answer followings:
* Is this M61 regression? Is it critical?
* Per comment #11 change landed in Canary. Is the change well baked/verified in Canary, having enough automation tests coverage and safe to merge to M61?
* Any other important details to justify the merge.

Please note M61 is already in Beta, so merge bar is very high. Thank you.

Yes, this is an M61 regression. The scripts that produce this crash are somewhat far-fetched and I don't think it's likely to be triggered in the wild by accident. The crash is a null (+offset) deref and not a security concern, but would allow sites to crash themselves after entering fullscreen.

On the other side of scale, the fix is simple and tested, and I believe very unlikely to itself be a cause of further crashes.

Approving merge to M61 branch 3163 based on comment #11 and #14. Pls merge ASAP. Thank you.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/49e328ed9dd3eaad87490b009bde71fe41cfb7dd

commit 49e328ed9dd3eaad87490b009bde71fe41cfb7dd
Author: Philip Jägenstedt <foolip@chromium.org>
Date: Fri Aug 11 09:16:12 2017

Avoid calling EnsureUAStyleForFullscreen() when exiting fullscreen

Because the document may be inactive, and when it isn't the UA style
sheet is not needed any longer, and should already be loaded.

The added test triggered the same DCHECK as discussed in the bug.

TBR=foolip@chromium.org

(cherry picked from commit a67e690ff85360414ce2db240ee5969b19595f67)

Bug:  747827 
Change-Id: I36e862ff3a8086315d8be47169b5d5a2a9033a0f
Reviewed-on: https://chromium-review.googlesource.com/602272
Reviewed-by: Rune Lillesveen <rune@opera.com>
Commit-Queue: Philip Jägenstedt <foolip@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#492328}
Reviewed-on: https://chromium-review.googlesource.com/611985
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#486}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[add] https://crrev.com/49e328ed9dd3eaad87490b009bde71fe41cfb7dd/third_party/WebKit/LayoutTests/external/wpt/fullscreen/model/move-to-inactive-document-manual.html
[modify] https://crrev.com/49e328ed9dd3eaad87490b009bde71fe41cfb7dd/third_party/WebKit/Source/core/dom/Element.cpp