Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(385)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 647343003: Fix crash with fullscreen elements inside frames. (Closed)

Created:
6 years, 2 months ago by aelias_OOO_until_Jul13
Modified:
6 years, 2 months ago
Reviewers:
falken, trchen
CC:
blink-reviews, mkwst+moarreviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Fix crash with fullscreen elements inside frames. My patch https://src.chromium.org/viewvc/blink?view=rev&revision=183623 wrongly assumed that the fullscreen element was on the main frame. Use the frame tracked by FullscreenController instead. NOTRY=true BUG=423457 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=183711

Patch Set 1 #

Patch Set 2 : Add tests #

Total comments: 1

Patch Set 3 : Add assertion #

Unified diffs Side-by-side diffs Delta from patch set Stats (+54 lines, -4 lines) Patch
M Source/web/FullscreenController.h View 2 chunks +3 lines, -0 lines 0 comments Download
M Source/web/FullscreenController.cpp View 1 2 1 chunk +10 lines, -0 lines 0 comments Download
M Source/web/WebViewImpl.cpp View 2 chunks +1 line, -3 lines 0 comments Download
M Source/web/tests/WebFrameTest.cpp View 1 2 chunks +35 lines, -1 line 0 comments Download
A Source/web/tests/data/fullscreen_iframe.html View 1 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 6 (2 generated)
aelias_OOO_until_Jul13
Hi falken@ and trchen@, PTAL.
6 years, 2 months ago (2014-10-14 22:34:39 UTC) #2
trchen
lgtm (non-owner) https://codereview.chromium.org/647343003/diff/20001/Source/web/FullscreenController.cpp File Source/web/FullscreenController.cpp (right): https://codereview.chromium.org/647343003/diff/20001/Source/web/FullscreenController.cpp#newcode165 Source/web/FullscreenController.cpp:165: ASSERT(Fullscreen::from(*m_fullScreenFrame->document()).fullScreenRenderer());
6 years, 2 months ago (2014-10-14 23:48:40 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/647343003/40001
6 years, 2 months ago (2014-10-15 00:24:54 UTC) #5
commit-bot: I haz the power
6 years, 2 months ago (2014-10-15 00:32:35 UTC) #6
Message was sent while issue was closed. 
Committed patchset #3 (id:40001) as 183711

Powered by Google App Engine
This is Rietveld 408576698