Settings: All Sites now includes sites using local storage.

Currently, the All Sites list only includes sites with non-default content
settings. Update it to include sites that use local storage (disk space) as
well.

Bug: 835712
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: I722c659b551aaf5023bb82a7475151df39a092be
Reviewed-on: https://chromium-review.googlesource.com/1137812Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Reviewed-by: Hector Carmona <hcarmona@chromium.org>
Commit-Queue: Patti <patricialor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579276}

chrome/browser/browsing_data/mock_browsing_data_local_storage_helper.cc

@@ -37,14 +37,16 @@ void MockBrowsingDataLocalStorageHelper::DeleteOrigin(
 void MockBrowsingDataLocalStorageHelper::AddLocalStorageSamples() {
   const GURL kOrigin1("http://host1:1/");
   const GURL kOrigin2("http://host2:2/");
-  response_.push_back(
-      BrowsingDataLocalStorageHelper::LocalStorageInfo(
-          kOrigin1, 1, base::Time()));
-  origins_[kOrigin1] = true;
-  response_.push_back(
-      BrowsingDataLocalStorageHelper::LocalStorageInfo(
-          kOrigin2, 2, base::Time()));
-  origins_[kOrigin2] = true;
+  AddLocalStorageForOrigin(kOrigin1, 1);
+  AddLocalStorageForOrigin(kOrigin2, 2);
+}
+
+void MockBrowsingDataLocalStorageHelper::AddLocalStorageForOrigin(
+    const GURL& origin,
+    size_t size) {
+  response_.push_back(BrowsingDataLocalStorageHelper::LocalStorageInfo(
+      origin, size, base::Time()));
+  origins_[origin] = true;
 }
 
 void MockBrowsingDataLocalStorageHelper::Notify() {

chrome/browser/browsing_data/mock_browsing_data_local_storage_helper.h

@@ -27,6 +27,9 @@ class MockBrowsingDataLocalStorageHelper
   // Adds some LocalStorageInfo samples.
   void AddLocalStorageSamples();
 
+  // Add a LocalStorageInfo entry for a single origin.
+  void AddLocalStorageForOrigin(const GURL& origin, size_t size);
+
   // Notifies the callback.
   void Notify();
 

chrome/browser/resources/settings/site_settings/all_sites.html

@@ -48,12 +48,12 @@
         </select>
       </div>
     </div>
-    <div class="list-frame" hidden$="[[siteGroupList.length]]">
+    <div class="list-frame" hidden$="[[siteGroupMap.size]]">
       <div class="list-item secondary">$i18n{noSitesAdded}</div>
     </div>
     <div class="list-frame without-heading" id="listContainer">
       <iron-list id="allSitesList"
-          items="[[filterPopulatedList_(siteGroupList, searchQuery_)]]"
+          items="[[filterPopulatedList_(siteGroupMap, searchQuery_)]]"
           scroll-target="[[subpageScrollTarget]]">
         <template>
           <site-entry site-group="[[item]]" list-index="[[index]]"

chrome/browser/resources/settings/site_settings/all_sites.js

@@ -19,13 +19,14 @@ Polymer({
 
   properties: {
     /**
-     * Array of sites to display in the widget, grouped into their eTLD+1s.
-     * @type {!Array<!SiteGroup>}
+     * Map containing sites to display in the widget, grouped into their
+     * eTLD+1 names.
+     * @type {!Map<string, !SiteGroup>}
      */
-    siteGroupList: {
-      type: Array,
+    siteGroupMap: {
+      type: Object,
       value: function() {
-        return [];
+        return new Map();
       },
     },
 
@@ -51,17 +52,15 @@ Polymer({
 
     /**
      * All possible sort methods.
-     * @type {Object}
+     * @type {!{name: string, mostVisited: string, storage: string}}
      * @private
      */
     sortMethods_: {
       type: Object,
-      value: function() {
-        return {
-          name: 'name',
-          mostVisited: 'most-visited',
-          storage: 'data-stored',
-        };
+      value: {
+        name: 'name',
+        mostVisited: 'most-visited',
+        storage: 'data-stored',
       },
       readOnly: true,
     },
@@ -87,6 +86,8 @@ Polymer({
   ready: function() {
     this.browserProxy_ =
         settings.SiteSettingsPrefsBrowserProxyImpl.getInstance();
+    this.addWebUIListener(
+        'onLocalStorageListFetched', this.onLocalStorageListFetched.bind(this));
     this.addWebUIListener(
         'contentSettingSitePermissionChanged', this.populateList_.bind(this));
     this.addEventListener(
@@ -122,26 +123,47 @@ Polymer({
       contentTypes.push(settings.ContentSettingsTypes.COOKIES);
 
     this.browserProxy_.getAllSites(contentTypes).then((response) => {
-      this.siteGroupList = this.sortSiteGroupList_(response);
+      response.forEach(siteGroup => {
+        this.siteGroupMap.set(siteGroup.etldPlus1, siteGroup);
+      });
+      this.forceListUpdate_();
+    });
+  },
+
+  /**
+   * Integrate sites using local storage into the existing sites map, as there
+   * may be overlap between the existing sites.
+   * @param {!Array<!SiteGroup>} list The list of sites using local storage.
+   */
+  onLocalStorageListFetched: function(list) {
+    list.forEach(storageSiteGroup => {
+      if (this.siteGroupMap.has(storageSiteGroup.etldPlus1)) {
+        const siteGroup = this.siteGroupMap.get(storageSiteGroup.etldPlus1);
+        storageSiteGroup.origins.forEach(origin => {
+          if (!siteGroup.origins.includes(origin))
+            siteGroup.origins.push(origin);
+        });
+      } else {
+        this.siteGroupMap.set(storageSiteGroup.etldPlus1, storageSiteGroup);
+      }
     });
+    this.forceListUpdate_();
   },
 
   /**
-   * Filters |this.siteGroupList| with the given search query text.
-   * @param {!Array<!SiteGroup>} siteGroupList The list of sites to filter.
+   * Filters the all sites list with the given search query text.
+   * @param {!Map<string, !SiteGroup>} siteGroupMap The map of sites to filter.
    * @param {string} searchQuery The filter text.
    * @return {!Array<!SiteGroup>}
    * @private
    */
-  filterPopulatedList_: function(siteGroupList, searchQuery) {
-    if (searchQuery.length == 0)
-      return siteGroupList;
-
-    return siteGroupList.filter((siteGroup) => {
-      return siteGroup.origins.find(origin => {
-        return origin.includes(searchQuery);
-      });
-    });
+  filterPopulatedList_: function(siteGroupMap, searchQuery) {
+    const result = [];
+    for (const [etldPlus1, siteGroup] of siteGroupMap) {
+      if (siteGroup.origins.find(origin => origin.includes(searchQuery)))
+        result.push(siteGroup);
+    }
+    return this.sortSiteGroupList_(result);
   },
 
   /**
@@ -152,7 +174,7 @@ Polymer({
    */
   sortSiteGroupList_: function(siteGroupList) {
     const sortMethod = this.$.sortMethod.value;
-    if (sortMethod == this.sortMethods_.name)
+    if (this.sortMethods_ && sortMethod == this.sortMethods_.name)
       siteGroupList.sort(this.nameComparator_);
     return siteGroupList;
   },
@@ -191,6 +213,17 @@ Polymer({
       this.$.allSitesList.fire('iron-resize');
   },
 
+  /**
+   * Forces the all sites list to update its list of items, taking into account
+   * the search query and the sort method, then re-renders it.
+   * @private
+   */
+  forceListUpdate_: function() {
+    this.$.allSitesList.items =
+        this.filterPopulatedList_(this.siteGroupMap, this.searchQuery_);
+    this.$.allSitesList.fire('iron-resize');
+  },
+
   /**
    * @param {!Map<string, (string|Function)>} newConfig
    * @param {?Map<string, (string|Function)>} oldConfig

chrome/browser/ui/webui/settings/site_settings_handler.cc

@@ -17,7 +17,6 @@
 #include "base/macros.h"
 #include "base/metrics/user_metrics.h"
 #include "base/values.h"
-#include "chrome/browser/browsing_data/browsing_data_local_storage_helper.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/content_settings/web_site_settings_uma_util.h"
@@ -149,12 +148,28 @@ void CreateOrAppendSiteGroupEntry(
   }
 }
 
-}  // namespace
+// Converts a given |site_group_map| to a list of base::DictionaryValues.
+void ConvertSiteGroupMapToListValue(
+    const std::map<std::string, std::set<std::string>>& site_group_map,
+    base::Value* list_value) {
+  DCHECK_EQ(base::Value::Type::LIST, list_value->type());
+  for (const auto& entry : site_group_map) {
+    // eTLD+1 is the effective top level domain + 1.
+    base::Value site_group(base::Value::Type::DICTIONARY);
+    site_group.SetKey(kEffectiveTopLevelDomainPlus1Name,
+                      base::Value(entry.first));
+    base::Value origin_list(base::Value::Type::LIST);
+    for (const std::string& origin : entry.second)
+      origin_list.GetList().emplace_back(base::Value(origin));
+    site_group.SetKey(kOriginList, std::move(origin_list));
+    list_value->GetList().push_back(std::move(site_group));
+  }
+}
 
+}  // namespace
 
 SiteSettingsHandler::SiteSettingsHandler(Profile* profile)
-    : profile_(profile), observer_(this) {
-}
+    : profile_(profile), observer_(this), local_storage_helper_(nullptr) {}
 
 SiteSettingsHandler::~SiteSettingsHandler() {
 }
@@ -425,9 +440,7 @@ void SiteSettingsHandler::HandleClearUsage(
                             base::Unretained(this), barrier));
 
     // Also clear the *local* storage data.
-    scoped_refptr<BrowsingDataLocalStorageHelper> local_storage_helper =
-        new BrowsingDataLocalStorageHelper(profile_);
-    local_storage_helper->DeleteOrigin(url, barrier);
+    GetLocalStorageHelper()->DeleteOrigin(url, barrier);
   }
 }
 
@@ -560,6 +573,10 @@ void SiteSettingsHandler::HandleGetAllSites(const base::ListValue* args) {
   // TODO(https://crbug.com/835712): Assess performance of this method for
   // unusually large numbers of stored content settings.
 
+  // Add sites that are using any local storage to the list.
+  GetLocalStorageHelper()->StartFetching(base::BindRepeating(
+      &SiteSettingsHandler::OnLocalStorageFetched, base::Unretained(this)));
+
   // Retrieve a list of embargoed settings to check separately. This ensures
   // that only settings included in |content_types| will be listed in all sites.
   ContentSettingsForOneType embargo_settings;
@@ -584,9 +601,6 @@ void SiteSettingsHandler::HandleGetAllSites(const base::ListValue* args) {
 
   // Convert |types| to a list of ContentSettingsTypes.
   for (ContentSettingsType content_type : content_types) {
-    // TODO(https://crbug.com/835712): Add extension content settings, plus
-    // sites that use any non-zero amount of storage.
-
     ContentSettingsForOneType entries;
     map->GetSettingsForOneType(content_type, std::string(), &entries);
     for (const ContentSettingPatternSource& e : entries) {
@@ -596,23 +610,24 @@ void SiteSettingsHandler::HandleGetAllSites(const base::ListValue* args) {
     }
   }
 
-  // Convert |all_sites_map| to a list of base::DictionaryValues.
   base::Value result(base::Value::Type::LIST);
-  for (const auto& entry : all_sites_map) {
-    // eTLD+1 is the effective top level domain + 1.
-    base::Value site_group(base::Value::Type::DICTIONARY);
-    site_group.SetKey(kEffectiveTopLevelDomainPlus1Name,
-                      base::Value(entry.first));
-    base::Value origin_list(base::Value::Type::LIST);
-    for (const std::string& origin : entry.second) {
-      origin_list.GetList().emplace_back(origin);
-    }
-    site_group.SetKey(kOriginList, std::move(origin_list));
-    result.GetList().push_back(std::move(site_group));
-  }
+  ConvertSiteGroupMapToListValue(all_sites_map, &result);
   ResolveJavascriptCallback(*callback_id, result);
 }
 
+void SiteSettingsHandler::OnLocalStorageFetched(
+    const std::list<BrowsingDataLocalStorageHelper::LocalStorageInfo>&
+        local_storage_info) {
+  std::map<std::string, std::set<std::string>> all_sites_map;
+  for (const BrowsingDataLocalStorageHelper::LocalStorageInfo& info :
+       local_storage_info) {
+    CreateOrAppendSiteGroupEntry(&all_sites_map, info.origin_url);
+  }
+  base::Value result(base::Value::Type::LIST);
+  ConvertSiteGroupMapToListValue(all_sites_map, &result);
+  FireWebUIListener("onLocalStorageListFetched", std::move(result));
+}
+
 void SiteSettingsHandler::HandleGetExceptionList(const base::ListValue* args) {
   AllowJavascript();
 
@@ -1053,4 +1068,16 @@ void SiteSettingsHandler::HandleRemoveZoomLevel(const base::ListValue* args) {
   host_zoom_map->SetZoomLevelForHost(origin, default_level);
 }
 
+void SiteSettingsHandler::SetBrowsingDataLocalStorageHelperForTesting(
+    scoped_refptr<BrowsingDataLocalStorageHelper> helper) {
+  DCHECK(!local_storage_helper_);
+  local_storage_helper_ = helper;
+}
+
+BrowsingDataLocalStorageHelper* SiteSettingsHandler::GetLocalStorageHelper() {
+  if (!local_storage_helper_)
+    local_storage_helper_ = new BrowsingDataLocalStorageHelper(profile_);
+  return local_storage_helper_.get();
+}
+
 }  // namespace settings

chrome/browser/ui/webui/settings/site_settings_handler.h

@@ -10,6 +10,7 @@
 #include <vector>
 
 #include "base/scoped_observer.h"
+#include "chrome/browser/browsing_data/browsing_data_local_storage_helper.h"
 #include "chrome/browser/storage/storage_info_fetcher.h"
 #include "chrome/browser/ui/webui/settings/settings_page_ui_handler.h"
 #include "components/content_settings/core/browser/content_settings_observer.h"
@@ -85,6 +86,7 @@ class SiteSettingsHandler : public SettingsPageUIHandler,
   FRIEND_TEST_ALL_PREFIXES(SiteSettingsHandlerTest, GetAndSetForInvalidURLs);
   FRIEND_TEST_ALL_PREFIXES(SiteSettingsHandlerTest, Incognito);
   FRIEND_TEST_ALL_PREFIXES(SiteSettingsHandlerTest, GetAllSites);
+  FRIEND_TEST_ALL_PREFIXES(SiteSettingsHandlerTest, GetAllSitesLocalStorage);
   FRIEND_TEST_ALL_PREFIXES(SiteSettingsHandlerTest, Origins);
   FRIEND_TEST_ALL_PREFIXES(SiteSettingsHandlerTest, Patterns);
   FRIEND_TEST_ALL_PREFIXES(SiteSettingsHandlerTest, ZoomLevels);
@@ -113,6 +115,12 @@ class SiteSettingsHandler : public SettingsPageUIHandler,
   // 1, affected by any of the content settings specified in |args|.
   void HandleGetAllSites(const base::ListValue* args);
 
+  // Called when the list of origins using local storage has been fetched, and
+  // sends this list back to the front end.
+  void OnLocalStorageFetched(
+      const std::list<BrowsingDataLocalStorageHelper::LocalStorageInfo>&
+          local_storage_info);
+
   // Returns the list of site exceptions for a given content settings type.
   void HandleGetExceptionList(const base::ListValue* args);
 
@@ -151,6 +159,11 @@ class SiteSettingsHandler : public SettingsPageUIHandler,
   // Removes a particular zoom level for a given host.
   void HandleRemoveZoomLevel(const base::ListValue* args);
 
+  void SetBrowsingDataLocalStorageHelperForTesting(
+      scoped_refptr<BrowsingDataLocalStorageHelper> helper);
+
+  BrowsingDataLocalStorageHelper* GetLocalStorageHelper();
+
   Profile* profile_;
 
   content::NotificationRegistrar notification_registrar_;
@@ -173,6 +186,8 @@ class SiteSettingsHandler : public SettingsPageUIHandler,
   std::unique_ptr<PrefChangeRegistrar> pref_change_registrar_;
 #endif
 
+  scoped_refptr<BrowsingDataLocalStorageHelper> local_storage_helper_;
+
   DISALLOW_COPY_AND_ASSIGN(SiteSettingsHandler);
 };
 

chrome/browser/ui/webui/settings/site_settings_handler_unittest.cc

@@ -9,6 +9,7 @@
 
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/simple_test_clock.h"
+#include "chrome/browser/browsing_data/mock_browsing_data_local_storage_helper.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ -444,6 +445,9 @@ TEST_F(SiteSettingsHandlerTest, GetAllSites) {
   // Test embargoed settings also appear.
   PermissionDecisionAutoBlocker* auto_blocker =
       PermissionDecisionAutoBlocker::GetForProfile(profile());
+  base::SimpleTestClock clock;
+  clock.SetNow(base::Time::Now());
+  auto_blocker->SetClockForTesting(&clock);
   const GURL url4("https://example2.co.uk");
   for (int i = 0; i < 3; ++i) {
     auto_blocker->RecordDismissAndEmbargo(url4,
@@ -465,11 +469,24 @@ TEST_F(SiteSettingsHandlerTest, GetAllSites) {
     EXPECT_EQ(3UL, site_groups.size());
   }
 
-  // Add an expired embargo setting to a) an existing eTLD+1 group and b) a new
-  // eTLD+1 group.
-  base::SimpleTestClock clock;
-  clock.SetNow(base::Time::Now());
-  auto_blocker->SetClockForTesting(&clock);
+  // Check |url4| disappears from the list when its embargo expires.
+  clock.Advance(base::TimeDelta::FromDays(8));
+  handler()->HandleGetAllSites(&get_all_sites_args);
+
+  {
+    const content::TestWebUI::CallData& data = *web_ui()->call_data().back();
+    EXPECT_EQ("cr.webUIResponse", data.function_name());
+    EXPECT_EQ(kCallbackId, data.arg1()->GetString());
+    ASSERT_TRUE(data.arg2()->GetBool());
+
+    const base::Value::ListStorage& site_groups = data.arg3()->GetList();
+    EXPECT_EQ(2UL, site_groups.size());
+    EXPECT_EQ("example.com", site_groups[0].FindKey("etldPlus1")->GetString());
+    EXPECT_EQ("example2.net", site_groups[1].FindKey("etldPlus1")->GetString());
+  }
+
+  // Add an expired embargo setting to an existing eTLD+1 group and make sure it
+  // still appears.
   for (int i = 0; i < 3; ++i) {
     auto_blocker->RecordDismissAndEmbargo(url3,
                                           CONTENT_SETTINGS_TYPE_NOTIFICATIONS);
@@ -484,6 +501,7 @@ TEST_F(SiteSettingsHandlerTest, GetAllSites) {
       auto_blocker->GetEmbargoResult(url3, CONTENT_SETTINGS_TYPE_NOTIFICATIONS)
           .content_setting);
 
+  handler()->HandleGetAllSites(&get_all_sites_args);
   {
     const content::TestWebUI::CallData& data = *web_ui()->call_data().back();
     EXPECT_EQ("cr.webUIResponse", data.function_name());
@@ -491,10 +509,12 @@ TEST_F(SiteSettingsHandlerTest, GetAllSites) {
     ASSERT_TRUE(data.arg2()->GetBool());
 
     const base::Value::ListStorage& site_groups = data.arg3()->GetList();
-    EXPECT_EQ(3UL, site_groups.size());
+    EXPECT_EQ(2UL, site_groups.size());
+    EXPECT_EQ("example.com", site_groups[0].FindKey("etldPlus1")->GetString());
+    EXPECT_EQ("example2.net", site_groups[1].FindKey("etldPlus1")->GetString());
   }
 
-  clock.SetNow(base::Time::Now());
+  // Add an expired embargo to a new eTLD+1 and make sure it doesn't appear.
   const GURL url5("http://test.example5.com");
   for (int i = 0; i < 3; ++i) {
     auto_blocker->RecordDismissAndEmbargo(url5,
@@ -510,6 +530,7 @@ TEST_F(SiteSettingsHandlerTest, GetAllSites) {
       auto_blocker->GetEmbargoResult(url5, CONTENT_SETTINGS_TYPE_NOTIFICATIONS)
           .content_setting);
 
+  handler()->HandleGetAllSites(&get_all_sites_args);
   {
     const content::TestWebUI::CallData& data = *web_ui()->call_data().back();
     EXPECT_EQ("cr.webUIResponse", data.function_name());
@@ -517,8 +538,69 @@ TEST_F(SiteSettingsHandlerTest, GetAllSites) {
     ASSERT_TRUE(data.arg2()->GetBool());
 
     const base::Value::ListStorage& site_groups = data.arg3()->GetList();
-    EXPECT_EQ(3UL, site_groups.size());
+    EXPECT_EQ(2UL, site_groups.size());
+    EXPECT_EQ("example.com", site_groups[0].FindKey("etldPlus1")->GetString());
+    EXPECT_EQ("example2.net", site_groups[1].FindKey("etldPlus1")->GetString());
   }
+
+  // Each call to HandleGetAllSites() above added a callback to the profile's
+  // BrowsingDataLocalStorageHelper, so make sure these aren't stuck waiting to
+  // run at the end of the test.
+  base::RunLoop run_loop;
+  run_loop.RunUntilIdle();
+}
+
+TEST_F(SiteSettingsHandlerTest, GetAllSitesLocalStorage) {
+  scoped_refptr<MockBrowsingDataLocalStorageHelper>
+      mock_browsing_data_local_storage_helper =
+          new MockBrowsingDataLocalStorageHelper(profile());
+  handler()->SetBrowsingDataLocalStorageHelperForTesting(
+      mock_browsing_data_local_storage_helper);
+
+  // Add local storage for |origin|.
+  const GURL origin("https://example.com:12378");
+  mock_browsing_data_local_storage_helper->AddLocalStorageForOrigin(origin, 1);
+
+  // Check these sites are included in the callback.
+  base::ListValue get_all_sites_args;
+  get_all_sites_args.AppendString(kCallbackId);
+  base::Value category_list(base::Value::Type::LIST);
+  get_all_sites_args.GetList().push_back(std::move(category_list));
+
+  // Wait for the fetch handler to finish, then check it includes |origin| in
+  // its result.
+  handler()->HandleGetAllSites(&get_all_sites_args);
+  EXPECT_EQ(1U, web_ui()->call_data().size());
+  mock_browsing_data_local_storage_helper->Notify();
+  EXPECT_EQ(2U, web_ui()->call_data().size());
+  base::RunLoop run_loop;
+  run_loop.RunUntilIdle();
+
+  const content::TestWebUI::CallData& data = *web_ui()->call_data().back();
+  EXPECT_EQ("cr.webUIListenerCallback", data.function_name());
+
+  std::string callback_id;
+  ASSERT_TRUE(data.arg1()->GetAsString(&callback_id));
+  EXPECT_EQ("onLocalStorageListFetched", callback_id);
+
+  const base::ListValue* local_storage_list;
+  ASSERT_TRUE(data.arg2()->GetAsList(&local_storage_list));
+  EXPECT_EQ(1U, local_storage_list->GetSize());
+
+  const base::DictionaryValue* site_group;
+  ASSERT_TRUE(local_storage_list->GetDictionary(0, &site_group));
+
+  std::string etld_plus1_string;
+  ASSERT_TRUE(site_group->GetString("etldPlus1", &etld_plus1_string));
+  ASSERT_EQ("example.com", etld_plus1_string);
+
+  const base::ListValue* origin_list;
+  ASSERT_TRUE(site_group->GetList("origins", &origin_list));
+  EXPECT_EQ(1U, origin_list->GetSize());
+
+  std::string actual_origin;
+  ASSERT_TRUE(origin_list->GetString(0, &actual_origin));
+  ASSERT_EQ(origin.spec(), actual_origin);
 }
 
 TEST_F(SiteSettingsHandlerTest, Origins) {

chrome/test/data/webui/settings/all_sites_tests.js

@@ -76,12 +76,7 @@ suite('AllSites', function() {
    */
   function setUpCategory(prefs) {
     browserProxy.setPrefs(prefs);
-    // Some route is needed, but the actual route doesn't matter.
-    testElement.currentRoute = {
-      page: 'dummy',
-      section: 'privacy',
-      subpage: ['site-settings', 'site-settings-category-location'],
-    };
+    settings.navigateTo(settings.routes.SITE_SETTINGS_ALL);
   }
 
   test('All sites list populated', function() {
@@ -92,7 +87,7 @@ suite('AllSites', function() {
       const resolver = new PromiseResolver();
       testElement.async(resolver.resolve);
       return resolver.promise.then(() => {
-        assertEquals(3, testElement.siteGroupList.length);
+        assertEquals(3, testElement.siteGroupMap.size);
 
         // Flush to be sure list container is populated.
         Polymer.dom.flush();
@@ -155,4 +150,47 @@ suite('AllSites', function() {
       assertEquals('google.com', siteEntries[2].$.displayName.innerText.trim());
     });
   });
+
+  test('merging additional SiteGroup lists works', function() {
+    setUpCategory(prefsVarious);
+    testElement.populateList_();
+    return browserProxy.whenCalled('getAllSites').then(() => {
+      Polymer.dom.flush();
+      let siteEntries =
+          testElement.$.listContainer.querySelectorAll('site-entry');
+      assertEquals(3, siteEntries.length);
+
+      // Pretend an additional set of SiteGroups were added.
+      const fooEtldPlus1 = 'foo.com';
+      const addEtldPlus1 = 'additional-site.net';
+      const fooOrigin = 'https://login.foo.com';
+      const addOrigin = 'http://www.additional-site.net';
+      const LOCAL_STORAGE_SITE_GROUP_LIST = /** @type {!Array{!SiteGroup}} */ ([
+        {
+          // Test merging an existing site works, with overlapping origin lists.
+          'etldPlus1': fooEtldPlus1,
+          'origins': [fooOrigin, 'https://foo.com'],
+        },
+        {
+          // Test adding a new site entry works.
+          'etldPlus1': addEtldPlus1,
+          'origins': [addOrigin]
+        }
+      ]);
+      testElement.onLocalStorageListFetched(LOCAL_STORAGE_SITE_GROUP_LIST);
+
+      Polymer.dom.flush();
+      siteEntries = testElement.$.listContainer.querySelectorAll('site-entry');
+      assertEquals(4, siteEntries.length);
+
+      assertEquals(addEtldPlus1, siteEntries[0].siteGroup.etldPlus1);
+      assertEquals(1, siteEntries[0].siteGroup.origins.length);
+      assertEquals(addOrigin, siteEntries[0].siteGroup.origins[0]);
+
+      assertEquals(fooEtldPlus1, siteEntries[2].siteGroup.etldPlus1);
+      assertEquals(2, siteEntries[2].siteGroup.origins.length);
+      assertTrue(siteEntries[2].siteGroup.origins.includes(fooOrigin));
+      assertTrue(siteEntries[2].siteGroup.origins.includes('https://foo.com'));
+    });
+  });
 });