Register granting of previously withheld permissions as a permissions

chrome/browser/extensions/crx_installer_browsertest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/at_exit.h"
 #include "base/memory/ref_counted.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/download/download_crx_util.h"
 #include "chrome/browser/extensions/browser_action_test_util.h"
 #include "chrome/browser/extensions/crx_installer.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/extensions/extension_install_prompt.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/extensions/fake_safe_browsing_database_manager.h"
+#include "chrome/browser/extensions/test_extension_dir.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/download_manager.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/download_test_observer.h"
 #include "extensions/browser/extension_prefs.h"
+#include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/management_policy.h"
 #include "extensions/browser/notification_types.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/feature_switch.h"
 #include "extensions/common/file_util.h"
 #include "extensions/common/permissions/permission_set.h"
 #include "extensions/common/switches.h"
 #include "ui/base/l10n/l10n_util.h"
 
@@ skipping 191 matching lines @@
             ->GetGrantedPermissions(mock_prompt->extension_id());
     ASSERT_TRUE(permissions.get());
   }
 };
 
 // This test is skipped on ChromeOS because it requires the NPAPI,
 // which is not available on that platform.
 #if !defined(OS_CHROMEOS)
 IN_PROC_BROWSER_TEST_F(ExtensionCrxInstallerTest, Whitelisting) {
   std::string id = "hdgllgikmikobbofgnabhfimcfoopgnd";
-  ExtensionService* service = extensions::ExtensionSystem::Get(
-      browser()->profile())->extension_service();
+  ExtensionRegistry* registry = ExtensionRegistry::Get(
+      browser()->profile());
 
   // Even whitelisted extensions with NPAPI should not prompt.
   scoped_refptr<MockPromptProxy> mock_prompt =
       CreateMockPromptProxyForBrowser(browser());
   InstallWithPrompt("uitest/plugins", id, mock_prompt);
   EXPECT_FALSE(mock_prompt->confirmation_requested());
-  EXPECT_TRUE(service->GetExtensionById(id, false));
+  EXPECT_TRUE(registry->enabled_extensions().GetByID(id));
 }
 #endif
 
 IN_PROC_BROWSER_TEST_F(ExtensionCrxInstallerTest,
                        GalleryInstallGetsExperimental) {
   // We must modify the command line temporarily in order to pack an extension
   // that requests the experimental permission.
   CommandLine* command_line = CommandLine::ForCurrentProcess();
   CommandLine old_command_line = *command_line;
   command_line->AppendSwitch(switches::kEnableExperimentalExtensionApis);
@@ skipping 106 matching lines @@
   }
 }
 
 IN_PROC_BROWSER_TEST_F(ExtensionCrxInstallerTest, HiDpiThemeTest) {
   base::FilePath crx_path = test_data_dir_.AppendASCII("theme_hidpi_crx");
   crx_path = crx_path.AppendASCII("theme_hidpi.crx");
 
   ASSERT_TRUE(InstallExtension(crx_path, 1));
 
   const std::string extension_id("gllekhaobjnhgeagipipnkpmmmpchacm");
-  ExtensionService* service = extensions::ExtensionSystem::Get(
-      browser()->profile())->extension_service();
-  ASSERT_TRUE(service);
+  ExtensionRegistry* registry = ExtensionRegistry::Get(
+      browser()->profile());
   const extensions::Extension* extension =
-     service->GetExtensionById(extension_id, false);
+     registry->enabled_extensions().GetByID(extension_id);
   ASSERT_TRUE(extension);
   EXPECT_EQ(extension_id, extension->id());
 
   UninstallExtension(extension_id);
-  EXPECT_FALSE(service->GetExtensionById(extension_id, false));
+  EXPECT_FALSE(registry->enabled_extensions().GetByID(extension_id));
 }
 
 // See http://crbug.com/315299.
 #if defined(OS_WIN) || defined(OS_LINUX)
 #define MAYBE_InstallDelayedUntilNextUpdate \
         DISABLED_InstallDelayedUntilNextUpdate
 #else
 #define MAYBE_InstallDelayedUntilNextUpdate InstallDelayedUntilNextUpdate
 #endif
 IN_PROC_BROWSER_TEST_F(ExtensionCrxInstallerTest,
                        MAYBE_InstallDelayedUntilNextUpdate) {
   const std::string extension_id("ldnnhddmnhbkjipkidpdiheffobcpfmf");
   base::FilePath crx_path = test_data_dir_.AppendASCII("delayed_install");
   ExtensionSystem* extension_system = extensions::ExtensionSystem::Get(
       browser()->profile());
   ExtensionService* service = extension_system->extension_service();
   ASSERT_TRUE(service);
+  ExtensionRegistry* registry = ExtensionRegistry::Get(
+      browser()->profile());
+  ASSERT_TRUE(registry);
 
   // Install version 1 of the test extension. This extension does not have
   // a background page but does have a browser action.
   ASSERT_TRUE(InstallExtension(crx_path.AppendASCII("v1.crx"), 1));
   const extensions::Extension* extension =
-     service->GetExtensionById(extension_id, false);
+     registry->enabled_extensions().GetByID(extension_id);
   ASSERT_TRUE(extension);
   ASSERT_EQ(extension_id, extension->id());
   ASSERT_EQ("1.0", extension->version()->GetString());
 
   // Make test extension non-idle by opening the extension's browser action
   // popup. This should cause the installation to be delayed.
   content::WindowedNotificationObserver loading_observer(
       extensions::NOTIFICATION_EXTENSION_HOST_DID_STOP_LOADING,
       content::Source<Profile>(profile()));
   BrowserActionTestUtil util(browser());
   // There is only one extension, so just click the first browser action.
   ASSERT_EQ(1, util.NumberOfBrowserActions());
   util.Press(0);
   loading_observer.Wait();
   ExtensionHost* extension_host =
       content::Details<ExtensionHost>(loading_observer.details()).ptr();
 
   // Install version 2 of the extension and check that it is indeed delayed.
   ASSERT_TRUE(UpdateExtensionWaitForIdle(
       extension_id, crx_path.AppendASCII("v2.crx"), 0));
 
   ASSERT_EQ(1u, service->delayed_installs()->size());
-  extension = service->GetExtensionById(extension_id, false);
+  extension = registry->enabled_extensions().GetByID(extension_id);
   ASSERT_EQ("1.0", extension->version()->GetString());
 
   // Make the extension idle again by closing the popup. This should not trigger
   // the delayed install.
   content::RenderProcessHostWatcher terminated_observer(
       extension_host->render_process_host(),
       content::RenderProcessHostWatcher::WATCH_FOR_HOST_DESTRUCTION);
   extension_host->render_view_host()->ClosePage();
   terminated_observer.Wait();
   ASSERT_EQ(1u, service->delayed_installs()->size());
 
   // Install version 3 of the extension. Because the extension is idle,
   // this install should succeed.
   ASSERT_TRUE(UpdateExtensionWaitForIdle(
       extension_id, crx_path.AppendASCII("v3.crx"), 0));
-  extension = service->GetExtensionById(extension_id, false);
+  extension = registry->enabled_extensions().GetByID(extension_id);
   ASSERT_EQ("3.0", extension->version()->GetString());
 
   // The version 2 delayed install should be cleaned up, and finishing
   // delayed extension installation shouldn't break anything.
   ASSERT_EQ(0u, service->delayed_installs()->size());
   service->MaybeFinishDelayedInstallations();
-  extension = service->GetExtensionById(extension_id, false);
+  extension = registry->enabled_extensions().GetByID(extension_id);
   ASSERT_EQ("3.0", extension->version()->GetString());
 }
 
 #if defined(FULL_SAFE_BROWSING)
 IN_PROC_BROWSER_TEST_F(ExtensionCrxInstallerTest, Blacklist) {
   scoped_refptr<FakeSafeBrowsingDatabaseManager> blacklist_db(
       new FakeSafeBrowsingDatabaseManager(true));
   Blacklist::ScopedDatabaseManagerForTest scoped_blacklist_db(blacklist_db);
 
   blacklist_db->SetUnsafe("gllekhaobjnhgeagipipnkpmmmpchacm");
@@ skipping 50 matching lines @@
 
   base::FilePath crx_path = test_data_dir_.AppendASCII("crx_installer/v1.crx");
   const extensions::Extension* extension = InstallExtension(
       crx_path, 1, extensions::Manifest::EXTERNAL_PREF);
   base::FilePath extension_path = extension->path();
   EXPECT_TRUE(cache_dir.path().IsParent(extension_path));
   EXPECT_TRUE(base::PathExists(extension_path));
 
   std::string extension_id = extension->id();
   UninstallExtension(extension_id);
-  ExtensionService* service = extensions::ExtensionSystem::Get(
-      browser()->profile())->extension_service();
-  EXPECT_FALSE(service->GetExtensionById(extension_id, false));
+  ExtensionRegistry* registry = ExtensionRegistry::Get(
+      browser()->profile());
+  EXPECT_FALSE(registry->enabled_extensions().GetByID(extension_id));
 
   // In the worst case you need to repeat this up to 3 times to make sure that
   // all pending tasks we sent from UI thread to task runner and back to UI.
   for (int i = 0; i < 3; i++) {
     // Wait for background task completion that sends replay to UI thread.
     content::BrowserThread::GetBlockingPool()->FlushForTesting();
     // Wait for UI thread task completion.
     base::RunLoop().RunUntilIdle();
   }
 
@@ skipping 21 matching lines @@
 IN_PROC_BROWSER_TEST_F(ExtensionCrxInstallerTest, ManagementPolicy) {
   ManagementPolicyMock policy;
   extensions::ExtensionSystem::Get(profile())
       ->management_policy()
       ->RegisterProvider(&policy);
 
   base::FilePath crx_path = test_data_dir_.AppendASCII("crx_installer/v1.crx");
   EXPECT_FALSE(InstallExtension(crx_path, 0));
 }
 
+IN_PROC_BROWSER_TEST_F(ExtensionCrxInstallerTest, WithheldElevationCheck) {
+  // Enable consent flag and install extension. The <all_hosts> permission will
+  // be withheld.
+  scoped_ptr<FeatureSwitch::ScopedOverride> enable_scripts_switch(
+      new FeatureSwitch::ScopedOverride(
+          FeatureSwitch::scripts_require_action(), true));
+
+  const char kManifest[] =
+      "{"
+      "  \"name\": \"Withheld test\","
+      "  \"version\": \"1.0\","
+      "  \"permissions\": ["
+      "    \"http://*/*\""
+      "  ],"
+      "  \"manifest_version\": 2"
+      "}";
+  TestExtensionDir dir;
+  dir.WriteManifest(kManifest);
+  base::FilePath crx_path = dir.Pack();
+  EXPECT_FALSE(crx_path.empty());
+  const extensions::Extension* extension = InstallExtension(crx_path, 1);

[Devlin, 2014/09/10 21:52:02]

nit: no prefix.

[gpdavis, 2014/09/11 00:51:01]

Done (will arrive in next patch set).

+  EXPECT_TRUE(base::PathExists(extension->path()));
+
+  std::string extension_id = extension->id();
+  ExtensionRegistry* registry = ExtensionRegistry::Get(
+      browser()->profile());
+  EXPECT_TRUE(registry->enabled_extensions().GetByID(extension_id));
+
+  // Disable consent flag and reinstall extension. It should now be disabled
+  // because previously withheld permissions are now being requested.
+  enable_scripts_switch.reset();
+  extension = InstallExtension(crx_path, -1);

[Devlin, 2014/09/10 21:52:02]

This part still isn't quite right.  Ideally, what we'd test is restarting the
browser with the switch off - but those types of tests tend to be a bit flaky. 
Instead, let's go ahead and use ExtensionService::ReloadExtensionsForTest() -
that's a pretty close approximation.

Also, we shouldn't just check that it's not in enabled extensions - that could
mean that it doesn't exist in the system at all (which is wrong).  We should
check a few things here, to be extra thorough:
- The extension should not be in enabled_extensions() (the check you have)
- The extension *should* be in disabled_extensions()
- The extension should have a disable reason of
Extension::DISABLE_PERMISSIONS_INCREASE.

[gpdavis, 2014/09/11 00:51:01]

ReloadExtensionsForTest is problematic in the browser test environment.  It ends
up calling ExtensionUpdater::Start(), which has already been called when
ExtensionSystem initializes for the browser test, and ExtensionUpdater::Stop()
never gets called inbetween.  So we end up in a situation where Start is called
twice, and that fails a DCHECK.  There seems to be no simple way to get around
this, unfortunately.  I had tried this before, but didn't dig as deeply into it
since I found I could use InstallExtension instead.

Using InstallExtension as is in this patch set, it passes all three of the
checks you just outlined.  Is there any better way to simulate a browser restart
that doesn't use ReloadExtensionsForTest?  Or is there something I'm missing
about the lifetimes of these objects in the browser test environment?

+  EXPECT_FALSE(registry->enabled_extensions().GetByID(extension_id));
+}
+
 }  // namespace extensions