weblayer: strengthen fullscreen handling

Specifically adds the following:

1. fullscreen for background tabs is processed when the tab is made
active. This matches what clank does.
2. Ensure the callback supplied to the delegate to exit fullscreen is
only applicable to the request it was created for. For example, if the
the delegate is supplied callback C1, fullscreen exits, then fullscreen
enters again with callback C2, callback C1 is no longer applicable.
3. Make the java side have similar logic to (2).

BUG=1142101
TEST=none

Change-Id: Ic28d6cbc761ff8b0f9651c018c258d569b9c556b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2628086Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843644}

weblayer/browser/browser_impl.cc

@@ -272,7 +272,7 @@ void BrowserImpl::SetActiveTab(Tab* tab) {
   for (BrowserObserver& obs : browser_observers_)
     obs.OnActiveTabChanged(active_tab_);
   if (active_tab_)
-    active_tab_->web_contents()->GetController().LoadIfNecessary();
+    active_tab_->OnGainedActive();
 }
 
 Tab* BrowserImpl::GetActiveTab() {

weblayer/browser/fullscreen_browsertest.cc

+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "weblayer/test/weblayer_browser_test.h"
+
+#include "base/callback.h"
+#include "weblayer/browser/tab_impl.h"
+#include "weblayer/public/browser.h"
+#include "weblayer/public/fullscreen_delegate.h"
+#include "weblayer/shell/browser/shell.h"
+#include "weblayer/test/weblayer_browser_test_utils.h"
+
+namespace weblayer {
+
+using FullscreenBrowserTest = WebLayerBrowserTest;
+
+class FullscreenDelegateImpl : public FullscreenDelegate {
+ public:
+  bool got_enter() const { return got_enter_; }
+  bool got_exit() const { return got_exit_; }
+
+  void ResetState() { got_enter_ = got_exit_ = false; }
+
+  // FullscreenDelegate:
+  void EnterFullscreen(base::OnceClosure exit_closure) override {
+    got_enter_ = true;
+  }
+  void ExitFullscreen() override { got_exit_ = true; }
+
+ private:
+  bool got_enter_ = false;
+  bool got_exit_ = false;
+};
+
+IN_PROC_BROWSER_TEST_F(FullscreenBrowserTest, EnterFromBackgroundTab) {
+  EXPECT_TRUE(embedded_test_server()->Start());
+
+  TabImpl* tab1 = static_cast<TabImpl*>(shell()->tab());
+  Browser* browser = tab1->GetBrowser();
+  TabImpl* tab2 = static_cast<TabImpl*>(browser->CreateTab());
+  EXPECT_NE(tab2, browser->GetActiveTab());
+  FullscreenDelegateImpl fullscreen_delegate;
+  tab2->SetFullscreenDelegate(&fullscreen_delegate);
+
+  // As `tab2` is in the background, the delegate should not be notified.
+  static_cast<content::WebContentsDelegate*>(tab2)->EnterFullscreenModeForTab(
+      nullptr, blink::mojom::FullscreenOptions());
+  EXPECT_TRUE(static_cast<content::WebContentsDelegate*>(tab2)
+                  ->IsFullscreenForTabOrPending(nullptr));
+  EXPECT_FALSE(fullscreen_delegate.got_enter());
+
+  // Making the tab active should trigger the going fullscreen.
+  browser->SetActiveTab(tab2);
+  EXPECT_TRUE(static_cast<content::WebContentsDelegate*>(tab2)
+                  ->IsFullscreenForTabOrPending(nullptr));
+  EXPECT_TRUE(fullscreen_delegate.got_enter());
+
+  tab2->SetFullscreenDelegate(nullptr);
+}
+
+IN_PROC_BROWSER_TEST_F(FullscreenBrowserTest, NoExitForBackgroundTab) {
+  EXPECT_TRUE(embedded_test_server()->Start());
+
+  Browser* browser = shell()->tab()->GetBrowser();
+  TabImpl* tab = static_cast<TabImpl*>(browser->CreateTab());
+  EXPECT_NE(tab, browser->GetActiveTab());
+  FullscreenDelegateImpl fullscreen_delegate;
+  tab->SetFullscreenDelegate(&fullscreen_delegate);
+
+  // As `tab` is in the background, the delegate should not be notified.
+  static_cast<content::WebContentsDelegate*>(tab)->EnterFullscreenModeForTab(
+      nullptr, blink::mojom::FullscreenOptions());
+  EXPECT_TRUE(static_cast<content::WebContentsDelegate*>(tab)
+                  ->IsFullscreenForTabOrPending(nullptr));
+  EXPECT_FALSE(fullscreen_delegate.got_enter());
+  EXPECT_TRUE(static_cast<content::WebContentsDelegate*>(tab)
+                  ->IsFullscreenForTabOrPending(nullptr));
+  EXPECT_FALSE(fullscreen_delegate.got_enter());
+  EXPECT_FALSE(fullscreen_delegate.got_exit());
+  fullscreen_delegate.ResetState();
+
+  // Simulate exiting. As the delegate wasn't told about the enter, it should
+  // not be told about the exit.
+  static_cast<content::WebContentsDelegate*>(tab)->ExitFullscreenModeForTab(
+      nullptr);
+  EXPECT_FALSE(static_cast<content::WebContentsDelegate*>(tab)
+                   ->IsFullscreenForTabOrPending(nullptr));
+  EXPECT_FALSE(fullscreen_delegate.got_enter());
+  EXPECT_FALSE(fullscreen_delegate.got_exit());
+
+  tab->SetFullscreenDelegate(nullptr);
+}
+
+}  // namespace weblayer

weblayer/browser/java/org/chromium/weblayer_private/FullscreenCallbackProxy.java

@@ -28,6 +28,8 @@ public final class FullscreenCallbackProxy {
     private TabImpl mTab;
     private FullscreenToast mToast;
     private boolean mIsNotifyingClientOfEnter;
+    // Used so that only the most recent callback supplied to the client is acted on.
+    private int mNextFullscreenId;
 
     FullscreenCallbackProxy(TabImpl tab, long nativeTab, IFullscreenCallbackClient client) {
         assert client != null;
@@ -63,10 +65,15 @@ public final class FullscreenCallbackProxy {
 
     @CalledByNative
     private void enterFullscreen() throws RemoteException {
+        final int id = ++mNextFullscreenId;
         ValueCallback<Void> exitFullscreenCallback = new ValueCallback<Void>() {
             @Override
             public void onReceiveValue(Void result) {
                 ThreadUtils.assertOnUiThread();
+                if (id != mNextFullscreenId) {
+                    // This is an old fullscreen request. Ignore it.
+                    return;
+                }
                 if (mNativeFullscreenCallbackProxy == 0) {
                     throw new IllegalStateException("Called after destroy()");
                 }

weblayer/browser/tab_impl.cc

@@ -555,6 +555,12 @@ void TabImpl::SetWebPreferences(blink::web_pref::WebPreferences* prefs) {
   browser_->SetWebPreferences(prefs);
 }
 
+void TabImpl::OnGainedActive() {
+  web_contents()->GetController().LoadIfNecessary();
+  if (enter_fullscreen_on_gained_active_)
+    EnterFullscreenImpl();
+}
+
 void TabImpl::OnLosingActive() {
   if (is_fullscreen_)
     web_contents_->ExitFullscreen(/* will_cause_resize */ false);
@@ -1087,20 +1093,21 @@ void TabImpl::EnterFullscreenModeForTab(
     const blink::mojom::FullscreenOptions& options) {
   // TODO: support |options|.
   is_fullscreen_ = true;
-  auto exit_fullscreen_closure = base::BindOnce(&TabImpl::OnExitFullscreen,
-                                                weak_ptr_factory_.GetWeakPtr());
-  base::AutoReset<bool> reset(&processing_enter_fullscreen_, true);
-  fullscreen_delegate_->EnterFullscreen(std::move(exit_fullscreen_closure));
-#if defined(OS_ANDROID)
-  // Make sure browser controls cannot show when the tab is fullscreen.
-  SetBrowserControlsConstraint(ControlsVisibilityReason::kFullscreen,
-                               cc::BrowserControlsState::kHidden);
-#endif
+  if (!IsActive()) {
+    // Process the request the tab is made active.
+    enter_fullscreen_on_gained_active_ = true;
+    return;
+  }
+  EnterFullscreenImpl();
 }
 
 void TabImpl::ExitFullscreenModeForTab(content::WebContents* web_contents) {
+  weak_ptr_factory_for_fullscreen_exit_.InvalidateWeakPtrs();
   is_fullscreen_ = false;
-  fullscreen_delegate_->ExitFullscreen();
+  if (enter_fullscreen_on_gained_active_)
+    enter_fullscreen_on_gained_active_ = false;
+  else
+    fullscreen_delegate_->ExitFullscreen();
 #if defined(OS_ANDROID)
   // Attempt to show browser controls when exiting fullscreen.
   SetBrowserControlsConstraint(ControlsVisibilityReason::kFullscreen,
@@ -1369,4 +1376,20 @@ bool TabImpl::SetDataInternal(const std::map<std::string, std::string>& data) {
   return true;
 }
 
+void TabImpl::EnterFullscreenImpl() {
+  // This ensures the existing callback is ignored.
+  weak_ptr_factory_for_fullscreen_exit_.InvalidateWeakPtrs();
+
+  auto exit_fullscreen_closure =
+      base::BindOnce(&TabImpl::OnExitFullscreen,
+                     weak_ptr_factory_for_fullscreen_exit_.GetWeakPtr());
+  base::AutoReset<bool> reset(&processing_enter_fullscreen_, true);
+  fullscreen_delegate_->EnterFullscreen(std::move(exit_fullscreen_closure));
+#if defined(OS_ANDROID)
+  // Make sure browser controls cannot show when the tab is fullscreen.
+  SetBrowserControlsConstraint(ControlsVisibilityReason::kFullscreen,
+                               cc::BrowserControlsState::kHidden);
+#endif
+}
+
 }  // namespace weblayer

weblayer/browser/tab_impl.h

@@ -127,7 +127,8 @@ class TabImpl : public Tab,
   bool has_new_tab_delegate() const { return new_tab_delegate_ != nullptr; }
   NewTabDelegate* new_tab_delegate() const { return new_tab_delegate_; }
 
-  // Called from Browser when this Tab is losing active status.
+  // Called from Browser when this Tab is gaining/losing active status.
+  void OnGainedActive();
   void OnLosingActive();
 
   bool IsActive();
@@ -363,6 +364,8 @@ class TabImpl : public Tab,
 
   bool SetDataInternal(const std::map<std::string, std::string>& data);
 
+  void EnterFullscreenImpl();
+
   BrowserImpl* browser_ = nullptr;
   ErrorPageDelegate* error_page_delegate_ = nullptr;
   FullscreenDelegate* fullscreen_delegate_ = nullptr;
@@ -399,6 +402,9 @@ class TabImpl : public Tab,
   // Set to true doing EnterFullscreenModeForTab().
   bool processing_enter_fullscreen_ = false;
 
+  // If true, the fullscreen delegate is called when the tab gains active.
+  bool enter_fullscreen_on_gained_active_ = false;
+
   std::unique_ptr<autofill::AutofillProvider> autofill_provider_;
 
   const std::string guid_;
@@ -410,7 +416,7 @@ class TabImpl : public Tab,
 
   std::unique_ptr<js_injection::JsCommunicationHost> js_communication_host_;
 
-  base::WeakPtrFactory<TabImpl> weak_ptr_factory_{this};
+  base::WeakPtrFactory<TabImpl> weak_ptr_factory_for_fullscreen_exit_{this};
 
   DISALLOW_COPY_AND_ASSIGN(TabImpl);
 };

weblayer/test/BUILD.gn

@@ -151,6 +151,7 @@ test("weblayer_browsertests") {
     "../browser/favicon/favicon_fetcher_browsertest.cc",
     "../browser/favicon/test_favicon_fetcher_delegate.cc",
     "../browser/favicon/test_favicon_fetcher_delegate.h",
+    "../browser/fullscreen_browsertest.cc",
     "../browser/google_accounts_browsertest.cc",
     "../browser/js_communication/web_message_browsertest.cc",
     "../browser/navigation_browsertest.cc",