Prevent installation of an app if its web extent overlaps another installed app.

BUG=41280

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

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

8 files changed, 70 insertions, 11 deletions

diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index b2eef6e..a489752 100644
--- a/chrome/app/generated_resources.grd
+++ b/chrome/app/generated_resources.grd
@@ -3418,6 +3418,9 @@ each locale. -->
       <message name="IDS_EXTENSION_MANIFEST_INVALID" desc="">
         Manifest file is invalid.
       </message>
+      <message name="IDS_EXTENSION_OVERLAPPING_WEB_EXTENT" desc="Error message when a user tries to install an app with a web extent that overlaps another installed app.">
+        This app claims to own '<ph name="APP_OVERLAPPING_URL">$2<ex>http://google.com/mail</ex></ph>' which already belongs to installed app '<ph name="APP_NAME">$1<ex>Google Mail</ex></ph>'.
+      </message>
       <message name="IDS_EXTENSION_LOCALES_NO_DEFAULT_LOCALE_SPECIFIED" desc="">
         Localization used, but default_locale wasn't specified in the manifest.
       </message>
diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc
index 34a51d2..832318c 100644
--- a/chrome/browser/extensions/crx_installer.cc
+++ b/chrome/browser/extensions/crx_installer.cc
@@ -172,6 +172,18 @@ void CrxInstaller::ConfirmInstall() {
     return;
   }
 
+  GURL overlapping_url;
+  Extension* overlapping_extension =
+      frontend_->GetExtensionByOverlappingWebExtent(
+          extension_->web_extent(), &overlapping_url);
+  if (overlapping_extension) {
+    ReportFailureFromUIThread(l10n_util::GetStringFUTF8(
+        IDS_EXTENSION_OVERLAPPING_WEB_EXTENT,
+        UTF8ToUTF16(overlapping_extension->name()),
+        UTF8ToUTF16(overlapping_url.spec())));
+    return;
+  }
+
   current_version_ =
       frontend_->extension_prefs()->GetVersionString(extension_->id());
 
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index ed6e6d3..1837d15 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -767,7 +767,7 @@ void ExtensionsService::OnLoadedInstalledExtensions() {
       NotificationService::NoDetails());
 }
 
-bool ExtensionsService::OnExtensionLoaded(Extension* extension,
+void ExtensionsService::OnExtensionLoaded(Extension* extension,
                                           bool allow_privilege_increase) {
   // Ensure extension is deleted unless we transfer ownership.
   scoped_ptr<Extension> scoped_extension(extension);
@@ -835,7 +835,6 @@ bool ExtensionsService::OnExtensionLoaded(Extension* extension,
   extension->set_being_upgraded(false);
 
   UpdateActiveExtensionsInCrashReporter();
-  return true;
 }
 
 void ExtensionsService::UpdateActiveExtensionsInCrashReporter() {
@@ -883,16 +882,14 @@ void ExtensionsService::OnExtensionInstalled(Extension* extension,
   }
 
   // Also load the extension.
-  bool success = OnExtensionLoaded(extension, allow_privilege_increase);
-  if (!success)
-    extension = NULL;  // extension is deleted on failure.
+  OnExtensionLoaded(extension, allow_privilege_increase);
 
   // Erase any pending extension.
   if (it != pending_extensions_.end()) {
     pending_extensions_.erase(it);
   }
 
-  if (success && profile_->GetTemplateURLModel())
+  if (profile_->GetTemplateURLModel())
     profile_->GetTemplateURLModel()->RegisterExtensionKeyword(extension);
 }
 
@@ -930,6 +927,20 @@ Extension* ExtensionsService::GetExtensionByWebExtent(const GURL& url) {
   return NULL;
 }
 
+Extension* ExtensionsService::GetExtensionByOverlappingWebExtent(
+    const ExtensionExtent& extent, GURL* overlapping_url) {
+  for (size_t i = 0; i < extensions_.size(); ++i) {
+    for (size_t j = 0; j < extent.paths().size(); ++j) {
+      GURL url(extent.origin().Resolve(extent.paths()[j]));
+      if (extensions_[i]->web_extent().ContainsURL(url)) {
+        *overlapping_url = url;
+        return extensions_[i];
+      }
+    }
+  }
+  return NULL;
+}
+
 void ExtensionsService::ClearProvidersForTesting() {
   ChromeThread::PostTask(
       ChromeThread::FILE, FROM_HERE,
diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h
index c8e3842..c7e0609 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -252,6 +252,12 @@ class ExtensionsService
   // returns the extension whose web extent contains |url|.
   Extension* GetExtensionByWebExtent(const GURL& url);
 
+  // Returns an extension that contains any URL that overlaps with the given
+  // extent, if one exists. Also fills |overlapping_url| with the first URL that
+  // overlaps.
+  Extension* GetExtensionByOverlappingWebExtent(const ExtensionExtent& extent,
+                                                GURL* overlapping_url);
+
   // Clear all ExternalExtensionProviders.
   void ClearProvidersForTesting();
 
@@ -263,9 +269,8 @@ class ExtensionsService
   // Called when the initial extensions load has completed.
   virtual void OnLoadedInstalledExtensions();
 
-  // Called when an extension has been loaded. Returns false if the extension
-  // failed to load (for example, if it has already been loaded).
-  bool OnExtensionLoaded(Extension* extension,
+  // Called when an extension has been loaded.
+  void OnExtensionLoaded(Extension* extension,
                          bool allow_privilege_increase);
 
   // Called by the backend when an extension has been installed.
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index c74b644..6cd03c2 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -60,6 +60,7 @@ const char* const good_crx = "ldnnhddmnhbkjipkidpdiheffobcpfmf";
 const char* const page_action = "obcimlgaoabeegjmmpldobjndiealpln";
 const char* const theme_crx = "iamefpfkojoapidjnbafmgkgncegbkad";
 const char* const theme2_crx = "pjpgmfcmabopnnfonnhmdjglfpjjfkbf";
+const char* const app_crx = "hnbefahlpjbkldcaldefcnonockppomb";
 
 struct ExtensionsOrder {
   bool operator()(const Extension* a, const Extension* b) {
@@ -241,6 +242,13 @@ ExtensionsServiceTestBase::~ExtensionsServiceTestBase() {
 
 void ExtensionsServiceTestBase::InitializeExtensionsService(
     const FilePath& pref_file, const FilePath& extensions_install_dir) {
+  // Must setup the commandline here, since Extension caches the switch value
+  // when the prefs are registered.
+  CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kEnableExtensionToolstrips);
+  CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kEnableApps);
+
   ExtensionTestingProfile* profile = new ExtensionTestingProfile();
   prefs_.reset(new PrefService(
       new JsonPrefStore(
@@ -251,8 +259,6 @@ void ExtensionsServiceTestBase::InitializeExtensionsService(
   browser::RegisterUserPrefs(prefs_.get());
   profile_.reset(profile);
 
-  CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kEnableExtensionToolstrips);
   service_ = new ExtensionsService(profile_.get(),
                                    CommandLine::ForCurrentProcess(),
                                    prefs_.get(),
@@ -951,6 +957,28 @@ TEST_F(ExtensionsServiceTest, InstallTheme) {
   ValidatePrefKeyCount(pref_count);
 }
 
+TEST_F(ExtensionsServiceTest, InstallApps) {
+  InitializeEmptyExtensionsService();
+  FilePath extensions_path;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path));
+  extensions_path = extensions_path.AppendASCII("extensions");
+
+  // An empty app.
+  InstallExtension(extensions_path.AppendASCII("app1.crx"), true);
+  int pref_count = 0;
+  ValidatePrefKeyCount(++pref_count);
+  ValidateIntegerPref(app_crx, L"state", Extension::ENABLED);
+  ValidateIntegerPref(app_crx, L"location", Extension::INTERNAL);
+
+  // Another app with non-overlapping extent. Should succeed.
+  InstallExtension(extensions_path.AppendASCII("app2.crx"), true);
+  ValidatePrefKeyCount(++pref_count);
+
+  // A third app whose extent overlaps the first. Should fail.
+  InstallExtension(extensions_path.AppendASCII("app3.crx"), false);
+  ValidatePrefKeyCount(pref_count);
+}
+
 // Test that when an extension version is reinstalled, nothing happens.
 TEST_F(ExtensionsServiceTest, Reinstall) {
   InitializeEmptyExtensionsService();
diff --git a/chrome/test/data/extensions/app1.crx b/chrome/test/data/extensions/app1.crx
new file mode 100644
index 0000000..df016a9
--- /dev/null
+++ b/chrome/test/data/extensions/app1.crx
diff --git a/chrome/test/data/extensions/app2.crx b/chrome/test/data/extensions/app2.crx
new file mode 100644
index 0000000..5dd5ea7
--- /dev/null
+++ b/chrome/test/data/extensions/app2.crx
diff --git a/chrome/test/data/extensions/app3.crx b/chrome/test/data/extensions/app3.crx
new file mode 100644
index 0000000..e840374
--- /dev/null
+++ b/chrome/test/data/extensions/app3.crx