Fullscreen: make fullscreen requests come from RenderFrame

content/browser/web_contents/web_contents_impl.cc

Index: content/browser/web_contents/web_contents_impl.cc
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 9614814d1a2232c9a4de57e2ea6bc8ddf9385c15..4cb48cd5981198b54f51087d1520e8b171af8b1e 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -1289,7 +1289,7 @@ void WebContentsImpl::RenderWidgetDeleted(
   if (render_widget_host &&
       render_widget_host->GetRoutingID() == fullscreen_widget_routing_id_) {
     if (delegate_ && delegate_->EmbedsFullscreenWidget())
-      delegate_->ToggleFullscreenModeForTab(this, false);
+      delegate_->ToggleFullscreenModeForTab(this, GURL(), false);
     FOR_EACH_OBSERVER(WebContentsObserver,
                       observers_,
                       DidDestroyFullscreenWidget(
@@ -1417,7 +1417,19 @@ void WebContentsImpl::HandleGestureEnd() {
     delegate_->HandleGestureEnd();
 }
 
-void WebContentsImpl::ToggleFullscreenMode(bool enter_fullscreen) {
+void WebContentsImpl::EnterFullscreenMode(const GURL& origin) {
+  ToggleFullscreenMode(origin, true);
+}
+
+void WebContentsImpl::ExitFullscreenMode() {
+  ToggleFullscreenMode(GURL(), false);
+}
+
+void WebContentsImpl::ToggleFullscreenMode(const GURL& origin,
+                                           bool enter_fullscreen) {
+  DCHECK(!enter_fullscreen || origin.is_valid());
+  DCHECK(enter_fullscreen || origin.is_empty());

[dcheng, 2014/12/10 06:58:00]

I'm not fond of this pattern. It seems that things would read much more clearly
as:

r->EnterFullscreenMode(origin);

and

r->ExitFullscreen();

Clearly this isn't actually a toggling function, since you have to specify
whether you're entering or exiting fullscreen mode anyway. I realize that this
pattern is older than this patch. However, IMO, all the new calls to
ToggleFullscreen(..., GURL(), false) are a lot less readable, so it would be
nice to fix it.

If having one entry point for this really is that much simpler, then it should
be called SetFullscreen() or something, not ToggleFullscreen().

[mlamouri (slow - plz ping), 2014/12/10 15:47:02]

I've change WebContentsDelegate to have EnterFullscreenModeForTab and
ExitFullscreenModeForTab. In most places I redirected to the previous
ToggleFullscreenModeForTab method but it should be better.

+
   // This method is being called to enter or leave renderer-initiated fullscreen
   // mode.  Either way, make sure any existing fullscreen widget is shut down
   // first.
@@ -1426,7 +1438,7 @@ void WebContentsImpl::ToggleFullscreenMode(bool enter_fullscreen) {
     RenderWidgetHostImpl::From(widget_view->GetRenderWidgetHost())->Shutdown();
 
   if (delegate_)
-    delegate_->ToggleFullscreenModeForTab(this, enter_fullscreen);
+    delegate_->ToggleFullscreenModeForTab(this, origin, enter_fullscreen);
 
   FOR_EACH_OBSERVER(WebContentsObserver,
                     observers_,
@@ -1689,7 +1701,7 @@ void WebContentsImpl::ShowCreatedWidget(int route_id,
     fullscreen_widget_routing_id_ = route_id;
     if (delegate_ && delegate_->EmbedsFullscreenWidget()) {
       widget_host_view->InitAsChild(GetRenderWidgetHostView()->GetNativeView());
-      delegate_->ToggleFullscreenModeForTab(this, true);
+      delegate_->ToggleFullscreenModeForTab(this, GURL(), true);
     } else {
       widget_host_view->InitAsFullscreen(view);
     }
@@ -3584,7 +3596,7 @@ void WebContentsImpl::RenderViewTerminated(RenderViewHost* rvh,
   // Ensure fullscreen mode is exited in the |delegate_| since a crashed
   // renderer may not have made a clean exit.
   if (IsFullscreenForCurrentTab())
-    ToggleFullscreenMode(false);
+    ExitFullscreenMode();
 
   // Cancel any visible dialogs so they are not left dangling over the sad tab.
   if (dialog_manager_)