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

Issue 277823002: Don't show video fullscreen until DidEnterFullscreen() is called (Closed)

Created:
6 years, 7 months ago by qinmin
Modified:
6 years, 7 months ago
CC:
blink-reviews, jamesr, krit, philipj_slow, jbroman, eric.carlson_apple.com, danakj, feature-media-reviews_chromium.org, Rik, Stephen Chennney, pdr., rwlbuis
Visibility:
Public.

Description

Don't show video fullscreen until DidEnterFullscreen() is called For fullscreen video on android, We call showFullscreenOverlay() too early. We should wait until DidEnterFullscreen() gets called, and then we should let the video element to request the surface view. Otherwise, if enterFullscreenForElement fails, we will incorrectly showing the fullscreen video in the background. This CL also removes the hideFullscreenOverlay() call since this is no longer needed. When exiting fullscreen, video view will automatically be destroyed. BUG=367346 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=173917

Patch Set 1 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -23 lines) Patch
M Source/web/FullscreenController.cpp View 3 chunks +11 lines, -23 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
qinmin
PTAL
6 years, 7 months ago (2014-05-09 01:49:23 UTC) #1
aelias_OOO_until_Jul13
lgtm. I don't understand this very well but but trchen tells me it's legit.
6 years, 7 months ago (2014-05-09 03:07:45 UTC) #2
qinmin
The CQ bit was checked by qinmin@chromium.org
6 years, 7 months ago (2014-05-12 21:12:53 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/qinmin@chromium.org/277823002/20001
6 years, 7 months ago (2014-05-12 21:13:21 UTC) #4
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). Please consider checking whether the failures are ...
6 years, 7 months ago (2014-05-12 22:25:28 UTC) #5
commit-bot: I haz the power
Change committed as 173917
6 years, 7 months ago (2014-05-12 23:00:47 UTC) #6
qinmin
6 years, 7 months ago (2014-05-13 17:45:42 UTC) #7
Message was sent while issue was closed.
A revert of this CL has been created in
https://codereview.chromium.org/284913002/ by qinmin@chromium.org.

The reason for reverting is: this breaks webview. DidEnterFullscreen will never
be received since webview doesn't support fullscreen api. As a result, video
won't enter fullscreen..

Powered by Google App Engine
This is Rietveld 408576698