Revert 95815 - Make extension file URL access opt-in.

This corrects an issue causing file URL access to default on for <all_urls> and file:/// permissions. We also revert all extension's "allow file access" flags to false since we can't distinguish between extensions that were installed with the bug present and those where the user clicked allow file access. Unpacked extensions will now have opt-in file access as well.

BUG=91577
TEST=ExtensionServiceTest.DefaultFileAccess

Review URL: http://codereview.chromium.org/7574017

TBR=jstritar@chromium.org
Review URL: http://codereview.chromium.org/7595005

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

8 files changed, 36 insertions, 28 deletions

diff --git a/chrome/browser/extensions/extension_browsertest.cc b/chrome/browser/extensions/extension_browsertest.cc
index 580870a..ec64fdb 100644
--- a/chrome/browser/extensions/extension_browsertest.cc
+++ b/chrome/browser/extensions/extension_browsertest.cc
@@ -114,9 +114,9 @@ const Extension* ExtensionBrowserTest::LoadExtensionWithOptions(
     ui_test_utils::WindowedNotificationObserver load_signal(
         chrome::NOTIFICATION_EXTENSION_LOADED,
         Source<Profile>(browser()->profile()));
-    CHECK(!service->AllowFileAccess(extension));
+    CHECK(service->AllowFileAccess(extension));
 
-    if (fileaccess_enabled) {
+    if (!fileaccess_enabled) {
       service->SetAllowFileAccess(extension, fileaccess_enabled);
       load_signal.Wait();
       extension = service->GetExtensionById(extension_id, false);
@@ -131,7 +131,7 @@ const Extension* ExtensionBrowserTest::LoadExtensionWithOptions(
 }
 
 const Extension* ExtensionBrowserTest::LoadExtension(const FilePath& path) {
-  return LoadExtensionWithOptions(path, false, false);
+  return LoadExtensionWithOptions(path, false, true);
 }
 
 const Extension* ExtensionBrowserTest::LoadExtensionIncognito(
diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc
index 88cedb7..49b892f 100644
--- a/chrome/browser/extensions/extension_prefs.cc
+++ b/chrome/browser/extensions/extension_prefs.cc
@@ -74,11 +74,7 @@ const char kPrefIncognitoEnabled[] = "incognito";
 
 // A preference to control whether an extension is allowed to inject script in
 // pages with file URLs.
-const char kPrefAllowFileAccess[] = "newAllowFileAccess";
-// TODO(jstritar): As part of fixing http://crbug.com/91577, we revoked all
-// extension file access by renaming the pref. We should eventually clean up
-// the old flag and possibly go back to that name.
-// const char kPrefAllowFileAccessOld[] = "allowFileAccess";
+const char kPrefAllowFileAccess[] = "allowFileAccess";
 
 // A preference set by the web store to indicate login information for
 // purchased apps.
@@ -808,6 +804,12 @@ void ExtensionPrefs::SetAllowFileAccess(const std::string& extension_id,
                       Value::CreateBooleanValue(allow));
 }
 
+bool ExtensionPrefs::HasAllowFileAccessSetting(
+    const std::string& extension_id) const {
+  const DictionaryValue* ext = GetExtensionPref(extension_id);
+  return ext && ext->HasKey(kPrefAllowFileAccess);
+}
+
 ExtensionPrefs::LaunchType ExtensionPrefs::GetLaunchType(
     const std::string& extension_id,
     ExtensionPrefs::LaunchType default_pref_value) {
diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h
index 8b40ad9..25c2c9a 100644
--- a/chrome/browser/extensions/extension_prefs.h
+++ b/chrome/browser/extensions/extension_prefs.h
@@ -207,6 +207,7 @@ class ExtensionPrefs : public ExtensionContentSettingsStore::Observer {
   // scripts into pages with file URLs.
   bool AllowFileAccess(const std::string& extension_id);
   void SetAllowFileAccess(const std::string& extension_id, bool allow);
+  bool HasAllowFileAccessSetting(const std::string& extension_id) const;
 
   // Get the launch type preference.  If no preference is set, return
   // |default_pref_value|.
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 0ac80c7..5c05c6c 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -304,7 +304,12 @@ void ExtensionServiceBackend::CheckExtensionFileAccess(
     const FilePath& extension_path, bool prompt_for_plugins) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   std::string id = Extension::GenerateIdForPath(extension_path);
-  bool allow_file_access = frontend_->extension_prefs()->AllowFileAccess(id);
+  // Unpacked extensions default to allowing file access, but if that has been
+  // overridden, don't reset the value.
+  bool allow_file_access =
+      Extension::ShouldAlwaysAllowFileAccess(Extension::LOAD);
+  if (frontend_->extension_prefs()->HasAllowFileAccessSetting(id))
+    allow_file_access = frontend_->extension_prefs()->AllowFileAccess(id);
 
   BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
       NewRunnableMethod(
@@ -1029,9 +1034,13 @@ void ExtensionService::LoadExtensionFromCommandLine(
   file_util::AbsolutePath(&extension_path);
 
   std::string id = Extension::GenerateIdForPath(extension_path);
+  bool allow_file_access =
+      Extension::ShouldAlwaysAllowFileAccess(Extension::LOAD);
+  if (extension_prefs()->HasAllowFileAccessSetting(id))
+    allow_file_access = extension_prefs()->AllowFileAccess(id);
 
   int flags = Extension::NO_FLAGS;
-  if (extension_prefs()->AllowFileAccess(id))
+  if (allow_file_access)
     flags |= Extension::ALLOW_FILE_ACCESS;
   if (Extension::ShouldDoStrictErrorChecking(Extension::LOAD))
     flags |= Extension::STRICT_ERROR_CHECKS;
@@ -2196,6 +2205,13 @@ void ExtensionService::OnExtensionInstalled(
       initial_enable ? Extension::ENABLED : Extension::DISABLED,
       from_webstore);
 
+  // Unpacked extensions default to allowing file access, but if that has been
+  // overridden, don't reset the value.
+  if (Extension::ShouldAlwaysAllowFileAccess(Extension::LOAD) &&
+      !extension_prefs_->HasAllowFileAccessSetting(id)) {
+    extension_prefs_->SetAllowFileAccess(id, true);
+  }
+
   NotificationService::current()->Notify(
       chrome::NOTIFICATION_EXTENSION_INSTALLED,
       Source<Profile>(profile_),
diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index b002501..8d7ea1a 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -1756,18 +1756,6 @@ TEST_F(ExtensionServiceTest, InstallApps) {
   ValidatePrefKeyCount(pref_count);
 }
 
-// Tests that file access is OFF by default.
-TEST_F(ExtensionServiceTest, DefaultFileAccess) {
-  InitializeEmptyExtensionService();
-  PackAndInstallCrx(data_dir_.AppendASCII("permissions").AppendASCII("files"),
-                    true);
-
-  EXPECT_EQ(0u, GetErrors().size());
-  EXPECT_EQ(1u, service_->extensions()->size());
-  std::string id = service_->extensions()->at(0)->id();
-  EXPECT_FALSE(service_->extension_prefs()->AllowFileAccess(id));
-}
-
 TEST_F(ExtensionServiceTest, UpdateApps) {
   InitializeEmptyExtensionService();
   FilePath extensions_path = data_dir_.AppendASCII("app_update");
diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index b3d6eb2..e46cba1 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -287,6 +287,12 @@ class Extension : public base::RefCountedThreadSafe<Extension> {
            location == Extension::COMPONENT;
   }
 
+  // Unpacked extensions start off with file access since they are a developer
+  // feature.
+  static inline bool ShouldAlwaysAllowFileAccess(Location location) {
+    return location == Extension::LOAD;
+  }
+
   // See Type definition above.
   Type GetType() const;
 
diff --git a/chrome/test/data/extensions/good/Preferences b/chrome/test/data/extensions/good/Preferences
index ff39f64..2d6f0bf 100644
--- a/chrome/test/data/extensions/good/Preferences
+++ b/chrome/test/data/extensions/good/Preferences
@@ -10,7 +10,7 @@
               "host": ["file://*", "http://*.google.com/*", "https://*.google.com/*", "http://*.news.com/*"]
             },
             "state": 1,
-            "newAllowFileAccess": true,
+            "allowFileAccess": true,
             "manifest": {
               "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDuUZGKCDbff6IRaxa4Pue7PPkxwPaNhGT3JEqppEsNWFjM80imEdqMbf3lrWqEfaHgaNku7nlpwPO1mu3/4Hr+XdNa5MhfnOnuPee4hyTLwOs3Vzz81wpbdzUxZSi2OmqMyI5oTaBYICfNHLwcuc65N5dbt6WKGeKgTpp4v7j7zwIDAQAB",
               "version": "1.0.0.0",
diff --git a/chrome/test/data/extensions/permissions/files/manifest.json b/chrome/test/data/extensions/permissions/files/manifest.json
deleted file mode 100644
index 38e71df..0000000
--- a/chrome/test/data/extensions/permissions/files/manifest.json
+++ /dev/null
@@ -1,5 +0,0 @@
-{
-  "name": "file access extension",
-  "version": "1",
-  "permissions": ["file:///*"]
-}