Only allow one PIP window between Chrome and Android.

Bug: 841886
Bug: b/69370942
Test: unit test
Test: manually tested Android PIP causing Chrome PIP to close and vice-versa
Change-Id: Ic524db7d48b31d3351a3a669fc48b769a155d956
Reviewed-on: https://chromium-review.googlesource.com/1145148Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
Commit-Queue: Eliot Courtney <edcourtney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584689}

chrome/browser/chromeos/BUILD.gn

@@ -462,6 +462,8 @@ source_set("chromeos") {
     "arc/optin/arc_terms_of_service_negotiator.h",
     "arc/optin/arc_terms_of_service_oobe_negotiator.cc",
     "arc/optin/arc_terms_of_service_oobe_negotiator.h",
+    "arc/pip/arc_picture_in_picture_window_controller_impl.cc",
+    "arc/pip/arc_picture_in_picture_window_controller_impl.h",
     "arc/pip/arc_pip_bridge.cc",
     "arc/pip/arc_pip_bridge.h",
     "arc/policy/arc_android_management_checker.cc",
@@ -2009,6 +2011,7 @@ source_set("unit_tests") {
     "arc/notification/arc_provision_notification_service_unittest.cc",
     "arc/notification/arc_supervision_transition_notification_unittest.cc",
     "arc/optin/arc_terms_of_service_default_negotiator_unittest.cc",
+    "arc/pip/arc_pip_bridge_unittest.cc",
     "arc/policy/arc_policy_bridge_unittest.cc",
     "arc/process/arc_process_unittest.cc",
     "arc/tts/arc_tts_service_unittest.cc",

chrome/browser/chromeos/arc/pip/arc_picture_in_picture_window_controller_impl.cc

+// Copyright 2018 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 "chrome/browser/chromeos/arc/pip/arc_picture_in_picture_window_controller_impl.h"
+
+#include "chrome/browser/chromeos/arc/pip/arc_pip_bridge.h"
+
+namespace arc {
+
+ArcPictureInPictureWindowControllerImpl::
+    ArcPictureInPictureWindowControllerImpl(arc::ArcPipBridge* arc_pip_bridge)
+    : arc_pip_bridge_(arc_pip_bridge) {}
+
+ArcPictureInPictureWindowControllerImpl::
+    ~ArcPictureInPictureWindowControllerImpl() {
+  Close(false);
+}
+
+gfx::Size ArcPictureInPictureWindowControllerImpl::Show() {
+  // Should be a no-op on ARC. This is managed on the Android side.
+  return gfx::Size();
+}
+
+void ArcPictureInPictureWindowControllerImpl::Close(bool should_pause_video) {
+  // TODO(edcourtney): Currently, |should_pause_video| will always be false
+  // here, but if that changes, we should pause the video on the Android side.
+  arc_pip_bridge_->ClosePip();
+}
+
+void ArcPictureInPictureWindowControllerImpl::OnWindowDestroyed() {
+  // Should be a no-op on ARC. This is managed on the Android side.
+}
+
+void ArcPictureInPictureWindowControllerImpl::ClickCustomControl(
+    const std::string& control_id) {
+  // Should be a no-op on ARC. This is managed on the Android side.
+}
+
+void ArcPictureInPictureWindowControllerImpl::SetPictureInPictureCustomControls(
+    const std::vector<blink::PictureInPictureControlInfo>& info) {
+  // Should be a no-op on ARC. This is managed on the Android side.
+}
+
+void ArcPictureInPictureWindowControllerImpl::EmbedSurface(
+    const viz::SurfaceId& surface_id,
+    const gfx::Size& natural_size) {
+  // Should be a no-op on ARC. This is managed on the Android side.
+}
+
+content::OverlayWindow*
+ArcPictureInPictureWindowControllerImpl::GetWindowForTesting() {
+  // Should be a no-op on ARC. This is managed on the Android side.
+  return nullptr;
+}
+
+void ArcPictureInPictureWindowControllerImpl::UpdateLayerBounds() {
+  // Should be a no-op on ARC. This is managed on the Android side.
+}
+
+bool ArcPictureInPictureWindowControllerImpl::IsPlayerActive() {
+  // Should be a no-op on ARC. This is managed on the Android side.
+  return false;
+}
+
+content::WebContents*
+ArcPictureInPictureWindowControllerImpl::GetInitiatorWebContents() {
+  // Should be a no-op on ARC. This is managed on the Android side.
+  return nullptr;
+}
+
+void ArcPictureInPictureWindowControllerImpl::UpdatePlaybackState(
+    bool is_playing,
+    bool reached_end_of_stream) {
+  // Should be a no-op on ARC. This is managed on the Android side.
+}
+
+bool ArcPictureInPictureWindowControllerImpl::TogglePlayPause() {
+  // Should be a no-op on ARC. This is managed on the Android side.
+  return false;
+}
+
+}  // namespace arc

chrome/browser/chromeos/arc/pip/arc_picture_in_picture_window_controller_impl.h

+// Copyright 2018 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.
+
+#ifndef CHROME_BROWSER_CHROMEOS_ARC_PIP_ARC_PICTURE_IN_PICTURE_WINDOW_CONTROLLER_IMPL_H_
+#define CHROME_BROWSER_CHROMEOS_ARC_PIP_ARC_PICTURE_IN_PICTURE_WINDOW_CONTROLLER_IMPL_H_
+
+#include <string>
+#include <vector>
+
+#include "base/macros.h"
+#include "content/public/browser/picture_in_picture_window_controller.h"
+#include "ui/gfx/geometry/size.h"
+
+namespace content {
+
+class WebContents;
+class OverlayWindow;
+
+}  // namespace content
+
+namespace arc {
+
+class ArcPipBridge;
+
+// Implementation of PictureInPictureWindowController for ARC. This does nothing
+// for most of the methods, as most of it is managed on the Android side.
+class ArcPictureInPictureWindowControllerImpl
+    : public content::PictureInPictureWindowController {
+ public:
+  explicit ArcPictureInPictureWindowControllerImpl(
+      arc::ArcPipBridge* arc_pip_bridge);
+  ~ArcPictureInPictureWindowControllerImpl() override;
+
+  // PictureInPictureWindowController:
+  gfx::Size Show() override;
+  void Close(bool should_pause_video) override;
+  void OnWindowDestroyed() override;
+  void ClickCustomControl(const std::string& control_id) override;
+  void SetPictureInPictureCustomControls(
+      const std::vector<blink::PictureInPictureControlInfo>& info) override;
+  void EmbedSurface(const viz::SurfaceId& surface_id,
+                    const gfx::Size& natural_size) override;
+  content::OverlayWindow* GetWindowForTesting() override;
+  void UpdateLayerBounds() override;
+  bool IsPlayerActive() override;
+  content::WebContents* GetInitiatorWebContents() override;
+  bool TogglePlayPause() override;
+  void UpdatePlaybackState(bool is_playing,
+                           bool reached_end_of_stream) override;
+
+ private:
+  arc::ArcPipBridge* const arc_pip_bridge_;
+
+  DISALLOW_COPY_AND_ASSIGN(ArcPictureInPictureWindowControllerImpl);
+};
+
+}  // namespace arc
+
+#endif  // CHROME_BROWSER_CHROMEOS_ARC_PIP_ARC_PICTURE_IN_PICTURE_WINDOW_CONTROLLER_IMPL_H_

chrome/browser/chromeos/arc/pip/arc_pip_bridge.cc

@@ -4,10 +4,16 @@
 
 #include "chrome/browser/chromeos/arc/pip/arc_pip_bridge.h"
 
+#include <utility>
+
+#include "base/auto_reset.h"
 #include "base/logging.h"
 #include "base/memory/singleton.h"
+#include "chrome/browser/chromeos/arc/pip/arc_picture_in_picture_window_controller_impl.h"
+#include "chrome/browser/picture_in_picture/picture_in_picture_window_manager.h"
 #include "components/arc/arc_bridge_service.h"
 #include "components/arc/arc_browser_context_keyed_service_factory_base.h"
+#include "content/public/browser/picture_in_picture_window_controller.h"
 
 namespace arc {
 
@@ -65,12 +71,32 @@ void ArcPipBridge::OnConnectionClosed() {
 
 void ArcPipBridge::OnPipEvent(arc::mojom::ArcPipEvent event) {
   DVLOG(1) << "ArcPipBridge::OnPipEvent";
-  // TODO: Implement.
+
+  switch (event) {
+    case mojom::ArcPipEvent::ENTER: {
+      auto pip_window_controller =
+          std::make_unique<ArcPictureInPictureWindowControllerImpl>(this);
+      // Make sure not to close PIP if we are replacing an existing Android PIP.
+      base::AutoReset<bool> auto_prevent_closing_pip(&prevent_closing_pip_,
+                                                     true);
+      PictureInPictureWindowManager::GetInstance()
+          ->EnterPictureInPictureWithController(pip_window_controller.get());
+      pip_window_controller_ = std::move(pip_window_controller);
+      break;
+    }
+  }
 }
 
 void ArcPipBridge::ClosePip() {
   DVLOG(1) << "ArcPipBridge::ClosePip";
-  // TODO: Implement.
+
+  auto* instance =
+      ARC_GET_INSTANCE_FOR_METHOD(arc_bridge_service_->pip(), ClosePip);
+  if (!instance)
+    return;
+
+  if (!prevent_closing_pip_)
+    instance->ClosePip();
 }
 
 }  // namespace arc

chrome/browser/chromeos/arc/pip/arc_pip_bridge.h

@@ -5,17 +5,22 @@
 #ifndef CHROME_BROWSER_CHROMEOS_ARC_PIP_ARC_PIP_BRIDGE_H_
 #define CHROME_BROWSER_CHROMEOS_ARC_PIP_ARC_PIP_BRIDGE_H_
 
+#include <memory>
+
 #include "components/arc/common/pip.mojom.h"
 #include "components/arc/connection_observer.h"
 #include "components/keyed_service/core/keyed_service.h"
 
 namespace content {
+
 class BrowserContext;
+
 }  // namespace content
 
 namespace arc {
 
 class ArcBridgeService;
+class ArcPictureInPictureWindowControllerImpl;
 
 class ArcPipBridge : public KeyedService,
                      public ConnectionObserver<mojom::PipInstance>,
@@ -42,6 +47,10 @@ class ArcPipBridge : public KeyedService,
  private:
   ArcBridgeService* const arc_bridge_service_;
 
+  std::unique_ptr<ArcPictureInPictureWindowControllerImpl>
+      pip_window_controller_;
+  bool prevent_closing_pip_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(ArcPipBridge);
 };
 

chrome/browser/chromeos/arc/pip/arc_pip_bridge_unittest.cc

+// Copyright 2018 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 "chrome/browser/chromeos/arc/pip/arc_picture_in_picture_window_controller_impl.h"
+
+#include "chrome/browser/chromeos/arc/pip/arc_pip_bridge.h"
+#include "chrome/test/base/testing_profile.h"
+#include "components/arc/arc_bridge_service.h"
+#include "components/arc/test/connection_holder_util.h"
+#include "components/arc/test/fake_pip_instance.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace arc {
+
+class ArcPipBridgeTest : public testing::Test {
+ public:
+  ArcPipBridgeTest() = default;
+  ~ArcPipBridgeTest() override = default;
+
+  void SetUp() override {
+    arc_bridge_service_ = std::make_unique<ArcBridgeService>();
+    bridge_ = std::make_unique<ArcPipBridge>(&testing_profile_,
+                                             arc_bridge_service_.get());
+    pip_instance_ = std::make_unique<arc::FakePipInstance>();
+    arc_bridge_service_->pip()->SetInstance(pip_instance_.get());
+    WaitForInstanceReady(arc_bridge_service_->pip());
+  }
+
+  void TearDown() override {
+    arc_bridge_service_->pip()->CloseInstance(pip_instance_.get());
+    pip_instance_.reset();
+    bridge_.reset();
+    arc_bridge_service_.reset();
+  }
+
+  ArcPipBridge* bridge() { return bridge_.get(); }
+  FakePipInstance* pip_instance() { return pip_instance_.get(); }
+
+ private:
+  content::TestBrowserThreadBundle thread_bundle_;
+  TestingProfile testing_profile_;
+  std::unique_ptr<ArcBridgeService> arc_bridge_service_;
+  std::unique_ptr<ArcPipBridge> bridge_;
+  std::unique_ptr<FakePipInstance> pip_instance_;
+};
+
+TEST_F(ArcPipBridgeTest, ClosingPipTellsAndroidToClosePip) {
+  bridge()->ClosePip();
+  EXPECT_EQ(1, pip_instance()->num_closed());
+}
+
+// Test that when PictureInPictureWindowManager notices one Android PIP
+// is being replaced by another we don't erroneously send a close command
+// to Android.
+TEST_F(ArcPipBridgeTest, OpeningAndroidPipTwiceElidesCloseCall) {
+  bridge()->OnPipEvent(arc::mojom::ArcPipEvent::ENTER);
+  EXPECT_EQ(0, pip_instance()->num_closed());
+
+  bridge()->OnPipEvent(arc::mojom::ArcPipEvent::ENTER);
+  EXPECT_EQ(0, pip_instance()->num_closed());
+}
+
+}  // namespace arc

components/arc/BUILD.gn

@@ -237,6 +237,8 @@ static_library("arc_test_support") {
     "test/fake_file_system_instance.h",
     "test/fake_intent_helper_instance.cc",
     "test/fake_intent_helper_instance.h",
+    "test/fake_pip_instance.cc",
+    "test/fake_pip_instance.h",
     "test/fake_policy_instance.cc",
     "test/fake_policy_instance.h",
     "test/fake_power_instance.cc",

components/arc/test/fake_pip_instance.cc

+// Copyright 2018 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 "components/arc/test/fake_pip_instance.h"
+
+#include <utility>
+
+namespace arc {
+
+FakePipInstance::FakePipInstance() = default;
+
+FakePipInstance::~FakePipInstance() = default;
+
+void FakePipInstance::Init(mojom::PipHostPtr host_ptr, InitCallback callback) {
+  host_ptr_ = std::move(host_ptr);
+  std::move(callback).Run();
+}
+
+void FakePipInstance::ClosePip() {
+  num_closed_++;
+}
+
+}  // namespace arc

components/arc/test/fake_pip_instance.h

+// Copyright 2018 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.
+
+#ifndef COMPONENTS_ARC_TEST_FAKE_PIP_INSTANCE_H_
+#define COMPONENTS_ARC_TEST_FAKE_PIP_INSTANCE_H_
+
+#include "base/macros.h"
+#include "components/arc/common/pip.mojom.h"
+
+namespace arc {
+
+class FakePipInstance : public mojom::PipInstance {
+ public:
+  FakePipInstance();
+  ~FakePipInstance() override;
+
+  int num_closed() { return num_closed_; }
+
+  // mojom::PipInstance overrides:
+  void Init(mojom::PipHostPtr host_ptr, InitCallback callback) override;
+  void ClosePip() override;
+
+ private:
+  mojom::PipHostPtr host_ptr_;
+  int num_closed_ = 0;
+
+  DISALLOW_COPY_AND_ASSIGN(FakePipInstance);
+};
+
+}  // namespace arc
+
+#endif  // COMPONENTS_ARC_TEST_FAKE_PIP_INSTANCE_H_