Delay determination of why a synced disabled extension was disabled.

Also simplify handling of legacy disabled extensions.

BUG=162062


Review URL: https://chromiumcodereview.appspot.com/11412239

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@171072 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/extensions/extension_disabled_ui_browsertest.cc

@@ -5,20 +5,30 @@
 #include "base/file_path.h"
 #include "base/files/scoped_temp_dir.h"
 #include "chrome/app/chrome_command_ids.h"
+#include "chrome/browser/extensions/autoupdate_interceptor.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/extensions/extension_prefs.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/updater/extension_updater.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/global_error/global_error.h"
 #include "chrome/browser/ui/global_error/global_error_service.h"
 #include "chrome/browser/ui/global_error/global_error_service_factory.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension.h"
+#include "net/url_request/url_fetcher.h"
 
 using extensions::Extension;
 
 class ExtensionDisabledGlobalErrorTest : public ExtensionBrowserTest {
  protected:
+  void SetUpCommandLine(CommandLine* command_line) {
+    ExtensionBrowserTest::SetUpCommandLine(command_line);
+    command_line->AppendSwitchASCII(switches::kAppsGalleryUpdateURL,
+                                    "http://localhost/autoupdate/updates.xml");
+  }
+
   void SetUpOnMainThread() {
     EXPECT_TRUE(scoped_temp_dir_.CreateUniqueTempDir());
     service_ = browser()->profile()->GetExtensionService();
@@ -161,3 +171,49 @@ IN_PROC_BROWSER_TEST_F(ExtensionDisabledGlobalErrorTest,
   ASSERT_TRUE(extension);
   ASSERT_TRUE(GetExtensionDisabledGlobalError());
 }
+
+// Test that an error appears if the extension gets disabled because a
+// version with higher permissions was installed by sync.
+IN_PROC_BROWSER_TEST_F(ExtensionDisabledGlobalErrorTest,
+                       HigherPermissionsFromSync) {
+  // Get data for extension v2 (disabled) into sync.
+  const Extension* extension = InstallAndUpdateIncreasingPermissionsExtension();
+  std::string extension_id = extension->id();
+  // service_->GrantPermissionsAndEnableExtension(extension, false);
+  extensions::ExtensionSyncData sync_data =
+      service_->GetExtensionSyncData(*extension);
+  UninstallExtension(extension_id);
+  extension = NULL;
+
+  // Install extension v1.
+  InstallIncreasingPermissionExtensionV1();
+
+  // Note: This interceptor gets requests on the IO thread.
+  scoped_refptr<extensions::AutoUpdateInterceptor> interceptor(
+      new extensions::AutoUpdateInterceptor());
+  net::URLFetcher::SetEnableInterceptionForTests(true);
+  interceptor->SetResponseOnIOThread(
+      "http://localhost/autoupdate/updates.xml",
+      test_data_dir_.AppendASCII("permissions_increase")
+                    .AppendASCII("updates.xml"));
+  interceptor->SetResponseOnIOThread(
+      "http://localhost/autoupdate/v2.crx",
+      scoped_temp_dir_.path().AppendASCII("permissions2.crx"));
+
+  extensions::ExtensionUpdater::CheckParams params;
+  params.check_blacklist = false;
+  service_->updater()->set_default_check_params(params);
+
+  // Sync is replacing an older version, so it pends.
+  EXPECT_FALSE(service_->ProcessExtensionSyncData(sync_data));
+
+  WaitForExtensionInstall();
+
+  extension = service_->GetExtensionById(extension_id, true);
+  ASSERT_TRUE(extension);
+  EXPECT_EQ("2", extension->VersionString());
+  EXPECT_EQ(1u, service_->disabled_extensions()->size());
+  EXPECT_EQ(Extension::DISABLE_PERMISSIONS_INCREASE,
+            service_->extension_prefs()->GetDisableReasons(extension_id));
+  EXPECT_TRUE(GetExtensionDisabledGlobalError());
+}

chrome/browser/extensions/extension_service.cc

@@ -1560,10 +1560,14 @@ bool ExtensionService::ProcessExtensionSyncDataHelper(
   }
 
   // Set user settings.
+  // If the extension has been disabled from sync, it may not have
+  // been installed yet, so we don't know if the disable reason was a
+  // permissions increase.  That will be updated once InitializePermissions
+  // is called for it.
   if (extension_sync_data.enabled())
     EnableExtension(id);
   else
-    DisableExtension(id, Extension::DISABLE_USER_ACTION);
+    DisableExtension(id, Extension::DISABLE_UNKNOWN_FROM_SYNC);
 
   // We need to cache some version information here because setting the
   // incognito flag invalidates the |extension| pointer (it reloads the
@@ -2116,7 +2120,9 @@ void ExtensionService::AddExtension(const Extension* extension) {
         content::Source<Profile>(profile_),
         content::Details<const Extension>(extension));
 
-    if (extension_prefs_->GetDisableReasons(extension->id()) &
+    // Show the extension disabled error if a permissions increase was the
+    // only reason it was disabled.
+    if (extension_prefs_->GetDisableReasons(extension->id()) ==
         Extension::DISABLE_PERMISSIONS_INCREASE) {
       extensions::AddExtensionDisabledError(this, extension);
     }
@@ -2265,19 +2271,22 @@ void ExtensionService::InitializePermissions(const Extension* extension) {
     // If the extension was already disabled, suppress any alerts for becoming
     // disabled on permissions increase.
     previously_disabled = extension_prefs_->IsExtensionDisabled(old->id());
-    if (previously_disabled) {
-      int reasons = extension_prefs_->GetDisableReasons(old->id());
-      if (reasons == Extension::DISABLE_NONE) {
-        // Initialize the reason for legacy disabled extensions from whether the
-        // extension already exceeded granted permissions.
-        if (extension_prefs_->DidExtensionEscalatePermissions(old->id()))
-          disable_reasons = Extension::DISABLE_PERMISSIONS_INCREASE;
-        else
-          disable_reasons = Extension::DISABLE_USER_ACTION;
-      }
-    } else {
-      disable_reasons = Extension::DISABLE_PERMISSIONS_INCREASE;
+    // Legacy disabled extensions do not have a disable reason. Infer that if
+    // there was no permission increase, it was likely disabled by the user.
+    if (previously_disabled && disable_reasons == Extension::DISABLE_NONE &&
+        !extension_prefs_->DidExtensionEscalatePermissions(old->id())) {
+      disable_reasons |= Extension::DISABLE_USER_ACTION;
+    }
+    // Extensions that came to us disabled from sync need a similar inference,
+    // except based on the new version's permissions.
+    if (previously_disabled &&
+        disable_reasons == Extension::DISABLE_UNKNOWN_FROM_SYNC) {
+      // Remove the DISABLE_UNKNOWN_FROM_SYNC reason.
+      extension_prefs_->ClearDisableReasons(extension->id());
+      if (!is_privilege_increase)
+        disable_reasons |= Extension::DISABLE_USER_ACTION;
     }
+    disable_reasons &= ~Extension::DISABLE_UNKNOWN_FROM_SYNC;
 
     // To upgrade an extension in place, unload the old one and
     // then load the new one.
@@ -2288,6 +2297,7 @@ void ExtensionService::InitializePermissions(const Extension* extension) {
   // Extension has changed permissions significantly. Disable it. A
   // notification should be sent by the caller.
   if (is_privilege_increase) {
+    disable_reasons |= Extension::DISABLE_PERMISSIONS_INCREASE;
     if (!extension_prefs_->DidExtensionEscalatePermissions(extension->id())) {
       RecordPermissionMessagesHistogram(
           extension, "Extensions.Permissions_AutoDisable");

chrome/common/extensions/extension.h

@@ -121,6 +121,7 @@ class Extension : public base::RefCountedThreadSafe<Extension> {
     DISABLE_RELOAD = 1 << 2,
     DISABLE_UNSUPPORTED_REQUIREMENT = 1 << 3,
     DISABLE_SIDELOAD_WIPEOUT = 1 << 4,
+    DISABLE_UNKNOWN_FROM_SYNC = 1 << 5,
   };
 
   enum InstallType {

chrome/test/data/extensions/permissions_increase/updates.xml

+<?xml version='1.0' encoding='UTF-8'?>
+<gupdate xmlns='http://www.google.com/update2/response' protocol='2.0'>
+  <app appid='pgdpcfcocojkjfbgpiianjngphoopgmo'>
+    <updatecheck codebase='http://localhost/autoupdate/v2.crx' version='2' />
+  </app>
+</gupdate>

chrome/test/data/extensions/permissions_increase/v1/manifest.json

 {
   "version": "1",
   "name": "Permission Test",
-  "background_page": "background.html"
+  "background_page": "background.html",
+  "update_url": "http://localhost/autoupdate/updates.xml"
 }

chrome/test/data/extensions/permissions_increase/v2/manifest.json

@@ -2,6 +2,7 @@
   "version": "2",
   "name": "Permission Test",
   "background_page": "background.html",
+  "update_url": "http://localhost/autoupdate/updates.xml",
   "permissions": [
     "tabs"
   ]

chrome/test/data/extensions/permissions_increase/v3/manifest.json

@@ -2,6 +2,7 @@
   "version": "3",
   "name": "Permission Test",
   "background_page": "background.html",
+  "update_url": "http://localhost/autoupdate/updates.xml",
   "permissions": [
     "tabs", "management"
   ]