Don't block shared_module extensions by the policy blacklist.

chrome/browser/extensions/crx_installer.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 "chrome/browser/extensions/crx_installer.h"
 
 #include <map>
 #include <set>
 
 #include "base/bind.h"
@@ skipping 468 matching lines @@
   // TODO(crbug.com/420147): Move this code to a utility class to avoid
   // duplication of SharedModuleService::CheckImports code.
   if (SharedModuleInfo::ImportsModules(extension())) {
     const std::vector<SharedModuleInfo::ImportInfo>& imports =
         SharedModuleInfo::GetImports(extension());
     std::vector<SharedModuleInfo::ImportInfo>::const_iterator i;
     for (i = imports.begin(); i != imports.end(); ++i) {
       Version version_required(i->minimum_version);
       const Extension* imported_module =
           service->GetExtensionById(i->extension_id, true);
-      if (!imported_module) {
-        ReportFailureFromUIThread(CrxInstallerError(l10n_util::GetStringFUTF16(
-            IDS_EXTENSION_INSTALL_DEPENDENCY_NOT_FOUND,
-            base::ASCIIToUTF16(i->extension_id),
-            base::ASCIIToUTF16(i->minimum_version))));
-        return;
-      } else if (imported_module &&
-                 !SharedModuleInfo::IsSharedModule(imported_module)) {
+      if (imported_module &&

[elijahtaylor1, 2014/10/22 16:40:01]

As I stated in the other CL, this is probably ok.  Since WebstoreInstaller
installs the shared modules first, there should be no way for this to fail from
there.  The only downside is that developers side-loading CRXs that forget to
install shared modules first will not get any error warning and the extension
gets into a weird state (delayed I guess), but we are phasing out that workflow

[Joao da Silva, 2014/10/22 18:31:05]

If you agree then it would be great to do this fix for now.

Another possibility is to move the logic that installs the shared_modules out of
the WebstoreInstaller and reuse it for installations via external_loaders too;
only developers loading unpacked extensions should bypass that.

WDYT?

[elijahtaylor1, 2014/10/22 19:57:49]

I don't know much about external_loaders, but if we fixed it this way:  Does
this mean that if you drag-n-drop a CRX that depends on a shared module that is
not installed, that you would download it from the webstore?  I've shied away
from downloading shared modules for users unless the depending extension is also
from the webstore, but that may be unreasonable.

At the very least we can leave this as-is for this CL and file a bug to track
moving the webstoreinstaller code as you described.  Please elaborate on the
external loaders stuff in the bug, as I'm unfamiliar.

[Joao da Silva, 2014/10/23 09:15:02]

I think the drag-and-drop flow is the same as the flow for external providers,
so yes, this would enable the installation of dependencies for CRXes installed
by drap-and-drop.

I've filed a bug to fix this at
https://code.google.com/p/chromium/issues/detail?id=426356.

IMO, extensions installed via external providers should get any dependencies
installed; this includes policy configuration, child accounts, retail-mode
preinstalled extensions, etc.

+          !SharedModuleInfo::IsSharedModule(imported_module)) {
         ReportFailureFromUIThread(CrxInstallerError(l10n_util::GetStringFUTF16(
             IDS_EXTENSION_INSTALL_DEPENDENCY_NOT_SHARED_MODULE,
             base::UTF8ToUTF16(imported_module->name()))));
         return;
       } else if (imported_module && (version_required.IsValid() &&

[elijahtaylor1, 2014/10/22 16:45:17]

one rare-ish edge-case here to consider: It is possible for an app which imports
a shared module to require a newer version of the shared module than what is
installed (possibly because another app that depends on the module has been long
running and is keeping the update from happening).  

For the webstore: for new extensions you try to install, it gives you an error
message, which is kind of actionable.  For updated extensions which require a
newer SM, the update just silently fails and it will retry next update cycle.

If there is a policy change which requires a new App to be installed which
requires a newer SM, is that handled at least as gracefully as web store
installs (retry later)?

[Joao da Silva, 2014/10/22 18:31:05]

Yes. Basically requests to install an extension via policy will stay in the
pending install list, and every extension update check will try to install them.
This check is also always done a couple of minutes after every restart.

                                      imported_module->version()->CompareTo(
                                          version_required) < 0)) {
         ReportFailureFromUIThread(CrxInstallerError(l10n_util::GetStringFUTF16(
             IDS_EXTENSION_INSTALL_DEPENDENCY_OLD_VERSION,
             base::UTF8ToUTF16(imported_module->name()),
             base::ASCIIToUTF16(i->minimum_version),
             base::ASCIIToUTF16(imported_module->version()->GetString()))));
         return;
       } else if (imported_module &&
-          !SharedModuleInfo::IsExportAllowedByWhitelist(imported_module,
-                                                        extension()->id())) {
+                 !SharedModuleInfo::IsExportAllowedByWhitelist(
+                     imported_module, extension()->id())) {
         ReportFailureFromUIThread(CrxInstallerError(l10n_util::GetStringFUTF16(
             IDS_EXTENSION_INSTALL_DEPENDENCY_NOT_WHITELISTED,
             base::UTF8ToUTF16(extension()->name()),
             base::UTF8ToUTF16(imported_module->name()))));
         return;
       }
     }
   }
 
   // Run the policy, requirements and blacklist checks in parallel.
@@ skipping 384 matching lines @@
   if (!prefs->DidExtensionEscalatePermissions(extension()->id()))
     return;
 
   if (client_) {
     AddRef();  // Balanced in InstallUIProceed() and InstallUIAbort().
     client_->ConfirmReEnable(this, extension());
   }
 }
 
 }  // namespace extensions