Commit ef859ea8 authored by dmurph's avatar dmurph Committed by Commit bot

[Durable] Updated Durable heuristic to use 'important sites'

This also involves moving the important sites util into a location
visible to the durable storage permission context.

BUG=595433

Review-Url: https://codereview.chromium.org/2393103002
Cr-Commit-Position: refs/heads/master@{#423695}
parent 015d9b58
...@@ -337,6 +337,8 @@ split_static_library("browser") { ...@@ -337,6 +337,8 @@ split_static_library("browser") {
"download/drag_download_item.h", "download/drag_download_item.h",
"download/save_package_file_picker.cc", "download/save_package_file_picker.cc",
"download/save_package_file_picker.h", "download/save_package_file_picker.h",
"engagement/important_sites_util.cc",
"engagement/important_sites_util.h",
"engagement/site_engagement_eviction_policy.cc", "engagement/site_engagement_eviction_policy.cc",
"engagement/site_engagement_eviction_policy.h", "engagement/site_engagement_eviction_policy.h",
"engagement/site_engagement_helper.cc", "engagement/site_engagement_helper.cc",
...@@ -3134,8 +3136,6 @@ split_static_library("browser") { ...@@ -3134,8 +3136,6 @@ split_static_library("browser") {
"android/precache/precache_launcher.h", "android/precache/precache_launcher.h",
"android/preferences/autofill/autofill_profile_bridge.cc", "android/preferences/autofill/autofill_profile_bridge.cc",
"android/preferences/autofill/autofill_profile_bridge.h", "android/preferences/autofill/autofill_profile_bridge.h",
"android/preferences/important_sites_util.cc",
"android/preferences/important_sites_util.h",
"android/preferences/pref_service_bridge.cc", "android/preferences/pref_service_bridge.cc",
"android/preferences/pref_service_bridge.h", "android/preferences/pref_service_bridge.h",
"android/preferences/website_preference_bridge.cc", "android/preferences/website_preference_bridge.cc",
......
...@@ -22,7 +22,6 @@ ...@@ -22,7 +22,6 @@
#include "base/scoped_observer.h" #include "base/scoped_observer.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "base/values.h" #include "base/values.h"
#include "chrome/browser/android/preferences/important_sites_util.h"
#include "chrome/browser/browser_process.h" #include "chrome/browser/browser_process.h"
#include "chrome/browser/browsing_data/browsing_data_filter_builder.h" #include "chrome/browser/browsing_data/browsing_data_filter_builder.h"
#include "chrome/browser/browsing_data/browsing_data_helper.h" #include "chrome/browser/browsing_data/browsing_data_helper.h"
...@@ -30,6 +29,7 @@ ...@@ -30,6 +29,7 @@
#include "chrome/browser/browsing_data/browsing_data_remover_factory.h" #include "chrome/browser/browsing_data/browsing_data_remover_factory.h"
#include "chrome/browser/browsing_data/registrable_domain_filter_builder.h" #include "chrome/browser/browsing_data/registrable_domain_filter_builder.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h" #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/engagement/important_sites_util.h"
#include "chrome/browser/history/web_history_service_factory.h" #include "chrome/browser/history/web_history_service_factory.h"
#include "chrome/browser/net/prediction_options.h" #include "chrome/browser/net/prediction_options.h"
#include "chrome/browser/prefs/incognito_mode_prefs.h" #include "chrome/browser/prefs/incognito_mode_prefs.h"
......
...@@ -18,7 +18,6 @@ ...@@ -18,7 +18,6 @@
#include "base/json/json_writer.h" #include "base/json/json_writer.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/macros.h" #include "base/macros.h"
#include "chrome/browser/android/preferences/important_sites_util.h"
#include "chrome/browser/browsing_data/browsing_data_flash_lso_helper.h" #include "chrome/browser/browsing_data/browsing_data_flash_lso_helper.h"
#include "chrome/browser/browsing_data/browsing_data_local_storage_helper.h" #include "chrome/browser/browsing_data/browsing_data_local_storage_helper.h"
#include "chrome/browser/browsing_data/browsing_data_quota_helper.h" #include "chrome/browser/browsing_data/browsing_data_quota_helper.h"
...@@ -28,6 +27,7 @@ ...@@ -28,6 +27,7 @@
#include "chrome/browser/content_settings/host_content_settings_map_factory.h" #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/content_settings/tab_specific_content_settings.h" #include "chrome/browser/content_settings/tab_specific_content_settings.h"
#include "chrome/browser/content_settings/web_site_settings_uma_util.h" #include "chrome/browser/content_settings/web_site_settings_uma_util.h"
#include "chrome/browser/engagement/important_sites_util.h"
#include "chrome/browser/notifications/desktop_notification_profile_util.h" #include "chrome/browser/notifications/desktop_notification_profile_util.h"
#include "chrome/browser/permissions/permission_uma_util.h" #include "chrome/browser/permissions/permission_uma_util.h"
#include "chrome/browser/permissions/permission_util.h" #include "chrome/browser/permissions/permission_util.h"
......
...@@ -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.
#include "chrome/browser/android/preferences/important_sites_util.h" #include "chrome/browser/engagement/important_sites_util.h"
#include <algorithm> #include <algorithm>
#include <map> #include <map>
...@@ -84,9 +84,10 @@ enum CrossedReason { ...@@ -84,9 +84,10 @@ enum CrossedReason {
}; };
CrossedReason GetCrossedReasonFromBitfield(int32_t reason_bitfield) { CrossedReason GetCrossedReasonFromBitfield(int32_t reason_bitfield) {
bool durable = reason_bitfield & (1 << ImportantReason::DURABLE); bool durable = (reason_bitfield & (1 << ImportantReason::DURABLE)) != 0;
bool notifications = reason_bitfield & (1 << ImportantReason::NOTIFICATIONS); bool notifications =
bool engagement = reason_bitfield & (1 << ImportantReason::ENGAGEMENT); (reason_bitfield & (1 << ImportantReason::NOTIFICATIONS)) != 0;
bool engagement = (reason_bitfield & (1 << ImportantReason::ENGAGEMENT)) != 0;
if (durable && notifications && engagement) if (durable && notifications && engagement)
return CROSSED_NOTIFICATIONS_AND_DURABLE_AND_ENGAGEMENT; return CROSSED_NOTIFICATIONS_AND_DURABLE_AND_ENGAGEMENT;
else if (notifications && durable) else if (notifications && durable)
...@@ -430,6 +431,7 @@ void ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites( ...@@ -430,6 +431,7 @@ void ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites(
void ImportantSitesUtil::MarkOriginAsImportantForTesting(Profile* profile, void ImportantSitesUtil::MarkOriginAsImportantForTesting(Profile* profile,
const GURL& origin) { const GURL& origin) {
SiteEngagementScore::SetParamValuesForTesting();
// First get data from site engagement. // First get data from site engagement.
SiteEngagementService* site_engagement_service = SiteEngagementService* site_engagement_service =
SiteEngagementService::Get(profile); SiteEngagementService::Get(profile);
......
...@@ -2,8 +2,8 @@ ...@@ -2,8 +2,8 @@
// 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.
#ifndef CHROME_BROWSER_ANDROID_PREFERENCES_IMPORTANT_SITES_UTIL_H_ #ifndef CHROME_BROWSER_ENGAGEMENT_IMPORTANT_SITES_UTIL_H_
#define CHROME_BROWSER_ANDROID_PREFERENCES_IMPORTANT_SITES_UTIL_H_ #define CHROME_BROWSER_ENGAGEMENT_IMPORTANT_SITES_UTIL_H_
#include <string> #include <string>
#include <vector> #include <vector>
...@@ -45,7 +45,9 @@ class ImportantSitesUtil { ...@@ -45,7 +45,9 @@ class ImportantSitesUtil {
const std::vector<std::string>& ignored_sites, const std::vector<std::string>& ignored_sites,
const std::vector<int32_t>& ignored_sites_reason_bitfield); const std::vector<int32_t>& ignored_sites_reason_bitfield);
// This marks the given origin as important for testing. // This marks the given origin as important for testing. Note: This changes
// the score requirements for the Site Engagement Service, so ONLY call for
// testing.
static void MarkOriginAsImportantForTesting(Profile* profile, static void MarkOriginAsImportantForTesting(Profile* profile,
const GURL& origin); const GURL& origin);
...@@ -53,4 +55,4 @@ class ImportantSitesUtil { ...@@ -53,4 +55,4 @@ class ImportantSitesUtil {
DISALLOW_IMPLICIT_CONSTRUCTORS(ImportantSitesUtil); DISALLOW_IMPLICIT_CONSTRUCTORS(ImportantSitesUtil);
}; };
#endif // CHROME_BROWSER_ANDROID_PREFERENCES_IMPORTANT_SITES_UTIL_H_ #endif // CHROME_BROWSER_ENGAGEMENT_IMPORTANT_SITES_UTIL_H_
...@@ -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.
#include "chrome/browser/android/preferences/important_sites_util.h" #include "chrome/browser/engagement/important_sites_util.h"
#include <memory> #include <memory>
#include <utility> #include <utility>
...@@ -69,11 +69,11 @@ class ImportantSitesUtilTest : public ChromeRenderViewHostTestHarness { ...@@ -69,11 +69,11 @@ class ImportantSitesUtilTest : public ChromeRenderViewHostTestHarness {
public: public:
void SetUp() override { void SetUp() override {
ChromeRenderViewHostTestHarness::SetUp(); ChromeRenderViewHostTestHarness::SetUp();
SiteEngagementScore::SetParamValuesForTesting();
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
g_temp_history_dir = temp_dir_.GetPath(); g_temp_history_dir = temp_dir_.GetPath();
HistoryServiceFactory::GetInstance()->SetTestingFactory( HistoryServiceFactory::GetInstance()->SetTestingFactory(
profile(), &BuildTestHistoryService); profile(), &BuildTestHistoryService);
SiteEngagementScore::SetParamValuesForTesting();
} }
void AddContentSetting(ContentSettingsType type, void AddContentSetting(ContentSettingsType type,
...@@ -113,6 +113,24 @@ class ImportantSitesUtilTest : public ChromeRenderViewHostTestHarness { ...@@ -113,6 +113,24 @@ class ImportantSitesUtilTest : public ChromeRenderViewHostTestHarness {
} }
} }
void ExpectImportantResultsEqualUnordered(
const std::vector<std::string>& domains,
const std::vector<GURL>& expected_sorted_origins,
const std::vector<ImportantDomainInfo>& important_sites) {
ASSERT_EQ(domains.size(), important_sites.size());
ASSERT_EQ(expected_sorted_origins.size(), important_sites.size());
std::vector<std::string> actual_domains;
std::vector<GURL> actual_origins;
for (size_t i = 0; i < important_sites.size(); i++) {
actual_domains.push_back(important_sites[i].registerable_domain);
actual_origins.push_back(important_sites[i].example_origin);
}
EXPECT_THAT(actual_domains, testing::UnorderedElementsAreArray(domains));
EXPECT_THAT(actual_origins,
testing::UnorderedElementsAreArray(expected_sorted_origins));
}
private: private:
base::ScopedTempDir temp_dir_; base::ScopedTempDir temp_dir_;
BookmarkModel* model_ = nullptr; BookmarkModel* model_ = nullptr;
...@@ -219,8 +237,8 @@ TEST_F(ImportantSitesUtilTest, TooManyBookmarks) { ...@@ -219,8 +237,8 @@ TEST_F(ImportantSitesUtilTest, TooManyBookmarks) {
profile(), kNumImportantSites); profile(), kNumImportantSites);
EXPECT_EQ(0u, important_sites.size()); EXPECT_EQ(0u, important_sites.size());
// If we add some site engagement, these should show up in order (even though // If we add some site engagement, they should show up (even though the site
// the engagement is too low for a signal by itself). // engagement score is too low for a signal by itself).
service->ResetScoreForURL(url1, 2); service->ResetScoreForURL(url1, 2);
service->ResetScoreForURL(url4, 3); service->ResetScoreForURL(url4, 3);
service->ResetScoreForURL(url7, 0); service->ResetScoreForURL(url7, 0);
...@@ -231,8 +249,8 @@ TEST_F(ImportantSitesUtilTest, TooManyBookmarks) { ...@@ -231,8 +249,8 @@ TEST_F(ImportantSitesUtilTest, TooManyBookmarks) {
std::vector<std::string> expected_sorted_domains = {"google.com", std::vector<std::string> expected_sorted_domains = {"google.com",
"chrome.com"}; "chrome.com"};
std::vector<GURL> expected_sorted_origins = {url1, url4}; std::vector<GURL> expected_sorted_origins = {url1, url4};
ExpectImportantResultsEq(expected_sorted_domains, expected_sorted_origins, ExpectImportantResultsEqualUnordered(
important_sites); expected_sorted_domains, expected_sorted_origins, important_sites);
} }
TEST_F(ImportantSitesUtilTest, Blacklisting) { TEST_F(ImportantSitesUtilTest, Blacklisting) {
...@@ -258,9 +276,11 @@ TEST_F(ImportantSitesUtilTest, Blacklisting) { ...@@ -258,9 +276,11 @@ TEST_F(ImportantSitesUtilTest, Blacklisting) {
ASSERT_EQ(1u, important_sites.size()); ASSERT_EQ(1u, important_sites.size());
// Record ignore twice. // Record ignore twice.
ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites( ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites(
profile(), {}, {}, {"google.com"}, {important_sites[0].reason_bitfield}); profile(), std::vector<std::string>(), std::vector<int32_t>(),
{"google.com"}, {important_sites[0].reason_bitfield});
ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites( ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites(
profile(), {}, {}, {"google.com"}, {important_sites[0].reason_bitfield}); profile(), std::vector<std::string>(), std::vector<int32_t>(),
{"google.com"}, {important_sites[0].reason_bitfield});
// Important fetch 2. // Important fetch 2.
important_sites = ImportantSitesUtil::GetImportantRegisterableDomains( important_sites = ImportantSitesUtil::GetImportantRegisterableDomains(
...@@ -272,7 +292,8 @@ TEST_F(ImportantSitesUtilTest, Blacklisting) { ...@@ -272,7 +292,8 @@ TEST_F(ImportantSitesUtilTest, Blacklisting) {
// Record ignore 3rd time. // Record ignore 3rd time.
ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites( ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites(
profile(), {}, {}, {"google.com"}, {important_sites[0].reason_bitfield}); profile(), std::vector<std::string>(), std::vector<int32_t>(),
{"google.com"}, {important_sites[0].reason_bitfield});
// Important fetch 3. We should be blacklisted now. // Important fetch 3. We should be blacklisted now.
important_sites = ImportantSitesUtil::GetImportantRegisterableDomains( important_sites = ImportantSitesUtil::GetImportantRegisterableDomains(
...@@ -299,9 +320,11 @@ TEST_F(ImportantSitesUtilTest, BlacklistingReset) { ...@@ -299,9 +320,11 @@ TEST_F(ImportantSitesUtilTest, BlacklistingReset) {
// Record ignored twice. // Record ignored twice.
ASSERT_EQ(1u, important_sites.size()); ASSERT_EQ(1u, important_sites.size());
ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites( ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites(
profile(), {}, {}, {"google.com"}, {important_sites[0].reason_bitfield}); profile(), std::vector<std::string>(), std::vector<int32_t>(),
{"google.com"}, {important_sites[0].reason_bitfield});
ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites( ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites(
profile(), {}, {}, {"google.com"}, {important_sites[0].reason_bitfield}); profile(), std::vector<std::string>(), std::vector<int32_t>(),
{"google.com"}, {important_sites[0].reason_bitfield});
// Important fetch, we should still be there. // Important fetch, we should still be there.
important_sites = ImportantSitesUtil::GetImportantRegisterableDomains( important_sites = ImportantSitesUtil::GetImportantRegisterableDomains(
...@@ -314,13 +337,16 @@ TEST_F(ImportantSitesUtilTest, BlacklistingReset) { ...@@ -314,13 +337,16 @@ TEST_F(ImportantSitesUtilTest, BlacklistingReset) {
// Record NOT ignored. // Record NOT ignored.
ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites( ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites(
profile(), {"google.com"}, {important_sites[0].reason_bitfield}, {}, {}); profile(), {"google.com"}, {important_sites[0].reason_bitfield},
std::vector<std::string>(), std::vector<int32_t>());
// Record ignored twice again. // Record ignored twice again.
ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites( ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites(
profile(), {}, {}, {"google.com"}, {important_sites[0].reason_bitfield}); profile(), std::vector<std::string>(), std::vector<int32_t>(),
{"google.com"}, {important_sites[0].reason_bitfield});
ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites( ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites(
profile(), {}, {}, {"google.com"}, {important_sites[0].reason_bitfield}); profile(), std::vector<std::string>(), std::vector<int32_t>(),
{"google.com"}, {important_sites[0].reason_bitfield});
// Important fetch, we should still be there. // Important fetch, we should still be there.
important_sites = ImportantSitesUtil::GetImportantRegisterableDomains( important_sites = ImportantSitesUtil::GetImportantRegisterableDomains(
...@@ -330,7 +356,8 @@ TEST_F(ImportantSitesUtilTest, BlacklistingReset) { ...@@ -330,7 +356,8 @@ TEST_F(ImportantSitesUtilTest, BlacklistingReset) {
// Record ignored 3rd time in a row. // Record ignored 3rd time in a row.
ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites( ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites(
profile(), {}, {}, {"google.com"}, {important_sites[0].reason_bitfield}); profile(), std::vector<std::string>(), std::vector<int32_t>(),
{"google.com"}, {important_sites[0].reason_bitfield});
// Blacklisted now. // Blacklisted now.
important_sites = ImportantSitesUtil::GetImportantRegisterableDomains( important_sites = ImportantSitesUtil::GetImportantRegisterableDomains(
...@@ -363,12 +390,11 @@ TEST_F(ImportantSitesUtilTest, Metrics) { ...@@ -363,12 +390,11 @@ TEST_F(ImportantSitesUtilTest, Metrics) {
{important_sites[0].reason_bitfield, important_sites[1].reason_bitfield}, {important_sites[0].reason_bitfield, important_sites[1].reason_bitfield},
{"bad.com"}, {important_sites[2].reason_bitfield}); {"bad.com"}, {important_sites[2].reason_bitfield});
EXPECT_THAT(histogram_tester.GetAllSamples( EXPECT_THAT(
"Storage.ImportantSites.CBDChosenReason"), histogram_tester.GetAllSamples("Storage.ImportantSites.CBDChosenReason"),
testing::ElementsAre( testing::ElementsAre(base::Bucket(ENGAGEMENT, 1),
base::Bucket(ENGAGEMENT, 1), base::Bucket(BOOKMARKS, 1),
base::Bucket(BOOKMARKS, 1), base::Bucket(NOTIFICATIONS, 1)));
base::Bucket(NOTIFICATIONS, 1)));
EXPECT_THAT( EXPECT_THAT(
histogram_tester.GetAllSamples("Storage.ImportantSites.CBDIgnoredReason"), histogram_tester.GetAllSamples("Storage.ImportantSites.CBDIgnoredReason"),
......
...@@ -158,6 +158,7 @@ class SiteEngagementScore { ...@@ -158,6 +158,7 @@ class SiteEngagementScore {
FRIEND_TEST_ALL_PREFIXES(SiteEngagementScoreTest, PopulatedDictionary); FRIEND_TEST_ALL_PREFIXES(SiteEngagementScoreTest, PopulatedDictionary);
FRIEND_TEST_ALL_PREFIXES(SiteEngagementScoreTest, Reset); FRIEND_TEST_ALL_PREFIXES(SiteEngagementScoreTest, Reset);
friend class ChromePluginServiceFilterTest; friend class ChromePluginServiceFilterTest;
friend class ImportantSitesUtil;
friend class ImportantSitesUtilTest; friend class ImportantSitesUtilTest;
friend class SiteEngagementHelperTest; friend class SiteEngagementHelperTest;
friend class SiteEngagementScoreTest; friend class SiteEngagementScoreTest;
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "chrome/browser/content_settings/cookie_settings_factory.h" #include "chrome/browser/content_settings/cookie_settings_factory.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h" #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/content_settings/tab_specific_content_settings.h" #include "chrome/browser/content_settings/tab_specific_content_settings.h"
#include "chrome/browser/engagement/important_sites_util.h"
#include "chrome/browser/permissions/permission_request_id.h" #include "chrome/browser/permissions/permission_request_id.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "components/bookmarks/browser/bookmark_model.h" #include "components/bookmarks/browser/bookmark_model.h"
...@@ -21,6 +22,7 @@ ...@@ -21,6 +22,7 @@
#include "content/public/browser/child_process_security_policy.h" #include "content/public/browser/child_process_security_policy.h"
#include "content/public/browser/permission_type.h" #include "content/public/browser/permission_type.h"
#include "content/public/common/origin_util.h" #include "content/public/common/origin_util.h"
#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
#include "url/gurl.h" #include "url/gurl.h"
using bookmarks::BookmarkModel; using bookmarks::BookmarkModel;
...@@ -62,13 +64,20 @@ void DurableStoragePermissionContext::DecidePermission( ...@@ -62,13 +64,20 @@ void DurableStoragePermissionContext::DecidePermission(
return; return;
} }
// TODO(dmurph): Remove bookmarks check in favor of important sites. const size_t kMaxImportantResults = 10;
BookmarkModel* model = std::vector<ImportantSitesUtil::ImportantDomainInfo> important_sites =
BookmarkModelFactory::GetForBrowserContextIfExists(profile()); ImportantSitesUtil::GetImportantRegisterableDomains(profile(),
if (model) { kMaxImportantResults);
std::vector<bookmarks::BookmarkModel::URLAndTitle> bookmarks;
model->GetBookmarks(&bookmarks); std::string registerable_domain =
if (IsOriginBookmarked(bookmarks, requesting_origin)) { net::registry_controlled_domains::GetDomainAndRegistry(
requesting_origin,
net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES);
if (registerable_domain.empty() && requesting_origin.HostIsIPAddress())
registerable_domain = requesting_origin.host();
for (const auto& important_site : important_sites) {
if (important_site.registerable_domain == registerable_domain) {
NotifyPermissionSet(id, requesting_origin, embedding_origin, callback, NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
true /* persist */, CONTENT_SETTING_ALLOW); true /* persist */, CONTENT_SETTING_ALLOW);
return; return;
...@@ -97,15 +106,3 @@ void DurableStoragePermissionContext::UpdateContentSetting( ...@@ -97,15 +106,3 @@ void DurableStoragePermissionContext::UpdateContentSetting(
bool DurableStoragePermissionContext::IsRestrictedToSecureOrigins() const { bool DurableStoragePermissionContext::IsRestrictedToSecureOrigins() const {
return true; return true;
} }
bool DurableStoragePermissionContext::IsOriginBookmarked(
const std::vector<bookmarks::BookmarkModel::URLAndTitle>& bookmarks,
const GURL& origin) {
BookmarkModel::URLAndTitle looking_for;
looking_for.url = origin;
return std::binary_search(bookmarks.begin(), bookmarks.end(), looking_for,
[](const BookmarkModel::URLAndTitle& a,
const BookmarkModel::URLAndTitle& b) {
return a.url.GetOrigin() < b.url.GetOrigin();
});
}
...@@ -30,13 +30,6 @@ class DurableStoragePermissionContext : public PermissionContextBase { ...@@ -30,13 +30,6 @@ class DurableStoragePermissionContext : public PermissionContextBase {
bool IsRestrictedToSecureOrigins() const override; bool IsRestrictedToSecureOrigins() const override;
private: private:
FRIEND_TEST_ALL_PREFIXES(BookmarksOriginTest, Exists);
FRIEND_TEST_ALL_PREFIXES(BookmarksOriginTest, DoesntExist);
static bool IsOriginBookmarked(
const std::vector<bookmarks::BookmarkModel::URLAndTitle>& bookmarks,
const GURL& origin);
DISALLOW_COPY_AND_ASSIGN(DurableStoragePermissionContext); DISALLOW_COPY_AND_ASSIGN(DurableStoragePermissionContext);
}; };
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "chrome/browser/bookmarks/bookmark_model_factory.h" #include "chrome/browser/bookmarks/bookmark_model_factory.h"
#include "chrome/browser/content_settings/cookie_settings_factory.h" #include "chrome/browser/content_settings/cookie_settings_factory.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h" #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/engagement/important_sites_util.h"
#include "chrome/browser/permissions/permission_request_id.h" #include "chrome/browser/permissions/permission_request_id.h"
#include "chrome/browser/permissions/permission_request_manager.h" #include "chrome/browser/permissions/permission_request_manager.h"
#include "chrome/test/base/chrome_render_view_host_test_harness.h" #include "chrome/test/base/chrome_render_view_host_test_harness.h"
...@@ -74,74 +75,19 @@ class TestDurablePermissionContext : public DurableStoragePermissionContext { ...@@ -74,74 +75,19 @@ class TestDurablePermissionContext : public DurableStoragePermissionContext {
} // namespace } // namespace
class BookmarksOriginTest : public ::testing::Test {
protected:
static std::vector<BookmarkModel::URLAndTitle> MakeBookmarks(
const std::string urls[],
const int array_size) {
std::vector<BookmarkModel::URLAndTitle> bookmarks;
for (int i = 0; i < array_size; ++i) {
BookmarkModel::URLAndTitle bookmark;
bookmark.url = GURL(urls[i]);
EXPECT_TRUE(bookmark.url.is_valid());
bookmarks.push_back(bookmark);
}
return bookmarks;
}
};
TEST_F(BookmarksOriginTest, Exists) {
std::string urls[] = {
"http://www.google.com/",
"https://dogs.com/somepage.html",
"https://mail.google.com/mail/u/0/#inbox",
};
std::vector<BookmarkModel::URLAndTitle> bookmarks =
MakeBookmarks(urls, arraysize(urls));
GURL looking_for("https://dogs.com");
EXPECT_TRUE(DurableStoragePermissionContext::IsOriginBookmarked(
bookmarks, looking_for));
}
TEST_F(BookmarksOriginTest, DoesntExist) {
std::string urls[] = {
"http://www.google.com/",
"https://www.google.com/",
};
std::vector<BookmarkModel::URLAndTitle> bookmarks =
MakeBookmarks(urls, arraysize(urls));
GURL looking_for("https://dogs.com");
EXPECT_FALSE(DurableStoragePermissionContext::IsOriginBookmarked(
bookmarks, looking_for));
}
class DurableStoragePermissionContextTest class DurableStoragePermissionContextTest
: public ChromeRenderViewHostTestHarness { : public ChromeRenderViewHostTestHarness {
protected: protected:
void SetUp() override { void MakeOriginImportant(const GURL& origin) {
ChromeRenderViewHostTestHarness::SetUp(); ImportantSitesUtil::MarkOriginAsImportantForTesting(profile(), origin);
HostContentSettingsMapFactory::GetForProfile(profile())
->ClearSettingsForOneType(CONTENT_SETTINGS_TYPE_DURABLE_STORAGE);
} }
void AddBookmark(const GURL& origin) {
if (!model_) {
profile()->CreateBookmarkModel(true);
model_ = BookmarkModelFactory::GetForBrowserContext(profile());
bookmarks::test::WaitForBookmarkModelToLoad(model_);
}
model_->AddURL(model_->bookmark_bar_node(), 0,
base::ASCIIToUTF16(origin.spec()), origin);
}
BookmarkModel* model_ = nullptr;
}; };
TEST_F(DurableStoragePermissionContextTest, Bookmarked) { TEST_F(DurableStoragePermissionContextTest, Bookmarked) {
TestDurablePermissionContext permission_context(profile()); TestDurablePermissionContext permission_context(profile());
GURL url("https://www.google.com"); GURL url("https://www.google.com");
AddBookmark(url); MakeOriginImportant(url);
NavigateAndCommit(url); NavigateAndCommit(url);
const PermissionRequestID id(web_contents()->GetRenderProcessHost()->GetID(), const PermissionRequestID id(web_contents()->GetRenderProcessHost()->GetID(),
...@@ -167,7 +113,7 @@ TEST_F(DurableStoragePermissionContextTest, BookmarkAndIncognitoMode) { ...@@ -167,7 +113,7 @@ TEST_F(DurableStoragePermissionContextTest, BookmarkAndIncognitoMode) {
TestDurablePermissionContext permission_context( TestDurablePermissionContext permission_context(
profile()->GetOffTheRecordProfile()); profile()->GetOffTheRecordProfile());
GURL url("https://www.google.com"); GURL url("https://www.google.com");
AddBookmark(url); MakeOriginImportant(url);
NavigateAndCommit(url); NavigateAndCommit(url);
const PermissionRequestID id(web_contents()->GetRenderProcessHost()->GetID(), const PermissionRequestID id(web_contents()->GetRenderProcessHost()->GetID(),
...@@ -217,7 +163,7 @@ TEST_F(DurableStoragePermissionContextTest, NoBookmark) { ...@@ -217,7 +163,7 @@ TEST_F(DurableStoragePermissionContextTest, NoBookmark) {
TEST_F(DurableStoragePermissionContextTest, CookiesNotAllowed) { TEST_F(DurableStoragePermissionContextTest, CookiesNotAllowed) {
TestDurablePermissionContext permission_context(profile()); TestDurablePermissionContext permission_context(profile());
GURL url("https://www.google.com"); GURL url("https://www.google.com");
AddBookmark(url); MakeOriginImportant(url);
NavigateAndCommit(url); NavigateAndCommit(url);
scoped_refptr<content_settings::CookieSettings> cookie_settings = scoped_refptr<content_settings::CookieSettings> cookie_settings =
...@@ -248,7 +194,7 @@ TEST_F(DurableStoragePermissionContextTest, EmbeddedFrame) { ...@@ -248,7 +194,7 @@ TEST_F(DurableStoragePermissionContextTest, EmbeddedFrame) {
TestDurablePermissionContext permission_context(profile()); TestDurablePermissionContext permission_context(profile());
GURL url("https://www.google.com"); GURL url("https://www.google.com");
GURL requesting_url("https://www.youtube.com"); GURL requesting_url("https://www.youtube.com");
AddBookmark(url); MakeOriginImportant(url);
NavigateAndCommit(url); NavigateAndCommit(url);
const PermissionRequestID id(web_contents()->GetRenderProcessHost()->GetID(), const PermissionRequestID id(web_contents()->GetRenderProcessHost()->GetID(),
......
...@@ -3009,7 +3009,6 @@ test("unit_tests") { ...@@ -3009,7 +3009,6 @@ test("unit_tests") {
"../browser/android/mock_location_settings.cc", "../browser/android/mock_location_settings.cc",
"../browser/android/mock_location_settings.h", "../browser/android/mock_location_settings.h",
"../browser/android/net/external_estimate_provider_android_unittest.cc", "../browser/android/net/external_estimate_provider_android_unittest.cc",
"../browser/android/preferences/important_sites_util_unittest.cc",
"../browser/android/preferences/pref_service_bridge_unittest.cc", "../browser/android/preferences/pref_service_bridge_unittest.cc",
"../browser/android/shortcut_info_unittest.cc", "../browser/android/shortcut_info_unittest.cc",
"../browser/android/thumbnail/scoped_ptr_expiring_cache_unittest.cc", "../browser/android/thumbnail/scoped_ptr_expiring_cache_unittest.cc",
...@@ -3089,6 +3088,7 @@ test("unit_tests") { ...@@ -3089,6 +3088,7 @@ test("unit_tests") {
"../browser/download/download_status_updater_unittest.cc", "../browser/download/download_status_updater_unittest.cc",
"../browser/download/download_target_determiner_unittest.cc", "../browser/download/download_target_determiner_unittest.cc",
"../browser/download/download_ui_controller_unittest.cc", "../browser/download/download_ui_controller_unittest.cc",
"../browser/engagement/important_sites_util_unittest.cc",
"../browser/engagement/site_engagement_eviction_policy_unittest.cc", "../browser/engagement/site_engagement_eviction_policy_unittest.cc",
"../browser/engagement/site_engagement_helper_unittest.cc", "../browser/engagement/site_engagement_helper_unittest.cc",
"../browser/engagement/site_engagement_score_unittest.cc", "../browser/engagement/site_engagement_score_unittest.cc",
......
...@@ -150,7 +150,8 @@ void WebsiteSettingsRegistry::Init() { ...@@ -150,7 +150,8 @@ void WebsiteSettingsRegistry::Init() {
WebsiteSettingsInfo::INHERIT_IN_INCOGNITO); WebsiteSettingsInfo::INHERIT_IN_INCOGNITO);
Register(CONTENT_SETTINGS_TYPE_IMPORTANT_SITE_INFO, "important-site-info", Register(CONTENT_SETTINGS_TYPE_IMPORTANT_SITE_INFO, "important-site-info",
nullptr, WebsiteSettingsInfo::UNSYNCABLE, WebsiteSettingsInfo::LOSSY, nullptr, WebsiteSettingsInfo::UNSYNCABLE, WebsiteSettingsInfo::LOSSY,
WebsiteSettingsInfo::REQUESTING_ORIGIN_ONLY_SCOPE, PLATFORM_ANDROID, WebsiteSettingsInfo::REQUESTING_ORIGIN_ONLY_SCOPE,
DESKTOP | PLATFORM_ANDROID,
WebsiteSettingsInfo::INHERIT_IN_INCOGNITO); WebsiteSettingsInfo::INHERIT_IN_INCOGNITO);
} }
......
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