Use the Intervention Policy DB to assist discarding/freezing decisions
Other bug fix in this CL: Don't use the Local DB heuristics when an
extension try to discard a tab.
Bug: 773383
Change-Id: I1dd378a097d462793f25c382c9f96aa9381775d7
Reviewed-on: https://chromium-review.googlesource.com/1129620
Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574312}diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc
index 7cc7c39..9936798 100644
--- a/chrome/browser/browser_process_impl.cc
+++ b/chrome/browser/browser_process_impl.cc
@@ -876,7 +876,8 @@
if (!tab_manager_) {
tab_manager_ = std::make_unique<resource_coordinator::TabManager>();
tab_lifecycle_unit_source_ =
- std::make_unique<resource_coordinator::TabLifecycleUnitSource>();
+ std::make_unique<resource_coordinator::TabLifecycleUnitSource>(
+ tab_manager_->intervention_policy_database());
tab_lifecycle_unit_source_->AddObserver(tab_manager_.get());
}
return tab_manager_.get();
diff --git a/chrome/browser/resource_coordinator/intervention_policy_database.cc b/chrome/browser/resource_coordinator/intervention_policy_database.cc
index 3e43def5..6210a00 100644
--- a/chrome/browser/resource_coordinator/intervention_policy_database.cc
+++ b/chrome/browser/resource_coordinator/intervention_policy_database.cc
@@ -54,6 +54,13 @@
weak_factory_.GetWeakPtr()));
}
+void InterventionPolicyDatabase::AddOriginPoliciesForTesting(
+ const url::Origin& origin,
+ OriginInterventionPolicies policies) {
+ database_.emplace(SerializeOriginIntoDatabaseKey(origin),
+ std::move(policies));
+}
+
// static
InterventionPolicyDatabase::InterventionsMap
InterventionPolicyDatabase::ReadDatabaseFromProtoFileOnSequence(
diff --git a/chrome/browser/resource_coordinator/intervention_policy_database.h b/chrome/browser/resource_coordinator/intervention_policy_database.h
index 5558fde1..95463f5f 100644
--- a/chrome/browser/resource_coordinator/intervention_policy_database.h
+++ b/chrome/browser/resource_coordinator/intervention_policy_database.h
@@ -50,6 +50,9 @@
const base::Version& version,
std::unique_ptr<base::DictionaryValue> manifest);
+ void AddOriginPoliciesForTesting(const url::Origin& origin,
+ OriginInterventionPolicies policies);
+
protected:
// Map that associates the MD5 hash of an origin to its polices.
using InterventionsMap =
diff --git a/chrome/browser/resource_coordinator/tab_lifecycle_unit.cc b/chrome/browser/resource_coordinator/tab_lifecycle_unit.cc
index 5482e3d..e29168f 100644
--- a/chrome/browser/resource_coordinator/tab_lifecycle_unit.cc
+++ b/chrome/browser/resource_coordinator/tab_lifecycle_unit.cc
@@ -16,11 +16,13 @@
#include "chrome/browser/media/webrtc/media_capture_devices_dispatcher.h"
#include "chrome/browser/media/webrtc/media_stream_capture_indicator.h"
#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/resource_coordinator/intervention_policy_database.h"
#include "chrome/browser/resource_coordinator/lifecycle_unit_state.mojom.h"
#include "chrome/browser/resource_coordinator/local_site_characteristics_data_store_factory.h"
#include "chrome/browser/resource_coordinator/tab_activity_watcher.h"
#include "chrome/browser/resource_coordinator/tab_helper.h"
#include "chrome/browser/resource_coordinator/tab_lifecycle_observer.h"
+#include "chrome/browser/resource_coordinator/tab_lifecycle_unit_source.h"
#include "chrome/browser/resource_coordinator/tab_load_tracker.h"
#include "chrome/browser/resource_coordinator/tab_manager_features.h"
#include "chrome/browser/resource_coordinator/time.h"
@@ -219,6 +221,10 @@
DecisionFailureReason::HEURISTIC_TITLE);
}
+InterventionPolicyDatabase* GetInterventionPolicyDatabase() {
+ return TabLifecycleUnitSource::GetInstance()->intervention_policy_database();
+}
+
} // namespace
TabLifecycleUnitSource::TabLifecycleUnit::TabLifecycleUnit(
@@ -438,11 +444,24 @@
return false;
}
+ auto intervention_policy = GetInterventionPolicyDatabase()->GetFreezingPolicy(
+ url::Origin::Create(GetWebContents()->GetLastCommittedURL()));
+
+ switch (intervention_policy) {
+ case OriginInterventions::OPT_IN:
+ decision_details->AddReason(DecisionSuccessReason::GLOBAL_WHITELIST);
+ break;
+ case OriginInterventions::OPT_OUT:
+ decision_details->AddReason(DecisionFailureReason::GLOBAL_BLACKLIST);
+ break;
+ case OriginInterventions::DEFAULT:
+ break;
+ }
+
if (GetWebContents()->GetVisibility() == content::Visibility::VISIBLE)
decision_details->AddReason(DecisionFailureReason::LIVE_STATE_VISIBLE);
- CheckIfTabIsUsedInBackground(decision_details,
- false /* urgent_intervention */);
+ CheckIfTabIsUsedInBackground(decision_details, InterventionType::kProactive);
if (decision_details->reasons().empty()) {
decision_details->AddReason(
@@ -504,9 +523,26 @@
#endif // defined(OS_CHROMEOS)
}
-// We deliberately run through all of the logic without early termination.
-// This ensures that the decision details lists all possible reasons that the
-// transition can be denied.
+ // We deliberately run through all of the logic without early termination.
+ // This ensures that the decision details lists all possible reasons that the
+ // transition can be denied.
+
+ if (reason == DiscardReason::kProactive) {
+ auto intervention_policy =
+ GetInterventionPolicyDatabase()->GetDiscardingPolicy(
+ url::Origin::Create(GetWebContents()->GetLastCommittedURL()));
+
+ switch (intervention_policy) {
+ case OriginInterventions::OPT_IN:
+ decision_details->AddReason(DecisionSuccessReason::GLOBAL_WHITELIST);
+ break;
+ case OriginInterventions::OPT_OUT:
+ decision_details->AddReason(DecisionFailureReason::GLOBAL_BLACKLIST);
+ break;
+ case OriginInterventions::DEFAULT:
+ break;
+ }
+ }
#if defined(OS_CHROMEOS)
if (GetWebContents()->GetVisibility() == content::Visibility::VISIBLE)
@@ -535,7 +571,9 @@
}
CheckIfTabIsUsedInBackground(decision_details,
- reason == DiscardReason::kUrgent);
+ reason == DiscardReason::kProactive
+ ? InterventionType::kProactive
+ : InterventionType::kExternalOrUrgent);
if (decision_details->reasons().empty()) {
decision_details->AddReason(
@@ -818,7 +856,7 @@
void TabLifecycleUnitSource::TabLifecycleUnit::CheckIfTabIsUsedInBackground(
DecisionDetails* decision_details,
- bool urgent_intervention) const {
+ InterventionType intervention_type) const {
DCHECK(decision_details);
// We deliberately run through all of the logic without early termination.
@@ -830,9 +868,9 @@
// Consult the local database to see if this tab could try to communicate with
// the user while in background (don't check for the visibility here as
- // there's already a check for that above). Skip this test if this is an
- // urgent intervention.
- if (!urgent_intervention) {
+ // there's already a check for that above). Only do this for proactive
+ // interventions.
+ if (intervention_type == InterventionType::kProactive) {
CheckIfTabCanCommunicateWithUserWhileInBackground(GetWebContents(),
decision_details);
}
diff --git a/chrome/browser/resource_coordinator/tab_lifecycle_unit.h b/chrome/browser/resource_coordinator/tab_lifecycle_unit.h
index 01aca6c..33516c1 100644
--- a/chrome/browser/resource_coordinator/tab_lifecycle_unit.h
+++ b/chrome/browser/resource_coordinator/tab_lifecycle_unit.h
@@ -113,6 +113,12 @@
friend class TabLifecycleUnitSource;
private:
+ // Indicates if an intervention (freezing or discarding) is proactive or not.
+ enum class InterventionType {
+ kProactive,
+ kExternalOrUrgent,
+ };
+
// Determines if the tab is a media tab, and populates an optional
// |decision_details| with full details.
bool IsMediaTabImpl(DecisionDetails* decision_details) const;
@@ -146,11 +152,11 @@
// Indicates if freezing or discarding this tab would be noticeable by the
// user even if it isn't brought back to the foreground. Populates
- // |decision_details| with full details. |urgent_intervention| indicates if
- // this is for an urgent intervention, in which case some heuristics will be
- // skipped.
+ // |decision_details| with full details. If |intervention_type| indicates that
+ // this is a proactive intervention then more heuristics will be
+ // applied.
void CheckIfTabIsUsedInBackground(DecisionDetails* decision_details,
- bool urgent_intervention) const;
+ InterventionType intervention_type) const;
// List of observers to notify when the discarded state or the auto-
// discardable state of this tab changes.
diff --git a/chrome/browser/resource_coordinator/tab_lifecycle_unit_source.cc b/chrome/browser/resource_coordinator/tab_lifecycle_unit_source.cc
index 5830d42..2d30144 100644
--- a/chrome/browser/resource_coordinator/tab_lifecycle_unit_source.cc
+++ b/chrome/browser/resource_coordinator/tab_lifecycle_unit_source.cc
@@ -47,13 +47,16 @@
DISALLOW_COPY_AND_ASSIGN(TabLifecycleUnitHolder);
};
-TabLifecycleUnitSource::TabLifecycleUnitSource()
- : browser_tab_strip_tracker_(this, nullptr, this) {
+TabLifecycleUnitSource::TabLifecycleUnitSource(
+ InterventionPolicyDatabase* intervention_policy_database)
+ : browser_tab_strip_tracker_(this, nullptr, this),
+ intervention_policy_database_(intervention_policy_database) {
DCHECK(!instance_);
// In unit tests, tabs might already exist when TabLifecycleUnitSource is
// instantiated. No TabLifecycleUnit is created for these tabs.
+ DCHECK(intervention_policy_database_);
browser_tab_strip_tracker_.Init();
instance_ = this;
// TODO(chrisha): Create a ScopedPageSignalObserver helper class to clean up
diff --git a/chrome/browser/resource_coordinator/tab_lifecycle_unit_source.h b/chrome/browser/resource_coordinator/tab_lifecycle_unit_source.h
index 06b65fd..25ac393 100644
--- a/chrome/browser/resource_coordinator/tab_lifecycle_unit_source.h
+++ b/chrome/browser/resource_coordinator/tab_lifecycle_unit_source.h
@@ -23,6 +23,7 @@
namespace resource_coordinator {
+class InterventionPolicyDatabase;
class TabLifecycleObserver;
class TabLifecycleUnitExternal;
@@ -32,7 +33,8 @@
public PageSignalObserver,
public TabStripModelObserver {
public:
- TabLifecycleUnitSource();
+ explicit TabLifecycleUnitSource(
+ InterventionPolicyDatabase* intervention_policy_database);
~TabLifecycleUnitSource() override;
static TabLifecycleUnitSource* GetInstance();
@@ -50,6 +52,10 @@
// Pretend that |tab_strip| is the TabStripModel of the focused window.
void SetFocusedTabStripModelForTesting(TabStripModel* tab_strip);
+ InterventionPolicyDatabase* intervention_policy_database() const {
+ return intervention_policy_database_;
+ }
+
class TabLifecycleUnitHolder;
private:
@@ -129,6 +135,10 @@
// changes.
base::ObserverList<TabLifecycleObserver> tab_lifecycle_observers_;
+ // The intervention policy database used to assist freezing/discarding
+ // decisions.
+ InterventionPolicyDatabase* intervention_policy_database_;
+
DISALLOW_COPY_AND_ASSIGN(TabLifecycleUnitSource);
};
diff --git a/chrome/browser/resource_coordinator/tab_lifecycle_unit_unittest.cc b/chrome/browser/resource_coordinator/tab_lifecycle_unit_unittest.cc
index 8ecd2df..23c9853f21 100644
--- a/chrome/browser/resource_coordinator/tab_lifecycle_unit_unittest.cc
+++ b/chrome/browser/resource_coordinator/tab_lifecycle_unit_unittest.cc
@@ -10,6 +10,8 @@
#include "base/observer_list.h"
#include "base/test/simple_test_tick_clock.h"
#include "build/build_config.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/resource_coordinator/intervention_policy_database.h"
#include "chrome/browser/resource_coordinator/lifecycle_unit_observer.h"
#include "chrome/browser/resource_coordinator/local_site_characteristics_data_unittest_utils.h"
#include "chrome/browser/resource_coordinator/local_site_characteristics_webcontents_observer.h"
@@ -79,6 +81,9 @@
void SetUp() override {
ChromeTestHarnessWithLocalDB::SetUp();
+ // Force TabManager/TabLifecycleUnitSource creation.
+ g_browser_process->GetTabManager();
+
std::unique_ptr<content::WebContents> test_web_contents =
CreateTestWebContents();
web_contents_ = test_web_contents.get();
@@ -168,14 +173,14 @@
EXPECT_FALSE(decision_details.IsPositive());
EXPECT_EQ(failure_reason, decision_details.FailureReason());
}
+
+ // Heuristics shouldn't be considered for urgent or external tab discarding.
{
DecisionDetails decision_details;
- EXPECT_FALSE(tab_lifecycle_unit.CanDiscard(DiscardReason::kExternal,
- &decision_details));
- EXPECT_FALSE(decision_details.IsPositive());
- EXPECT_EQ(failure_reason, decision_details.FailureReason());
+ EXPECT_TRUE(tab_lifecycle_unit.CanDiscard(DiscardReason::kExternal,
+ &decision_details));
+ EXPECT_TRUE(decision_details.IsPositive());
}
- // Heuristics shouldn't be considered for urgent tab discarding.
{
DecisionDetails decision_details;
EXPECT_TRUE(tab_lifecycle_unit.CanDiscard(DiscardReason::kUrgent,
@@ -390,6 +395,104 @@
DecisionFailureReason::HEURISTIC_INSUFFICIENT_OBSERVATION, nullptr);
}
+TEST_F(TabLifecycleUnitTest, CannotProactivelyDiscardTabIfOriginOptedOut) {
+ InterventionPolicyDatabase* policy_db =
+ TabLifecycleUnitSource::GetInstance()->intervention_policy_database();
+ policy_db->AddOriginPoliciesForTesting(
+ url::Origin::Create(web_contents_->GetLastCommittedURL()),
+ {OriginInterventions::OPT_OUT, OriginInterventions::DEFAULT});
+
+ TabLifecycleUnit tab_lifecycle_unit(&observers_, web_contents_,
+ tab_strip_model_.get());
+ // Proactive discarding shouldn't be possible, urgent and external discarding
+ // should still be possible.
+ {
+ DecisionDetails decision_details;
+ EXPECT_FALSE(tab_lifecycle_unit.CanDiscard(DiscardReason::kProactive,
+ &decision_details));
+ EXPECT_FALSE(decision_details.IsPositive());
+ EXPECT_EQ(DecisionFailureReason::GLOBAL_BLACKLIST,
+ decision_details.FailureReason());
+ EXPECT_EQ(1U, decision_details.reasons().size());
+ }
+ {
+ DecisionDetails decision_details;
+ EXPECT_TRUE(tab_lifecycle_unit.CanDiscard(DiscardReason::kUrgent,
+ &decision_details));
+ EXPECT_TRUE(decision_details.IsPositive());
+ }
+ {
+ DecisionDetails decision_details;
+ EXPECT_TRUE(tab_lifecycle_unit.CanDiscard(DiscardReason::kExternal,
+ &decision_details));
+ EXPECT_TRUE(decision_details.IsPositive());
+ }
+}
+
+TEST_F(TabLifecycleUnitTest, CannotFreezeTabIfOriginOptedOut) {
+ auto* policy_db =
+ TabLifecycleUnitSource::GetInstance()->intervention_policy_database();
+ policy_db->AddOriginPoliciesForTesting(
+ url::Origin::Create(web_contents_->GetLastCommittedURL()),
+ InterventionPolicyDatabase::OriginInterventionPolicies(
+ OriginInterventions::DEFAULT, OriginInterventions::OPT_OUT));
+
+ TabLifecycleUnit tab_lifecycle_unit(&observers_, web_contents_,
+ tab_strip_model_.get());
+ TabLoadTracker::Get()->TransitionStateForTesting(web_contents_,
+ LoadingState::LOADED);
+ DecisionDetails decision_details;
+ EXPECT_FALSE(tab_lifecycle_unit.CanFreeze(&decision_details));
+ EXPECT_FALSE(decision_details.IsPositive());
+ EXPECT_EQ(DecisionFailureReason::GLOBAL_BLACKLIST,
+ decision_details.FailureReason());
+}
+
+TEST_F(TabLifecycleUnitTest, OptInTabsGetsDiscarded) {
+ auto* policy_db =
+ TabLifecycleUnitSource::GetInstance()->intervention_policy_database();
+ policy_db->AddOriginPoliciesForTesting(
+ url::Origin::Create(web_contents_->GetLastCommittedURL()),
+ InterventionPolicyDatabase::OriginInterventionPolicies(
+ OriginInterventions::OPT_IN, OriginInterventions::DEFAULT));
+
+ TabLifecycleUnit tab_lifecycle_unit(&observers_, web_contents_,
+ tab_strip_model_.get());
+
+ // Mark the tab as recently audible, this should protect it from being
+ // discarded.
+ tab_lifecycle_unit.SetRecentlyAudible(true);
+
+ DecisionDetails decision_details;
+ EXPECT_TRUE(tab_lifecycle_unit.CanDiscard(DiscardReason::kProactive,
+ &decision_details));
+ EXPECT_TRUE(decision_details.IsPositive());
+ EXPECT_EQ(DecisionSuccessReason::GLOBAL_WHITELIST,
+ decision_details.SuccessReason());
+}
+
+TEST_F(TabLifecycleUnitTest, CanFreezeOptedInTabs) {
+ auto* policy_db =
+ TabLifecycleUnitSource::GetInstance()->intervention_policy_database();
+ policy_db->AddOriginPoliciesForTesting(
+ url::Origin::Create(web_contents_->GetLastCommittedURL()),
+ {OriginInterventions::DEFAULT, OriginInterventions::OPT_IN});
+
+ TabLifecycleUnit tab_lifecycle_unit(&observers_, web_contents_,
+ tab_strip_model_.get());
+ TabLoadTracker::Get()->TransitionStateForTesting(web_contents_,
+ LoadingState::LOADED);
+
+ // Mark the tab as recently audible, this should protect it from being frozen.
+ tab_lifecycle_unit.SetRecentlyAudible(true);
+
+ DecisionDetails decision_details;
+ EXPECT_TRUE(tab_lifecycle_unit.CanFreeze(&decision_details));
+ EXPECT_TRUE(decision_details.IsPositive());
+ EXPECT_EQ(DecisionSuccessReason::GLOBAL_WHITELIST,
+ decision_details.SuccessReason());
+}
+
TEST_F(TabLifecycleUnitTest, CannotFreezeAFrozenTab) {
TabLifecycleUnit tab_lifecycle_unit(&observers_, web_contents_,
tab_strip_model_.get());
diff --git a/chrome/test/base/testing_browser_process.cc b/chrome/test/base/testing_browser_process.cc
index 1a946f8..1df7b45 100644
--- a/chrome/test/base/testing_browser_process.cc
+++ b/chrome/test/base/testing_browser_process.cc
@@ -432,7 +432,8 @@
if (!tab_manager_) {
tab_manager_ = std::make_unique<resource_coordinator::TabManager>();
tab_lifecycle_unit_source_ =
- std::make_unique<resource_coordinator::TabLifecycleUnitSource>();
+ std::make_unique<resource_coordinator::TabLifecycleUnitSource>(
+ tab_manager_->intervention_policy_database());
tab_lifecycle_unit_source_->AddObserver(tab_manager_.get());
}
return tab_manager_.get();
