pageinfo: don't update cookie count after page load

Updating the cookie count every time it changes is extremely expensive (each
update causes a relayout of the page info dialog) and, worse, causes the dialog
to be unusable to screen readers. In the extreme case, JS-heavy websites cause
this dialog to be read as endless repetitions of "Cookies" as the screen reader
repeatedly cuts itself off while trying to read the changing text.

This change makes the dialog text not update; other possibilities considered
were:
* Removing the text altogether
* Collapsing the state down to "cookies in use" or "no cookies in use"

I feel that this change is less intrusive and less of a design change to the
dialog than either of those two, but one of them (especially the first, IMO) is
probably a good direction to go later.

To test this change, try https://www.washingtonpost.com: navigate to the site
with a cold cache, open the page info dialog, and watch the text change rapidly.

Bug: 984651
Change-Id: If7f996470a1768a7f9acadc72fcf13cec41a73a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1853112Reviewed-by: Emily Stark <estark@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705049}

chrome/browser/ui/page_info/page_info.cc

@@ -327,9 +327,7 @@ PageInfo::PageInfo(
     const GURL& url,
     security_state::SecurityLevel security_level,
     const security_state::VisibleSecurityState& visible_security_state)
-    : TabSpecificContentSettings::SiteDataObserver(
-          tab_specific_content_settings),
-      content::WebContentsObserver(web_contents),
+    : content::WebContentsObserver(web_contents),
       ui_(ui),
       show_info_bar_(false),
       site_url_(url),
@@ -341,6 +339,7 @@ PageInfo::PageInfo(
       content_settings_(HostContentSettingsMapFactory::GetForProfile(profile)),
       chrome_ssl_host_state_delegate_(
           ChromeSSLHostStateDelegateFactory::GetForProfile(profile)),
+      tab_specific_content_settings_(tab_specific_content_settings),
       did_revoke_user_ssl_decisions_(false),
       profile_(profile),
       security_level_(security_state::NONE),
@@ -471,7 +470,7 @@ void PageInfo::RecordPageInfoAction(PageInfoAction action) {
 
 void PageInfo::OnSitePermissionChanged(ContentSettingsType type,
                                        ContentSetting setting) {
-  tab_specific_content_settings()->ContentSettingChangedViaPageInfo(type);
+  tab_specific_content_settings_->ContentSettingChangedViaPageInfo(type);
 
   // Count how often a permission for a specific content type is changed using
   // the Page Info UI.
@@ -548,10 +547,6 @@ void PageInfo::OnSiteChosenObjectDeleted(const ChooserUIInfo& ui_info,
   PresentSitePermissions();
 }
 
-void PageInfo::OnSiteDataAccessed() {
-  PresentSiteData();
-}
-
 void PageInfo::OnUIClosing(bool* reload_prompt) {
   if (reload_prompt)
     *reload_prompt = false;
@@ -945,7 +940,7 @@ void PageInfo::PresentSitePermissions() {
     }
 
     if (ShouldShowPermission(permission_info, site_url_, content_settings_,
-                             web_contents(), tab_specific_content_settings())) {
+                             web_contents(), tab_specific_content_settings_)) {
       permission_info_list.push_back(permission_info);
     }
   }
@@ -968,9 +963,9 @@ void PageInfo::PresentSitePermissions() {
 void PageInfo::PresentSiteData() {
   CookieInfoList cookie_info_list;
   const LocalSharedObjectsContainer& allowed_objects =
-      tab_specific_content_settings()->allowed_local_shared_objects();
+      tab_specific_content_settings_->allowed_local_shared_objects();
   const LocalSharedObjectsContainer& blocked_objects =
-      tab_specific_content_settings()->blocked_local_shared_objects();
+      tab_specific_content_settings_->blocked_local_shared_objects();
 
   // Add first party cookie and site data counts.
   PageInfoUI::CookieInfo cookie_info;

chrome/browser/ui/page_info/page_info.h

@@ -46,8 +46,7 @@ using password_manager::metrics_util::PasswordType;
 // information and allows users to change the permissions. |PageInfo|
 // objects must be created on the heap. They destroy themselves after the UI is
 // closed.
-class PageInfo : public TabSpecificContentSettings::SiteDataObserver,
-                 public content::WebContentsObserver {
+class PageInfo : public content::WebContentsObserver {
  public:
   // TODO(palmer): Figure out if it is possible to unify SiteConnectionStatus
   // and SiteIdentityStatus.
@@ -211,9 +210,6 @@ class PageInfo : public TabSpecificContentSettings::SiteDataObserver,
 
   const base::string16& organization_name() const { return organization_name_; }
 
-  // SiteDataObserver implementation.
-  void OnSiteDataAccessed() override;
-
  private:
   FRIEND_TEST_ALL_PREFIXES(PageInfoTest,
                            NonFactoryDefaultAndRecentlyChangedPermissionsShown);
@@ -323,6 +319,12 @@ class PageInfo : public TabSpecificContentSettings::SiteDataObserver,
   // decisions by users.
   ChromeSSLHostStateDelegate* chrome_ssl_host_state_delegate_;
 
+  // The TabSpecificContentSettings for this site, used to propagate changes
+  // from the UI back to the model. This is held as a raw pointer because the
+  // lifetime of TabSpecificContentSettings is tightly bound to that of the
+  // observed WebContents.
+  TabSpecificContentSettings* tab_specific_content_settings_;
+
   bool did_revoke_user_ssl_decisions_;
 
   Profile* profile_;

chrome/browser/ui/page_info/page_info_unittest.cc

@@ -399,14 +399,6 @@ TEST_F(PageInfoTest, OnPermissionsChanged) {
   EXPECT_EQ(setting, CONTENT_SETTING_ALLOW);
 }
 
-TEST_F(PageInfoTest, OnSiteDataAccessed) {
-  EXPECT_CALL(*mock_ui(), SetPermissionInfoStub());
-  EXPECT_CALL(*mock_ui(), SetIdentityInfo(_));
-  EXPECT_CALL(*mock_ui(), SetCookieInfo(_)).Times(2);
-
-  page_info()->OnSiteDataAccessed();
-}
-
 TEST_F(PageInfoTest, OnChosenObjectDeleted) {
   // Connect the UsbChooserContext with FakeUsbDeviceManager.
   device::FakeUsbDeviceManager usb_device_manager;