Skip to content

GitLab

T
tangled
Commit f8c5a0bb authored by Chris Hamilton's avatar Chris Hamilton Committed by Commit Bot
Browse files
Options

Revert "[RC] Plumb in the enterprise policy setting." 

This reverts commit 15986e3d.

Reason for revert: Failures on official builds.

BUG=865420

Original change's description:
> [RC] Plumb in the enterprise policy setting.
> 
> This adds plumbing so that the enterprise policy setting can actually override
> the tab lifecycles feature.
> 
> BUG=
> 
> Change-Id: If86dd7687e993c7ed66955692be8eae88b505990
> Reviewed-on: https://chromium-review.googlesource.com/1142355
> Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org>
> Commit-Queue: Chris Hamilton <chrisha@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#576304}

TBR=chrisha@chromium.org,sebmarchand@chromium.org

Change-Id: I85eaea71836ceee31522cb36503d29341486b8a2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/1143564Reviewed-by: default avatarChris Hamilton <chrisha@chromium.org>
Commit-Queue: Chris Hamilton <chrisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576492}
parent 2a8b56a4
Hide whitespace changes
Inline Side-by-side
Showing with 1 addition and 221 deletions
chrome/browser/resource_coordinator/tab_lifecycle_unit.cc
View file @ f8c5a0bb
......@@ -228,11 +228,6 @@ void CheckIfTabCanCommunicateWithUserWhileInBackground(
CheckFeatureUsage(reader.get(), details);
}
bool GetTabLifecyclesEnterprisePolicy() {
return TabLifecycleUnitSource::GetInstance()
->tab_lifecycles_enterprise_policy();
}
InterventionPolicyDatabase* GetInterventionPolicyDatabase() {
return TabLifecycleUnitSource::GetInstance()->intervention_policy_database();
}
......@@ -465,11 +460,6 @@ bool TabLifecycleUnitSource::TabLifecycleUnit::CanFreeze(
return false;
}
if (!GetTabLifecyclesEnterprisePolicy()) {
decision_details->AddReason(
DecisionFailureReason::LIFECYCLES_ENTERPRISE_POLICY_OPT_OUT);
}
auto intervention_policy = GetInterventionPolicyDatabase()->GetFreezingPolicy(
url::Origin::Create(GetWebContents()->GetLastCommittedURL()));
......@@ -536,11 +526,6 @@ bool TabLifecycleUnitSource::TabLifecycleUnit::CanDiscard(
// This ensures that the decision details lists all possible reasons that the
// transition can be denied.
if (!GetTabLifecyclesEnterprisePolicy()) {
decision_details->AddReason(
DecisionFailureReason::LIFECYCLES_ENTERPRISE_POLICY_OPT_OUT);
}
if (reason == DiscardReason::kProactive) {
auto intervention_policy =
GetInterventionPolicyDatabase()->GetDiscardingPolicy(
......
chrome/browser/resource_coordinator/tab_lifecycle_unit_source.cc
View file @ f8c5a0bb
......@@ -4,10 +4,8 @@
#include "chrome/browser/resource_coordinator/tab_lifecycle_unit_source.h"
#include "base/bind.h"
#include "base/logging.h"
#include "base/stl_util.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/resource_coordinator/discard_metrics_lifecycle_unit_observer.h"
#include "chrome/browser/resource_coordinator/lifecycle_unit_source_observer.h"
#include "chrome/browser/resource_coordinator/tab_lifecycle_unit.h"
......@@ -16,9 +14,6 @@
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/recently_audible_helper.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/common/pref_names.h"
#include "components/prefs/pref_change_registrar.h"
#include "components/prefs/pref_service.h"
#include "content/public/browser/web_contents_user_data.h"
namespace resource_coordinator {
......@@ -70,17 +65,6 @@ TabLifecycleUnitSource::TabLifecycleUnitSource(
// this manual lifetime management.
if (auto* page_signal_receiver = PageSignalReceiver::GetInstance())
page_signal_receiver->AddObserver(this);
#if defined(GOOGLE_CHROME_BUILD)
// In production builds monitor the policy override of the lifecycles feature.
// This class owns the monitor so it is okay to use base::Unretained.
tab_lifecycles_enterprise_preference_monitor_ =
std::make_unique<TabLifecylesEnterprisePreferenceMonitor>(
g_browser_process->local_state(),
base::BindRepeating(
&TabLifecycleUnitSource::SetTabLifecyclesEnterprisePolicy,
base::Unretained(this)));
#endif
}
TabLifecycleUnitSource::~TabLifecycleUnitSource() {
......@@ -254,43 +238,6 @@ void TabLifecycleUnitSource::OnLifecycleStateChanged(
lifecycle_unit->UpdateLifecycleState(state);
}
void TabLifecycleUnitSource::SetTabLifecyclesEnterprisePolicy(bool enabled) {
tab_lifecycles_enterprise_policy_ = enabled;
}
TabLifecylesEnterprisePreferenceMonitor::
TabLifecylesEnterprisePreferenceMonitor(
PrefService* pref_service,
OnPreferenceChangedCallback callback)
: pref_service_(pref_service), callback_(callback) {
// Create a registrar to track changes to the setting.
pref_change_registrar_ = std::make_unique<PrefChangeRegistrar>();
pref_change_registrar_->Init(pref_service_);
pref_change_registrar_->Add(
prefs::kTabLifecyclesEnabled,
base::BindRepeating(&TabLifecylesEnterprisePreferenceMonitor::GetPref,
base::Unretained(this)));
// Do an initial check of the value.
GetPref();
}
TabLifecylesEnterprisePreferenceMonitor::
~TabLifecylesEnterprisePreferenceMonitor() = default;
void TabLifecylesEnterprisePreferenceMonitor::GetPref() {
bool enabled = true;
// If the preference is set to false by enterprise policy then disable the
// lifecycles feature.
const PrefService::Preference* pref =
pref_service_->FindPreference(prefs::kTabLifecyclesEnabled);
if (pref->IsManaged() && !pref->GetValue()->GetBool())
enabled = false;
callback_.Run(enabled);
}
} // namespace resource_coordinator
DEFINE_WEB_CONTENTS_USER_DATA_KEY(
......
chrome/browser/resource_coordinator/tab_lifecycle_unit_source.h
View file @ f8c5a0bb
......@@ -15,8 +15,6 @@
#include "chrome/browser/ui/browser_tab_strip_tracker.h"
#include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
class PrefChangeRegistrar;
class PrefService;
class TabStripModel;
namespace content {
......@@ -26,7 +24,6 @@ class WebContents;
namespace resource_coordinator {
class InterventionPolicyDatabase;
class TabLifecylesEnterprisePreferenceMonitor;
class TabLifecycleObserver;
class TabLifecycleUnitExternal;
class UsageClock;
......@@ -61,12 +58,6 @@ class TabLifecycleUnitSource : public BrowserListObserver,
return intervention_policy_database_;
}
// Returns the state of the tab lifecycles feature enterprise control. This
// returns true if the feature should be enabled, false otherwise.
bool tab_lifecycles_enterprise_policy() const {
return tab_lifecycles_enterprise_policy_;
}
class TabLifecycleUnitHolder;
private:
......@@ -133,9 +124,6 @@ class TabLifecycleUnitSource : public BrowserListObserver,
const PageNavigationIdentity& page_navigation_id,
mojom::LifecycleState state) override;
// Callback for TabLifecyclesEnterprisePreferenceMonitor.
void SetTabLifecyclesEnterprisePolicy(bool enabled);
// Tracks the BrowserList and all TabStripModels.
BrowserTabStripTracker browser_tab_strip_tracker_;
......@@ -156,39 +144,9 @@ class TabLifecycleUnitSource : public BrowserListObserver,
// A clock that advances when Chrome is in use.
UsageClock* const usage_clock_;
// The enterprise policy for overriding the tab lifecycles feature.
bool tab_lifecycles_enterprise_policy_ = true;
// In official production builds this monitors policy settings and reflects
// them in |tab_lifecycles_enterprise_policy_|.
std::unique_ptr<TabLifecylesEnterprisePreferenceMonitor>
tab_lifecycles_enterprise_preference_monitor_;
DISALLOW_COPY_AND_ASSIGN(TabLifecycleUnitSource);
};
// Helper class used for getting and monitoring enterprise-policy controlled
// preferences that can control the tab lifecycles feature. Exposed for testing.
class TabLifecylesEnterprisePreferenceMonitor {
public:
using OnPreferenceChangedCallback = base::RepeatingCallback<void(bool)>;
// Creates a preference monitor that monitors the provided PrefService. When
// the preference is initially checked or changed its value is provided via
// the provided callback.
TabLifecylesEnterprisePreferenceMonitor(PrefService* pref_service,
OnPreferenceChangedCallback callback);
~TabLifecylesEnterprisePreferenceMonitor();
private:
void GetPref();
PrefService* pref_service_;
OnPreferenceChangedCallback callback_;
std::unique_ptr<PrefChangeRegistrar> pref_change_registrar_;
};
} // namespace resource_coordinator
#endif // CHROME_BROWSER_RESOURCE_COORDINATOR_TAB_LIFECYCLE_UNIT_SOURCE_H_
chrome/browser/resource_coordinator/tab_lifecycle_unit_source_unittest.cc
View file @ f8c5a0bb
......@@ -22,10 +22,7 @@
#include "chrome/browser/resource_coordinator/time.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/browser/ui/tabs/test_tab_strip_model_delegate.h"
#include "chrome/common/pref_names.h"
#include "chrome/test/base/testing_profile.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/testing_pref_service.h"
#include "content/public/browser/navigation_controller.h"
#include "content/public/browser/web_contents.h"
#include "content/public/test/web_contents_tester.h"
......@@ -683,54 +680,4 @@ TEST_F(TabLifecycleUnitSourceTest, TabProactiveDiscardedByFrozenTimeout) {
::testing::Mock::VerifyAndClear(&tab_observer_);
}
namespace {
class MockOnPrefChanged {
public:
MockOnPrefChanged() = default;
~MockOnPrefChanged() = default;
MOCK_METHOD1(OnPrefChanged, void(bool));
private:
DISALLOW_COPY_AND_ASSIGN(MockOnPrefChanged);
};
} // namespace
TEST(TabLifecylesEnterprisePreferenceMonitor, ObservesChanges) {
TestingPrefServiceSimple pref_service;
pref_service.registry()->RegisterBooleanPref(prefs::kTabLifecyclesEnabled,
true);
::testing::StrictMock<MockOnPrefChanged> obs;
// Create a monitor that dispatches to the mock. The constructor should have
// checked the value and it should return the default.
EXPECT_CALL(obs, OnPrefChanged(true));
TabLifecylesEnterprisePreferenceMonitor monitor(
&pref_service, base::BindRepeating(&MockOnPrefChanged::OnPrefChanged,
base::Unretained(&obs)));
::testing::Mock::VerifyAndClear(&obs);
// Set the preference in an unmanaged way to false. The preference should
// still be true.
EXPECT_CALL(obs, OnPrefChanged(true));
pref_service.SetUserPref(prefs::kTabLifecyclesEnabled,
std::make_unique<base::Value>(false));
::testing::Mock::VerifyAndClear(&obs);
// Set the preference in a managed way to false.
EXPECT_CALL(obs, OnPrefChanged(false));
pref_service.SetManagedPref(prefs::kTabLifecyclesEnabled,
std::make_unique<base::Value>(false));
::testing::Mock::VerifyAndClear(&obs);
// Set the preference in a managed way to true.
EXPECT_CALL(obs, OnPrefChanged(true));
pref_service.SetManagedPref(prefs::kTabLifecyclesEnabled,
std::make_unique<base::Value>(true));
::testing::Mock::VerifyAndClear(&obs);
}
} // namespace resource_coordinator
chrome/browser/resource_coordinator/tab_lifecycle_unit_unittest.cc
View file @ f8c5a0bb
......@@ -32,6 +32,7 @@
#include "chrome/test/base/testing_profile.h"
#include "content/public/browser/web_contents.h"
#include "content/public/test/web_contents_tester.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace resource_coordinator {
......@@ -77,10 +78,6 @@ class TabLifecycleUnitTest : public testing::ChromeTestHarnessWithLocalDB {
protected:
using TabLifecycleUnit = TabLifecycleUnitSource::TabLifecycleUnit;
// This is an internal class so that it is also friends with
// TabLifecycleUnitTest.
class ScopedEnterpriseOptOut;
TabLifecycleUnitTest() : scoped_set_tick_clock_for_testing_(&test_clock_) {
observers_.AddObserver(&observer_);
}
......@@ -156,26 +153,12 @@ class TabLifecycleUnitTest : public testing::ChromeTestHarnessWithLocalDB {
std::unique_ptr<UsageClock> usage_clock_;
private:
// So that the main thread looks like the UI thread as expected.
TestTabStripModelDelegate tab_strip_model_delegate_;
ScopedSetTickClockForTesting scoped_set_tick_clock_for_testing_;
DISALLOW_COPY_AND_ASSIGN(TabLifecycleUnitTest);
};
class TabLifecycleUnitTest::ScopedEnterpriseOptOut {
public:
ScopedEnterpriseOptOut() {
TabLifecycleUnitSource::GetInstance()->SetTabLifecyclesEnterprisePolicy(
false);
}
~ScopedEnterpriseOptOut() {
TabLifecycleUnitSource::GetInstance()->SetTabLifecyclesEnterprisePolicy(
true);
}
};
void TabLifecycleUnitTest::TestCannotDiscardBasedOnHeuristicUsage(
DecisionFailureReason failure_reason,
void (SiteCharacteristicsDataWriter::*notify_feature_usage_method)()) {
......@@ -623,44 +606,4 @@ TEST_F(TabLifecycleUnitTest, NotifiedOfWebContentsVisibilityChanges) {
tab_lifecycle_unit.RemoveObserver(&observer);
}
TEST_F(TabLifecycleUnitTest, CannotDiscardIfEnterpriseOptOutUsed) {
TabLifecycleUnit tab_lifecycle_unit(&observers_, usage_clock_.get(),
web_contents_, tab_strip_model_.get());
ExpectCanDiscardTrueAllReasons(&tab_lifecycle_unit);
{
ScopedEnterpriseOptOut enterprise_opt_out;
ExpectCanDiscardFalseAllReasons(
&tab_lifecycle_unit,
DecisionFailureReason::LIFECYCLES_ENTERPRISE_POLICY_OPT_OUT);
}
ExpectCanDiscardTrueAllReasons(&tab_lifecycle_unit);
}
TEST_F(TabLifecycleUnitTest, CannotFreezeIfEnterpriseOptOutUsed) {
TabLifecycleUnit tab_lifecycle_unit(&observers_, usage_clock_.get(),
web_contents_, tab_strip_model_.get());
TabLoadTracker::Get()->TransitionStateForTesting(web_contents_,
LoadingState::LOADED);
DecisionDetails decision_details;
EXPECT_TRUE(tab_lifecycle_unit.CanFreeze(&decision_details));
EXPECT_TRUE(decision_details.IsPositive());
{
ScopedEnterpriseOptOut enterprise_opt_out;
decision_details.Clear();
EXPECT_FALSE(tab_lifecycle_unit.CanFreeze(&decision_details));
EXPECT_FALSE(decision_details.IsPositive());
EXPECT_EQ(DecisionFailureReason::LIFECYCLES_ENTERPRISE_POLICY_OPT_OUT,
decision_details.FailureReason());
}
decision_details.Clear();
EXPECT_TRUE(tab_lifecycle_unit.CanFreeze(&decision_details));
EXPECT_TRUE(decision_details.IsPositive());
}
} // namespace resource_coordinator
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