Hide ASK settings from PageInfo in incognito mode

When a user grants a permission in regular mode, we replace the ALLOW
content settings with an ASK setting in incognito mode. These settings
should not be shown in the UI if ASK is the default setting.

The current behavior also fails an assert in
PermissionParamsListBuilder::createPermissionParams, which is fixed
by this CL.

Bug: 1092809
Change-Id: Ic74f77cca11b6d3ff0839e6f908002cf76838c21
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2235822Reviewed-by: Balazs Engedy <engedy@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#781249}

chrome/browser/ui/page_info/page_info_unittest.cc

@@ -41,6 +41,7 @@
 #include "content/public/browser/ssl_host_state_delegate.h"
 #include "content/public/browser/ssl_status.h"
 #include "content/public/common/content_switches.h"
+#include "content/public/test/web_contents_tester.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "net/cert/cert_status_flags.h"
 #include "net/cert/x509_certificate.h"
@@ -158,9 +159,12 @@ class PageInfoTest : public ChromeRenderViewHostTestHarness {
   }
 
   void TearDown() override {
-    ASSERT_TRUE(page_info_.get()) << "No PageInfo instance created.";
+    ASSERT_TRUE(page_info_ || incognito_page_info_)
+        << "No PageInfo instance created.";
+    incognito_web_contents_.reset();
     RenderViewHostTestHarness::TearDown();
     page_info_.reset();
+    incognito_page_info_.reset();
   }
 
   void SetDefaultUIExpectations(MockPageInfoUI* mock_ui) {
@@ -227,12 +231,43 @@ class PageInfoTest : public ChromeRenderViewHostTestHarness {
     return page_info_.get();
   }
 
+  PageInfo* incognito_page_info() {
+    if (!incognito_page_info_.get()) {
+      incognito_web_contents_ =
+          content::WebContentsTester::CreateTestWebContents(
+              profile()->GetPrimaryOTRProfile(), nullptr);
+
+      content_settings::TabSpecificContentSettings::CreateForWebContents(
+          incognito_web_contents_.get(),
+          std::make_unique<chrome::TabSpecificContentSettingsDelegate>(
+              incognito_web_contents_.get()));
+
+      incognito_mock_ui_ = std::make_unique<MockPageInfoUI>();
+      incognito_mock_ui_->set_permission_info_callback_ =
+          base::Bind(&PageInfoTest::SetPermissionInfo, base::Unretained(this));
+
+      auto delegate = std::make_unique<ChromePageInfoDelegate>(
+          incognito_web_contents_.get());
+      delegate->SetSecurityStateForTests(security_level_,
+                                         visible_security_state_);
+      incognito_page_info_ = std::make_unique<PageInfo>(
+          std::move(delegate), incognito_web_contents_.get(), url());
+      incognito_page_info_->InitializeUiState(incognito_mock_ui_.get());
+    }
+    return incognito_page_info_.get();
+  }
+
   security_state::SecurityLevel security_level_;
   security_state::VisibleSecurityState visible_security_state_;
 
  private:
   std::unique_ptr<PageInfo> page_info_;
   std::unique_ptr<MockPageInfoUI> mock_ui_;
+
+  std::unique_ptr<content::WebContents> incognito_web_contents_;
+  std::unique_ptr<PageInfo> incognito_page_info_;
+  std::unique_ptr<MockPageInfoUI> incognito_mock_ui_;
+
   scoped_refptr<net::X509Certificate> cert_;
   GURL url_;
   url::Origin origin_;
@@ -250,6 +285,16 @@ bool PermissionInfoListContainsPermission(const PermissionInfoList& permissions,
   return false;
 }
 
+void ExpectPermissionInfoList(
+    const std::set<ContentSettingsType>& expected_permissions,
+    const PermissionInfoList& permissions) {
+  EXPECT_EQ(expected_permissions.size(), permissions.size());
+  for (ContentSettingsType type : expected_permissions) {
+    EXPECT_TRUE(PermissionInfoListContainsPermission(permissions, type))
+        << "expected: " << static_cast<int>(type);
+  }
+}
+
 }  // namespace
 
 TEST_F(PageInfoTest, NonFactoryDefaultAndRecentlyChangedPermissionsShown) {
@@ -262,8 +307,8 @@ TEST_F(PageInfoTest, NonFactoryDefaultAndRecentlyChangedPermissionsShown) {
   // on because this test isn't testing with a default search engine origin.
   expected_visible_permissions.insert(ContentSettingsType::GEOLOCATION);
 #endif
-  EXPECT_EQ(expected_visible_permissions.size(),
-            last_permission_info_list().size());
+  ExpectPermissionInfoList(expected_visible_permissions,
+                           last_permission_info_list());
 
   // Change some default-ask settings away from the default.
   page_info()->OnSitePermissionChanged(ContentSettingsType::GEOLOCATION,
@@ -275,56 +320,99 @@ TEST_F(PageInfoTest, NonFactoryDefaultAndRecentlyChangedPermissionsShown) {
   page_info()->OnSitePermissionChanged(ContentSettingsType::MEDIASTREAM_MIC,
                                        CONTENT_SETTING_ALLOW);
   expected_visible_permissions.insert(ContentSettingsType::MEDIASTREAM_MIC);
-  EXPECT_EQ(expected_visible_permissions.size(),
-            last_permission_info_list().size());
+  ExpectPermissionInfoList(expected_visible_permissions,
+                           last_permission_info_list());
 
   expected_visible_permissions.insert(ContentSettingsType::POPUPS);
   // Change a default-block setting to a user-preference block instead.
   page_info()->OnSitePermissionChanged(ContentSettingsType::POPUPS,
                                        CONTENT_SETTING_BLOCK);
-  EXPECT_EQ(expected_visible_permissions.size(),
-            last_permission_info_list().size());
+  ExpectPermissionInfoList(expected_visible_permissions,
+                           last_permission_info_list());
 
   expected_visible_permissions.insert(ContentSettingsType::JAVASCRIPT);
   // Change a default-allow setting away from the default.
   page_info()->OnSitePermissionChanged(ContentSettingsType::JAVASCRIPT,
                                        CONTENT_SETTING_BLOCK);
-  EXPECT_EQ(expected_visible_permissions.size(),
-            last_permission_info_list().size());
+  ExpectPermissionInfoList(expected_visible_permissions,
+                           last_permission_info_list());
 
   // Make sure changing a setting to its default causes it to show up, since it
   // has been recently changed.
   expected_visible_permissions.insert(ContentSettingsType::MEDIASTREAM_CAMERA);
   page_info()->OnSitePermissionChanged(ContentSettingsType::MEDIASTREAM_CAMERA,
                                        CONTENT_SETTING_DEFAULT);
-  EXPECT_EQ(expected_visible_permissions.size(),
-            last_permission_info_list().size());
+  ExpectPermissionInfoList(expected_visible_permissions,
+                           last_permission_info_list());
 
   // Set the Javascript setting to default should keep it shown.
   page_info()->OnSitePermissionChanged(ContentSettingsType::JAVASCRIPT,
                                        CONTENT_SETTING_DEFAULT);
-  EXPECT_EQ(expected_visible_permissions.size(),
-            last_permission_info_list().size());
+  ExpectPermissionInfoList(expected_visible_permissions,
+                           last_permission_info_list());
 
   // Change the default setting for Javascript away from the factory default.
   page_info()->GetContentSettings()->SetDefaultContentSetting(
       ContentSettingsType::JAVASCRIPT, CONTENT_SETTING_BLOCK);
   page_info()->PresentSitePermissions();
-  EXPECT_EQ(expected_visible_permissions.size(),
-            last_permission_info_list().size());
+  ExpectPermissionInfoList(expected_visible_permissions,
+                           last_permission_info_list());
 
   // Change it back to ALLOW, which is its factory default, but has a source
   // from the user preference (i.e. it counts as non-factory default).
   page_info()->OnSitePermissionChanged(ContentSettingsType::JAVASCRIPT,
                                        CONTENT_SETTING_ALLOW);
-  EXPECT_EQ(expected_visible_permissions.size(),
-            last_permission_info_list().size());
+  ExpectPermissionInfoList(expected_visible_permissions,
+                           last_permission_info_list());
+}
 
-  // Sanity check the correct permissions are being shown.
-  for (ContentSettingsType type : expected_visible_permissions) {
-    EXPECT_TRUE(PermissionInfoListContainsPermission(
-        last_permission_info_list(), type));
-  }
+TEST_F(PageInfoTest, IncognitoPermissionsEmptyByDefault) {
+  incognito_page_info()->PresentSitePermissions();
+  EXPECT_EQ(0u, last_permission_info_list().size());
+}
+
+TEST_F(PageInfoTest, IncognitoPermissionsDontShowAsk) {
+  page_info()->PresentSitePermissions();
+  std::set<ContentSettingsType> expected_permissions;
+  std::set<ContentSettingsType> expected_incognito_permissions;
+#if defined(OS_ANDROID)
+  // Geolocation is always allowed to pass through to Android-specific logic to
+  // check for DSE settings (so expect 1 item), but isn't actually shown later
+  // on because this test isn't testing with a default search engine origin.
+  expected_permissions.insert(ContentSettingsType::GEOLOCATION);
+#endif
+  ExpectPermissionInfoList(expected_permissions, last_permission_info_list());
+
+  // Add some permissions to regular page info.
+  page_info()->OnSitePermissionChanged(ContentSettingsType::GEOLOCATION,
+                                       CONTENT_SETTING_ALLOW);
+
+  page_info()->OnSitePermissionChanged(ContentSettingsType::MEDIASTREAM_MIC,
+                                       CONTENT_SETTING_BLOCK);
+  expected_permissions.insert(ContentSettingsType::MEDIASTREAM_MIC);
+  expected_incognito_permissions.insert(ContentSettingsType::MEDIASTREAM_MIC);
+
+  // Both permissions should show in regular page info.
+  EXPECT_EQ(2u, last_permission_info_list().size());
+
+  // Only the block permissions should show in incognito mode as ALLOW
+  // permissions are inherited as ASK.
+  incognito_page_info()->PresentSitePermissions();
+  ExpectPermissionInfoList(expected_incognito_permissions,
+                           last_permission_info_list());
+
+  // Changing the permission to BLOCK should show it.
+  incognito_page_info()->OnSitePermissionChanged(
+      ContentSettingsType::GEOLOCATION, CONTENT_SETTING_BLOCK);
+  expected_incognito_permissions.insert(ContentSettingsType::GEOLOCATION);
+  ExpectPermissionInfoList(expected_incognito_permissions,
+                           last_permission_info_list());
+
+  // Switching a permission back to default should not hide the permission.
+  incognito_page_info()->OnSitePermissionChanged(
+      ContentSettingsType::GEOLOCATION, CONTENT_SETTING_DEFAULT);
+  ExpectPermissionInfoList(expected_incognito_permissions,
+                           last_permission_info_list());
 }
 
 TEST_F(PageInfoTest, OnPermissionsChanged) {

components/page_info/page_info.cc

@@ -136,9 +136,16 @@ bool IsPermissionFactoryDefault(HostContentSettingsMap* content_settings,
       content_settings::ContentSettingsRegistry::GetInstance()
           ->Get(info.type)
           ->GetInitialDefaultSetting();
-  return (info.source == content_settings::SETTING_SOURCE_USER &&
-          factory_default_setting == info.default_setting &&
-          info.setting == CONTENT_SETTING_DEFAULT);
+
+  // Settings that are granted in regular mode get reduced to ASK in incognito
+  // mode. These settings should not be displayed either.
+  const bool is_incognito_default =
+      info.is_incognito && info.setting == CONTENT_SETTING_ASK &&
+      factory_default_setting == CONTENT_SETTING_ASK;
+
+  return info.source == content_settings::SETTING_SOURCE_USER &&
+         factory_default_setting == info.default_setting &&
+         (info.setting == CONTENT_SETTING_DEFAULT || is_incognito_default);
 }
 
 // Determines whether to show permission |type| in the Page Info UI. Only
@@ -173,7 +180,8 @@ bool ShouldShowPermission(const PageInfoUI::PermissionInfo& info,
 #if defined(OS_ANDROID)
   // Special geolocation DSE settings apply only on Android, so make sure it
   // gets checked there regardless of default setting on Desktop.
-  if (info.type == ContentSettingsType::GEOLOCATION)
+  // DSE settings don't apply to incognito mode.
+  if (info.type == ContentSettingsType::GEOLOCATION && !info.is_incognito)
     return true;
 
   // The Native File System write permission is desktop only at the moment.
@@ -425,6 +433,8 @@ PageInfo::~PageInfo() {
 void PageInfo::InitializeUiState(PageInfoUI* ui) {
   ui_ = ui;
   DCHECK(ui_);
+  // TabSpecificContentSetting needs to be created before page load.
+  DCHECK(GetTabSpecificContentSettings());
 
   ComputeUIInputs(site_url_);
   PresentSitePermissions();
@@ -909,8 +919,8 @@ void PageInfo::PresentSitePermissions() {
 
   PageInfoUI::PermissionInfo permission_info;
   HostContentSettingsMap* content_settings = GetContentSettings();
-  for (size_t i = 0; i < base::size(kPermissionType); ++i) {
-    permission_info.type = kPermissionType[i];
+  for (const ContentSettingsType type : kPermissionType) {
+    permission_info.type = type;
 
     content_settings::SettingInfo info;
 
@@ -937,7 +947,7 @@ void PageInfo::PresentSitePermissions() {
     } else {
       permission_info.default_setting =
           content_settings->GetDefaultContentSetting(permission_info.type,
-                                                     NULL);
+                                                     nullptr);
     }
 
     // For permissions that are still prompting the user and haven't been
@@ -1060,8 +1070,8 @@ HostContentSettingsMap* PageInfo::GetContentSettings() const {
 
 std::vector<ContentSettingsType> PageInfo::GetAllPermissionsForTesting() {
   std::vector<ContentSettingsType> permission_list;
-  for (size_t i = 0; i < base::size(kPermissionType); ++i)
-    permission_list.push_back(kPermissionType[i]);
+  for (const ContentSettingsType type : kPermissionType)
+    permission_list.push_back(type);
 
   return permission_list;
 }

components/page_info/page_info.h

@@ -217,6 +217,8 @@ class PageInfo : public content::WebContentsObserver {
  private:
   FRIEND_TEST_ALL_PREFIXES(PageInfoTest,
                            NonFactoryDefaultAndRecentlyChangedPermissionsShown);
+  FRIEND_TEST_ALL_PREFIXES(PageInfoTest, IncognitoPermissionsEmptyByDefault);
+  FRIEND_TEST_ALL_PREFIXES(PageInfoTest, IncognitoPermissionsDontShowAsk);
   friend class PageInfoBubbleViewBrowserTest;
   // Populates this object's UI state with provided security context. This
   // function does not update visible UI-- that's part of Present*().