Commit 5f90142c authored by yoshiki iguchi's avatar yoshiki iguchi Committed by Commit Bot

Add a way to suppress picture-in-picture

This CL adds a way to suppress (and un-suppress) picture-in-picture
windows. If it is set, picture-in-picture windows can't be crated. If
any picture-in-picture windows exist already, they are closed.

This CL suppresses only ARC PIP window. Chrome PIP window will be done
separately.

Summary of change of this CL:
- Makes ArcPipBridge observe spoken-feedback pref change
- Sends a newly-added mojo message when the spoken feedback en/disables

Related CLs:
- crrev.com/c/1285253 (chromium/src)
- ag/5287718 (vendor/google_arc)
- ag/5287717 (frameworks/base)

Bug: b/112060145
Test: manual (see pip window is supppressed correctly)
Change-Id: I554d74672e4e5136ae21fc0d2440a4150e5b5608
Reviewed-on: https://chromium-review.googlesource.com/c/1285253Reviewed-by: default avatarHidehiko Abe <hidehiko@chromium.org>
Reviewed-by: default avatarGreg Kerr <kerrnel@chromium.org>
Reviewed-by: default avatarEliot Courtney <edcourtney@chromium.org>
Commit-Queue: Yoshiki Iguchi <yoshiki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607489}
parent eacb27f8
...@@ -6,13 +6,17 @@ ...@@ -6,13 +6,17 @@
#include <utility> #include <utility>
#include "ash/public/cpp/ash_pref_names.h"
#include "base/auto_reset.h" #include "base/auto_reset.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/memory/singleton.h" #include "base/memory/singleton.h"
#include "chrome/browser/chromeos/arc/pip/arc_picture_in_picture_window_controller_impl.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 "chrome/browser/picture_in_picture/picture_in_picture_window_manager.h"
#include "chrome/browser/profiles/profile.h"
#include "components/arc/arc_bridge_service.h" #include "components/arc/arc_bridge_service.h"
#include "components/arc/arc_browser_context_keyed_service_factory_base.h" #include "components/arc/arc_browser_context_keyed_service_factory_base.h"
#include "components/prefs/pref_change_registrar.h"
#include "components/prefs/pref_service.h"
#include "content/public/browser/picture_in_picture_window_controller.h" #include "content/public/browser/picture_in_picture_window_controller.h"
namespace arc { namespace arc {
...@@ -49,10 +53,20 @@ ArcPipBridge* ArcPipBridge::GetForBrowserContext( ...@@ -49,10 +53,20 @@ ArcPipBridge* ArcPipBridge::GetForBrowserContext(
ArcPipBridge::ArcPipBridge(content::BrowserContext* context, ArcPipBridge::ArcPipBridge(content::BrowserContext* context,
ArcBridgeService* bridge_service) ArcBridgeService* bridge_service)
: arc_bridge_service_(bridge_service) { : arc_bridge_service_(bridge_service),
profile_(Profile::FromBrowserContext(context)) {
DCHECK(context);
DCHECK(profile_);
DVLOG(2) << "ArcPipBridge::ArcPipBridge"; DVLOG(2) << "ArcPipBridge::ArcPipBridge";
arc_bridge_service_->pip()->SetHost(this); arc_bridge_service_->pip()->SetHost(this);
arc_bridge_service_->pip()->AddObserver(this); arc_bridge_service_->pip()->AddObserver(this);
pref_change_registrar_.Init(profile_->GetPrefs());
pref_change_registrar_.Add(
ash::prefs::kAccessibilitySpokenFeedbackEnabled,
base::BindRepeating(&ArcPipBridge::OnSpokenFeedbackChanged,
base::Unretained(this)));
} }
ArcPipBridge::~ArcPipBridge() { ArcPipBridge::~ArcPipBridge() {
...@@ -63,6 +77,8 @@ ArcPipBridge::~ArcPipBridge() { ...@@ -63,6 +77,8 @@ ArcPipBridge::~ArcPipBridge() {
void ArcPipBridge::OnConnectionReady() { void ArcPipBridge::OnConnectionReady() {
DVLOG(1) << "ArcPipBridge::OnConnectionReady"; DVLOG(1) << "ArcPipBridge::OnConnectionReady";
// Send the initial status.
OnSpokenFeedbackChanged();
} }
void ArcPipBridge::OnConnectionClosed() { void ArcPipBridge::OnConnectionClosed() {
...@@ -99,4 +115,26 @@ void ArcPipBridge::ClosePip() { ...@@ -99,4 +115,26 @@ void ArcPipBridge::ClosePip() {
instance->ClosePip(); instance->ClosePip();
} }
bool ArcPipBridge::ShouldSuppressPip() const {
return profile_->GetPrefs()->GetBoolean(
ash::prefs::kAccessibilitySpokenFeedbackEnabled);
}
void ArcPipBridge::OnSpokenFeedbackChanged() {
const bool should_suppress = ShouldSuppressPip();
DVLOG(1) << "ArcPipBridge::OnSpokenFeedbackChanged (status: "
<< should_suppress << ")";
auto* instance = ARC_GET_INSTANCE_FOR_METHOD(arc_bridge_service_->pip(),
SetPipSuppressionStatus);
if (!instance)
return;
instance->SetPipSuppressionStatus(should_suppress);
// TODO(yoshiki): Add the code to suppress Chrome PIP window when the spoken
// feedback gets enabled.
}
} // namespace arc } // namespace arc
...@@ -10,6 +10,9 @@ ...@@ -10,6 +10,9 @@
#include "components/arc/common/pip.mojom.h" #include "components/arc/common/pip.mojom.h"
#include "components/arc/connection_observer.h" #include "components/arc/connection_observer.h"
#include "components/keyed_service/core/keyed_service.h" #include "components/keyed_service/core/keyed_service.h"
#include "components/prefs/pref_change_registrar.h"
class Profile;
namespace content { namespace content {
...@@ -44,11 +47,18 @@ class ArcPipBridge : public KeyedService, ...@@ -44,11 +47,18 @@ class ArcPipBridge : public KeyedService,
// PipInstance methods: // PipInstance methods:
void ClosePip(); void ClosePip();
void OnSpokenFeedbackChanged();
private: private:
bool ShouldSuppressPip() const;
ArcBridgeService* const arc_bridge_service_; ArcBridgeService* const arc_bridge_service_;
Profile* const profile_;
std::unique_ptr<ArcPictureInPictureWindowControllerImpl> std::unique_ptr<ArcPictureInPictureWindowControllerImpl>
pip_window_controller_; pip_window_controller_;
PrefChangeRegistrar pref_change_registrar_;
bool prevent_closing_pip_ = false; bool prevent_closing_pip_ = false;
DISALLOW_COPY_AND_ASSIGN(ArcPipBridge); DISALLOW_COPY_AND_ASSIGN(ArcPipBridge);
......
...@@ -4,11 +4,13 @@ ...@@ -4,11 +4,13 @@
#include "chrome/browser/chromeos/arc/pip/arc_picture_in_picture_window_controller_impl.h" #include "chrome/browser/chromeos/arc/pip/arc_picture_in_picture_window_controller_impl.h"
#include "ash/public/cpp/ash_pref_names.h"
#include "chrome/browser/chromeos/arc/pip/arc_pip_bridge.h" #include "chrome/browser/chromeos/arc/pip/arc_pip_bridge.h"
#include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile.h"
#include "components/arc/arc_bridge_service.h" #include "components/arc/arc_bridge_service.h"
#include "components/arc/test/connection_holder_util.h" #include "components/arc/test/connection_holder_util.h"
#include "components/arc/test/fake_pip_instance.h" #include "components/arc/test/fake_pip_instance.h"
#include "components/prefs/pref_service.h"
#include "content/public/test/test_browser_thread_bundle.h" #include "content/public/test/test_browser_thread_bundle.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -38,6 +40,12 @@ class ArcPipBridgeTest : public testing::Test { ...@@ -38,6 +40,12 @@ class ArcPipBridgeTest : public testing::Test {
ArcPipBridge* bridge() { return bridge_.get(); } ArcPipBridge* bridge() { return bridge_.get(); }
FakePipInstance* pip_instance() { return pip_instance_.get(); } FakePipInstance* pip_instance() { return pip_instance_.get(); }
protected:
void SetSpokenFeedbackEnabled(bool enabled) {
return testing_profile_.GetPrefs()->SetBoolean(
ash::prefs::kAccessibilitySpokenFeedbackEnabled, enabled);
}
private: private:
content::TestBrowserThreadBundle thread_bundle_; content::TestBrowserThreadBundle thread_bundle_;
TestingProfile testing_profile_; TestingProfile testing_profile_;
...@@ -62,4 +70,18 @@ TEST_F(ArcPipBridgeTest, OpeningAndroidPipTwiceElidesCloseCall) { ...@@ -62,4 +70,18 @@ TEST_F(ArcPipBridgeTest, OpeningAndroidPipTwiceElidesCloseCall) {
EXPECT_EQ(0, pip_instance()->num_closed()); EXPECT_EQ(0, pip_instance()->num_closed());
} }
TEST_F(ArcPipBridgeTest, ChromeVoxSuppressesPip) {
// Initialized as "false" on connection ready.
EXPECT_TRUE(pip_instance()->suppressed().has_value());
EXPECT_FALSE(pip_instance()->suppressed().value());
// Enabling spoken-feedback suppresses pip.
SetSpokenFeedbackEnabled(true);
EXPECT_TRUE(pip_instance()->suppressed().value());
// Disabling spoken-feedback unsuppresses pip.
SetSpokenFeedbackEnabled(false);
EXPECT_FALSE(pip_instance()->suppressed().value());
}
} // namespace arc } // namespace arc
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
// //
// Next MinVersion: 1 // Next MinVersion: 2
module arc.mojom; module arc.mojom;
...@@ -25,7 +25,7 @@ interface PipHost { ...@@ -25,7 +25,7 @@ interface PipHost {
}; };
// Interface for communicating to Android. // Interface for communicating to Android.
// Next Method ID: 2 // Next Method ID: 3
interface PipInstance { interface PipInstance {
// Establishes full-duplex communication with the host. // Establishes full-duplex communication with the host.
Init@0(PipHost host_ptr) => (); Init@0(PipHost host_ptr) => ();
...@@ -34,4 +34,9 @@ interface PipInstance { ...@@ -34,4 +34,9 @@ interface PipInstance {
// This is used if the user initiates a Chrome side PIP window, since we // This is used if the user initiates a Chrome side PIP window, since we
// want to avoid showing two PIP windows at the same time. // want to avoid showing two PIP windows at the same time.
ClosePip@1(); ClosePip@1();
// Updates the suppression status of PIP. True to suppress PIP windows, or
// false not to suppress. Setting this flag also closes existing PIP window.
[MinVersion=1]
SetPipSuppressionStatus@2(bool suppressed);
}; };
...@@ -21,4 +21,8 @@ void FakePipInstance::ClosePip() { ...@@ -21,4 +21,8 @@ void FakePipInstance::ClosePip() {
num_closed_++; num_closed_++;
} }
void FakePipInstance::SetPipSuppressionStatus(bool suppressed) {
suppressed_ = suppressed;
}
} // namespace arc } // namespace arc
...@@ -16,14 +16,17 @@ class FakePipInstance : public mojom::PipInstance { ...@@ -16,14 +16,17 @@ class FakePipInstance : public mojom::PipInstance {
~FakePipInstance() override; ~FakePipInstance() override;
int num_closed() { return num_closed_; } int num_closed() { return num_closed_; }
base::Optional<bool> suppressed() const { return suppressed_; }
// mojom::PipInstance overrides: // mojom::PipInstance overrides:
void Init(mojom::PipHostPtr host_ptr, InitCallback callback) override; void Init(mojom::PipHostPtr host_ptr, InitCallback callback) override;
void ClosePip() override; void ClosePip() override;
void SetPipSuppressionStatus(bool suppressed) override;
private: private:
mojom::PipHostPtr host_ptr_; mojom::PipHostPtr host_ptr_;
int num_closed_ = 0; int num_closed_ = 0;
base::Optional<bool> suppressed_;
DISALLOW_COPY_AND_ASSIGN(FakePipInstance); DISALLOW_COPY_AND_ASSIGN(FakePipInstance);
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment