- Removes fallback from Prefs to LoadExtension on UI thread.

- Fix extension_service_unittest to actually load extensions from prefs.

BUG=19733
TEST=Remove extension manifest from Preferences file. Extension shouldn't load even if the necessary files are present.
Review URL: http://codereview.chromium.org/339058

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

4 files changed, 76 insertions, 18 deletions

diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index d681e83d..0234ec5 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -21,6 +21,7 @@
 #include "chrome/browser/net/chrome_url_request_context.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension.h"
+#include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/extensions/extension_error_reporter.h"
 #include "chrome/common/notification_service.h"
 #include "chrome/common/pref_names.h"
@@ -280,11 +281,7 @@ void ExtensionsService::LoadInstalledExtension(
       extension = tmp.release();
     }
   } else {
-    // TODO(mpcomplete): obsolete. remove after migration period.
-    // http://code.google.com/p/chromium/issues/detail?id=19733
-    extension = extension_file_util::LoadExtension(path,
-                                                   true,  // Require id
-                                                   &error);
+    error = extension_manifest_errors::kManifestUnreadable;
   }
 
   if (!extension) {
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index 15c41e5..8cd1af2 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -503,11 +503,6 @@ TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectorySuccess) {
   service_->Init();
   loop_.RunAllPending();
 
-  std::vector<std::string> errors = GetErrors();
-  for (std::vector<std::string>::iterator err = errors.begin();
-    err != errors.end(); ++err) {
-    LOG(ERROR) << *err;
-  }
   ASSERT_EQ(3u, loaded_.size());
 
   EXPECT_EQ(std::string(good0), loaded_[0]->id());
@@ -616,8 +611,8 @@ TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectoryFail) {
   ASSERT_EQ(0u, loaded_.size());
 
   EXPECT_TRUE(MatchPattern(GetErrors()[0],
-      std::string("Could not load extension from '*'. * ") +
-      base::JSONReader::kBadRootElementType)) << GetErrors()[0];
+      std::string("Could not load extension from '*'. ") +
+      extension_manifest_errors::kManifestUnreadable)) << GetErrors()[0];
 
   EXPECT_TRUE(MatchPattern(GetErrors()[1],
       std::string("Could not load extension from '*'. ") +
@@ -1110,7 +1105,7 @@ TEST_F(ExtensionsServiceTest, WillNotLoadBlacklistedExtensionsFromDirectory) {
     err != errors.end(); ++err) {
     LOG(ERROR) << *err;
   }
-  EXPECT_EQ(2u, loaded_.size());
+  ASSERT_EQ(2u, loaded_.size());
 
   EXPECT_NE(std::string(good0), loaded_[0]->id());
   EXPECT_NE(std::string(good0), loaded_[1]->id());
diff --git a/chrome/test/data/extensions/bad/Preferences b/chrome/test/data/extensions/bad/Preferences
index 5ce4d84..cd03b71 100644
--- a/chrome/test/data/extensions/bad/Preferences
+++ b/chrome/test/data/extensions/bad/Preferences
@@ -4,7 +4,8 @@
          "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa": {
             "location": 1,
             "path": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/1",
-            "state": 1
+            "state": 1,
+            "manifest": "I am not a manifest"
          },
          "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb": {
             "location": 1,
@@ -14,7 +15,17 @@
          "bjafgdebaacbbbecmhlhpofkepfkgcpa": {
             "location": 1,
             "path": "bjafgdebaacbbbecmhlhpofkepfkgcpa/1.0",
-            "state": 1
+            "state": 1,
+            "manifest": {
+              "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDRS2GUBOUAO5VZ2CMRId/eRR8/e9V42nUvY5XG+0sZ+JDHEjIQdq8qQy7HqdqEpCXKPMSPuMiC2t2HE9/hpL89SblNn3mwYPtSJGQdZvAzuv6SB0oA6jZ66V7+h/k0noGD3Tcu+Ko/vfkt5wCx2uHVK29k5JR/vGr0klaoVezGlwIDAQAB",
+              "version": "1.0",
+              "name": "My extension 3",
+              "content_scripts": [
+                {
+                  "matches": ["http://*/*"]
+                }
+              ]
+            }
          },
             "cccccccccccccccccccccccccccccccc": {
             "location": 1,
diff --git a/chrome/test/data/extensions/good/Preferences b/chrome/test/data/extensions/good/Preferences
index f65cf48..42a2713 100644
--- a/chrome/test/data/extensions/good/Preferences
+++ b/chrome/test/data/extensions/good/Preferences
@@ -4,17 +4,72 @@
          "behllobkkfkfnphdnhnkndlbkcpglgmj": {
             "location": 1,
             "path": "behllobkkfkfnphdnhnkndlbkcpglgmj/1.0.0.0",
-            "state": 1
+            "state": 1,
+            "manifest": {
+              "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDuUZGKCDbff6IRaxa4Pue7PPkxwPaNhGT3JEqppEsNWFjM80imEdqMbf3lrWqEfaHgaNku7nlpwPO1mu3/4Hr+XdNa5MhfnOnuPee4hyTLwOs3Vzz81wpbdzUxZSi2OmqMyI5oTaBYICfNHLwcuc65N5dbt6WKGeKgTpp4v7j7zwIDAQAB",
+              "version": "1.0.0.0",
+              "name": "My extension 1",
+              "description": "The first extension that I made.",
+              "icons": {
+                "128": "icon_128.png"
+              },
+              "default_locale": "en_US",
+              "toolstrips": [
+                {
+                  "path": "toolstrip1.html",
+                  "mole": "lorem_ipsum.html",
+                  "mole_height": 200
+                },
+                "toolstrip2.html"
+              ],
+              "background_page": "backgroundpage.html",
+              "permissions": ["tabs", "http://*.google.com/*", "https://*.google.com/*"],
+              "content_scripts": [
+                {
+                  "matches": ["file://*", "http://*.google.com/*", "https://*.google.com/*"],
+                  "js": ["script1.js", "script2.js"],
+                  "css": ["style1.css", "style2.css", "style2.css"]
+                },
+                {
+                  "matches": ["http://*.news.com/*"],
+                  "js": ["js_files/script3.js"]
+                }
+              ],
+              "privacy_blacklists": ["blacklist.pbl"]
+            }
          },
          "bjafgdebaacbbbecmhlhpofkepfkgcpa": {
             "location": 1,
             "path": "bjafgdebaacbbbecmhlhpofkepfkgcpa/1.0",
-            "state": 1
+            "state": 1,
+            "manifest": {
+              "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDRS2GUBOUAO5VZ2CMRId/eRR8/e9V42nUvY5XG+0sZ+JDHEjIQdq8qQy7HqdqEpCXKPMSPuMiC2t2HE9/hpL89SblNn3mwYPtSJGQdZvAzuv6SB0oA6jZ66V7+h/k0noGD3Tcu+Ko/vfkt5wCx2uHVK29k5JR/vGr0klaoVezGlwIDAQAB",
+              "version": "1.0",
+              "name": "My extension 3",
+              "toolstrips": ["toolstrip.html"],
+              "permissions": ["tabs"],
+              "content_scripts": [
+                {
+                  "matches": ["file://*"],
+                  "js": ["page.js"]
+                }
+              ]
+            }
          },
          "hpiknbiabeeppbpihjehijgoemciehgk": {
             "location": 1,
             "path": "hpiknbiabeeppbpihjehijgoemciehgk/2",
-            "state": 1
+            "state": 1,
+            "manifest": {
+              "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQC1nrgIE4OIQez0DVtc3JPR5O69s0XrH7TYC/xUC2e7Cla6eZldsA1PCWzLacimqtwfu7ljGXslk0HffkWNNou28Ip07KIC5oQHGEieAkNdPd3Pfi7QcAz+D0/xNPxuMtTKyuptWNtb2TTWD0MG7IdLHbMYFO6avkZtP+ldiKqxOwIDAQAB",
+              "version": "2",
+              "name": "My extension 2",
+              "plugins": [
+                { "path": "content_plugin.dll", "public": true },
+                { "path": "extension_plugin.dll" }
+              ],
+              "background_page": "background.html"
+            }
          }
       }
    }