Restrict use of two app-launching command line flags

chrome/browser/extensions/unpacked_installer.cc

Index: chrome/browser/extensions/unpacked_installer.cc
diff --git a/chrome/browser/extensions/unpacked_installer.cc b/chrome/browser/extensions/unpacked_installer.cc
index f812a2f5f368e2391f6404ffb8128699c844337f..4b8ad21a173bf39c6b305f394258efb30b49324d 100644
--- a/chrome/browser/extensions/unpacked_installer.cc
+++ b/chrome/browser/extensions/unpacked_installer.cc
@@ -40,6 +40,10 @@ namespace {
 const char kUnpackedExtensionsBlacklistedError[] =
     "Loading of unpacked extensions is disabled by the administrator.";
 
+const char kUnpackedExtensionInsteadOfAppError[] =
+    "App loading flags cannot be used to load extensions. Please use "
+    "--load-extension instead.";
+
 const char kImportMinVersionNewer[] =
     "'import' version requested is newer than what is installed.";
 const char kImportMissing[] = "'import' extension is not installed.";
@@ -130,7 +134,8 @@ void UnpackedInstaller::Load(const base::FilePath& path_in) {
 }
 
 bool UnpackedInstaller::LoadFromCommandLine(const base::FilePath& path_in,
-                                            std::string* extension_id) {
+                                            std::string* extension_id,
+                                            bool only_allow_apps) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(extension_path_.empty());
 
@@ -152,6 +157,16 @@ bool UnpackedInstaller::LoadFromCommandLine(const base::FilePath& path_in,
       file_util::LoadExtension(
           extension_path_, Manifest::COMMAND_LINE, GetFlags(), &error).get());
 
+  if (only_allow_apps && !extension()->is_app()) {

[Devlin, 2016/06/29 14:59:54]

If we don't opt for the std::set<Type> allowed_types approach, we should
restrict this to is_platform_app(), rather than is_app().  We don't want to
allow loading of legacy packaged apps here.

Otherwise, this can just be allowed_types.count(extension()->type()).

[proberge, 2016/06/29 20:56:49]

Done.

+#if defined(GOOGLE_CHROME_BUILD)
+    // Avoid crashing for users with hijacked shortcuts.

[Devlin, 2016/06/29 14:59:54]

I can understand the motivation to avoid crashing, but if this fails, we just
pop up a dialog saying the extension can't be loaded.  Since that doesn't brick
the user's chrome, it seems like it would also serve as an indicator that
something was wrong, and allow them to fix it.  If a user has one bad
commandline flag, it makes sense they might have more issues, so I'd rather we
let them know so they can take action to get back into a good state, rather than
just try to continue gracefully and give no indication.

[proberge, 2016/06/29 20:56:49]

The comment is to explain the "return true;". If we "return false", Chrome stops
trying to load (and actually crashes during shutdown), effectively bricking the
user's Chrome unless they know how to edit command line flags. 

I'm open to having ReportExtensionLoadError in both cases, but I'm worried that
for most users this will just be an annoying popup on every Chrome launch that
they don't know how to (and will never) fix. 

+    return true;
+#else
+    ReportExtensionLoadError(kUnpackedExtensionInsteadOfAppError);
+    return false;
+#endif
+  }
+
   if (!extension() ||
       !extension_l10n_util::ValidateExtensionLocales(
           extension_path_, extension()->manifest()->value(), &error)) {