Commit 4b598cd2 authored by Tetsui Ohkubo's avatar Tetsui Ohkubo Committed by Commit Bot

arc: Make minimize on back button finch switchable

As the impact of the feature is unclear, we want to add a kill switch of
the feature through Finch experiment, so that even when the feature
lands in stable it can be disabled quickly.

Android side CL: http://ag/9778557

TEST=Add --force-fieldtrials="ArcMinimizeOnBackButton/Enabled/" or
"ArcMinimizeOnBackButton/Disabled/" to /etc/chrome_dev.conf to see
TEST=ArcPropertyBridgeTest
the setting is properly overridden
BUG=972708,b:144896228

Change-Id: If82db66508f623a7a6f7846d8c7d702fa4bc4e95
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1930367Reviewed-by: default avatarJorge Lucangeli Obes <jorgelo@chromium.org>
Reviewed-by: default avatarHidehiko Abe <hidehiko@chromium.org>
Reviewed-by: default avatarStefan Kuhne <skuhne@chromium.org>
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720765}
parent 2e241020
...@@ -367,6 +367,7 @@ source_set("unit_tests") { ...@@ -367,6 +367,7 @@ source_set("unit_tests") {
"net/always_on_vpn_manager_unittest.cc", "net/always_on_vpn_manager_unittest.cc",
"net/arc_net_host_impl_unittest.cc", "net/arc_net_host_impl_unittest.cc",
"power/arc_power_bridge_unittest.cc", "power/arc_power_bridge_unittest.cc",
"property/arc_property_bridge_unittest.cc",
"session/arc_data_remover_unittest.cc", "session/arc_data_remover_unittest.cc",
"session/arc_session_impl_unittest.cc", "session/arc_session_impl_unittest.cc",
"session/arc_session_runner_unittest.cc", "session/arc_session_runner_unittest.cc",
......
...@@ -2,12 +2,16 @@ ...@@ -2,12 +2,16 @@
// 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;
// Next method ID: 2 // Next method ID: 3
interface PropertyInstance { interface PropertyInstance {
// Get Google camera app migration property. // Get Google camera app migration property.
GetGcaMigrationProperty@1() => (string? value); GetGcaMigrationProperty@1() => (string? value);
// Override the Android-side setting and minimize tasks on back button
// activation.
[MinVersion=2] SetMinimizeOnBackButton@2(bool enable);
}; };
...@@ -4,8 +4,12 @@ ...@@ -4,8 +4,12 @@
#include "components/arc/property/arc_property_bridge.h" #include "components/arc/property/arc_property_bridge.h"
#include <string>
#include <utility>
#include "base/bind.h" #include "base/bind.h"
#include "base/memory/singleton.h" #include "base/memory/singleton.h"
#include "base/metrics/field_trial.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/arc/session/arc_bridge_service.h" #include "components/arc/session/arc_bridge_service.h"
...@@ -60,6 +64,8 @@ void ArcPropertyBridge::OnConnectionReady() { ...@@ -60,6 +64,8 @@ void ArcPropertyBridge::OnConnectionReady() {
property_instance->GetGcaMigrationProperty(std::move(pending_request)); property_instance->GetGcaMigrationProperty(std::move(pending_request));
} }
pending_requests_.clear(); pending_requests_.clear();
SyncMinimizeOnBackButton();
} }
void ArcPropertyBridge::GetGcaMigrationProperty( void ArcPropertyBridge::GetGcaMigrationProperty(
...@@ -75,4 +81,17 @@ void ArcPropertyBridge::GetGcaMigrationProperty( ...@@ -75,4 +81,17 @@ void ArcPropertyBridge::GetGcaMigrationProperty(
property_instance->GetGcaMigrationProperty(std::move(callback)); property_instance->GetGcaMigrationProperty(std::move(callback));
} }
void ArcPropertyBridge::SyncMinimizeOnBackButton() {
mojom::PropertyInstance* property_instance = ARC_GET_INSTANCE_FOR_METHOD(
arc_bridge_service_->property(), SetMinimizeOnBackButton);
if (!property_instance)
return;
const std::string group =
base::FieldTrialList::FindFullName(kMinimizeOnBackButtonTrialName);
if (group == kMinimizeOnBackButtonEnabled)
property_instance->SetMinimizeOnBackButton(true);
else if (group == kMinimizeOnBackButtonDisabled)
property_instance->SetMinimizeOnBackButton(false);
}
} // namespace arc } // namespace arc
...@@ -5,14 +5,8 @@ ...@@ -5,14 +5,8 @@
#ifndef COMPONENTS_ARC_PROPERTY_ARC_PROPERTY_BRIDGE_H_ #ifndef COMPONENTS_ARC_PROPERTY_ARC_PROPERTY_BRIDGE_H_
#define COMPONENTS_ARC_PROPERTY_ARC_PROPERTY_BRIDGE_H_ #define COMPONENTS_ARC_PROPERTY_ARC_PROPERTY_BRIDGE_H_
#include <string>
#include <utility>
#include <vector> #include <vector>
#include "base/callback.h"
#include "base/macros.h"
#include "base/optional.h"
#include "base/strings/string_piece.h"
#include "base/threading/thread_checker.h" #include "base/threading/thread_checker.h"
#include "components/arc/mojom/property.mojom.h" #include "components/arc/mojom/property.mojom.h"
#include "components/arc/session/connection_observer.h" #include "components/arc/session/connection_observer.h"
...@@ -30,6 +24,12 @@ class ArcBridgeService; ...@@ -30,6 +24,12 @@ class ArcBridgeService;
class ArcPropertyBridge : public KeyedService, class ArcPropertyBridge : public KeyedService,
public ConnectionObserver<mojom::PropertyInstance> { public ConnectionObserver<mojom::PropertyInstance> {
public: public:
// Public for testing.
static constexpr const char* kMinimizeOnBackButtonTrialName =
"ArcMinimizeOnBackButton";
static constexpr const char* kMinimizeOnBackButtonEnabled = "Enabled";
static constexpr const char* kMinimizeOnBackButtonDisabled = "Disabled";
// Returns singleton instance for the given BrowserContext, // Returns singleton instance for the given BrowserContext,
// or nullptr if the browser |context| is not allowed to use ARC. // or nullptr if the browser |context| is not allowed to use ARC.
static ArcPropertyBridge* GetForBrowserContext( static ArcPropertyBridge* GetForBrowserContext(
...@@ -38,6 +38,8 @@ class ArcPropertyBridge : public KeyedService, ...@@ -38,6 +38,8 @@ class ArcPropertyBridge : public KeyedService,
ArcPropertyBridge(content::BrowserContext* context, ArcPropertyBridge(content::BrowserContext* context,
ArcBridgeService* bridge_service); ArcBridgeService* bridge_service);
~ArcPropertyBridge() override; ~ArcPropertyBridge() override;
ArcPropertyBridge(const ArcPropertyBridge&) = delete;
ArcPropertyBridge& operator=(const ArcPropertyBridge&) = delete;
// ConnectionObserver<mojom::PropertyInstance> overrides: // ConnectionObserver<mojom::PropertyInstance> overrides:
void OnConnectionReady() override; void OnConnectionReady() override;
...@@ -52,9 +54,10 @@ class ArcPropertyBridge : public KeyedService, ...@@ -52,9 +54,10 @@ class ArcPropertyBridge : public KeyedService,
std::vector<mojom::PropertyInstance::GetGcaMigrationPropertyCallback> std::vector<mojom::PropertyInstance::GetGcaMigrationPropertyCallback>
pending_requests_; pending_requests_;
THREAD_CHECKER(thread_checker_); // Send minimize on back button setting if specified by a field trial.
void SyncMinimizeOnBackButton();
DISALLOW_COPY_AND_ASSIGN(ArcPropertyBridge); THREAD_CHECKER(thread_checker_);
}; };
} // namespace arc } // namespace arc
......
// Copyright 2019 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/property/arc_property_bridge.h"
#include <memory>
#include "base/metrics/field_trial.h"
#include "base/test/scoped_field_trial_list_resetter.h"
#include "components/arc/session/arc_bridge_service.h"
#include "components/arc/test/connection_holder_util.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace arc {
namespace {
class FakePropertyInstance : public mojom::PropertyInstance {
public:
FakePropertyInstance() = default;
~FakePropertyInstance() override = default;
FakePropertyInstance(const FakePropertyInstance&) = delete;
FakePropertyInstance& operator=(const FakePropertyInstance&) = delete;
void GetGcaMigrationProperty(
GetGcaMigrationPropertyCallback callback) override {}
void SetMinimizeOnBackButton(bool enable) override {
minimize_on_back_ = enable;
}
base::Optional<bool> minimize_on_back() const { return minimize_on_back_; }
private:
base::Optional<bool> minimize_on_back_;
};
} // namespace
class ArcPropertyBridgeTest : public testing::Test {
public:
ArcPropertyBridgeTest()
: bridge_service_(std::make_unique<ArcBridgeService>()),
property_bridge_(
std::make_unique<ArcPropertyBridge>(nullptr,
bridge_service_.get())) {}
~ArcPropertyBridgeTest() override { DestroyPropertyInstance(); }
ArcPropertyBridgeTest(const ArcPropertyBridgeTest&) = delete;
ArcPropertyBridgeTest& operator=(const ArcPropertyBridgeTest&) = delete;
void SetUp() override {
trial_list_resetter_ =
std::make_unique<base::test::ScopedFieldTrialListResetter>();
trial_list_ = std::make_unique<base::FieldTrialList>(nullptr);
}
void TearDown() override {
trial_list_ = nullptr;
trial_list_resetter_ = nullptr;
}
void CreatePropertyInstance() {
instance_ = std::make_unique<FakePropertyInstance>();
bridge_service_->property()->SetInstance(instance_.get());
WaitForInstanceReady(bridge_service_->property());
}
void DestroyPropertyInstance() {
if (!instance_)
return;
bridge_service_->property()->CloseInstance(instance_.get());
instance_ = nullptr;
}
base::Optional<bool> GetMinimizeOnBackState() const {
return instance_->minimize_on_back();
}
private:
std::unique_ptr<ArcBridgeService> bridge_service_;
std::unique_ptr<ArcPropertyBridge> property_bridge_;
std::unique_ptr<FakePropertyInstance> instance_;
std::unique_ptr<base::test::ScopedFieldTrialListResetter>
trial_list_resetter_;
std::unique_ptr<base::FieldTrialList> trial_list_;
};
TEST_F(ArcPropertyBridgeTest, MinimizeOnBackButtonDefault) {
// The field trial is unset.
CreatePropertyInstance();
EXPECT_FALSE(GetMinimizeOnBackState().has_value());
}
TEST_F(ArcPropertyBridgeTest, MinimizeOnBackButtonEnabled) {
base::FieldTrialList::CreateFieldTrial(
ArcPropertyBridge::kMinimizeOnBackButtonTrialName,
ArcPropertyBridge::kMinimizeOnBackButtonEnabled);
CreatePropertyInstance();
EXPECT_EQ(true, GetMinimizeOnBackState());
}
TEST_F(ArcPropertyBridgeTest, MinimizeOnBackButtonDisabled) {
base::FieldTrialList::CreateFieldTrial(
ArcPropertyBridge::kMinimizeOnBackButtonTrialName,
ArcPropertyBridge::kMinimizeOnBackButtonDisabled);
CreatePropertyInstance();
EXPECT_EQ(false, GetMinimizeOnBackState());
}
} // namespace arc
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