Enforce the 'requirements' field in manifests.

chrome/browser/extensions/crx_installer.cc

Index: chrome/browser/extensions/crx_installer.cc
diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc
index dcb1046cfc7d58fc4488769287b85110b2fe3f74..33a0eb432323a7f4eae114b90a6a1e2ef0718677 100644
--- a/chrome/browser/extensions/crx_installer.cc
+++ b/chrome/browser/extensions/crx_installer.cc
@@ -16,6 +16,7 @@
 #include "base/scoped_temp_dir.h"
 #include "base/stl_util.h"
 #include "base/stringprintf.h"
+#include "base/string_util.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
@@ -29,6 +30,7 @@
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/extensions/permissions_updater.h"
+#include "chrome/browser/extensions/requirements_checker.h"
 #include "chrome/browser/extensions/webstore_installer.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/shell_integration.h"
@@ -101,7 +103,9 @@ CrxInstaller::CrxInstaller(
       creation_flags_(Extension::NO_FLAGS),
       off_store_install_allow_reason_(OffStoreInstallDisallowed),
       did_handle_successfully_(true),
-      record_oauth2_grant_(false) {
+      record_oauth2_grant_(false),
+      error_on_unsported_requirements_(false),
+      requirements_checker_(new extensions::RequirementsChecker()) {
   if (!approval)
     return;
 
@@ -384,6 +388,30 @@ void CrxInstaller::OnUnpackSuccess(const FilePath& temp_dir,
                           &install_icon_);
   }
 
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+      base::Bind(&CrxInstaller::RunRequirementsChecker, this));
+}
+
+void CrxInstaller::RunRequirementsChecker() {
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  requirements_checker_->Check(extension_,
+                               base::Bind(&CrxInstaller::RequirementsChecked,

[Aaron Boodman, 2012/08/01 03:58:54]

Why run this on the file thread? It seems like this is just introducing more
thread hops. If you have it call back on UI thread, then RequirementsCheck can
just directly call ConfirmInstall() rather than PostTask'ing to it.

[eaugusti, 2012/08/03 01:06:26]

Done.

+                                          this),
+                               BrowserThread::FILE);
+}
+
+void CrxInstaller::RequirementsChecked(std::vector<std::string> errors) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+
+  if (!errors.empty()) {
+    if (error_on_unsported_requirements_) {
+      ReportFailureFromFileThread(CrxInstallerError(
+          UTF8ToUTF16(JoinString(errors, ' '))));
+      return;
+    }
+    requirement_errors_.swap(errors);
+  }
+
   if (!BrowserThread::PostTask(
           BrowserThread::UI, FROM_HERE,
           base::Bind(&CrxInstaller::ConfirmInstall, this)))
@@ -520,6 +548,18 @@ void CrxInstaller::CompleteInstall() {
       &error);
   CHECK(error.empty()) << error;
 
+  if (!requirement_errors_.empty()) {
+    extensions::Extension::InstallWarningVector install_warnings;
+    std::vector<std::string>::iterator it;

[Aaron Boodman, 2012/08/01 03:58:54]

Since you don't need |it| outside the for loop define it there.

[eaugusti, 2012/08/03 01:06:26]

Done.

+    for (it = requirement_errors_.begin(); it != requirement_errors_.end();
+         ++it) {
+      install_warnings.push_back(Extension::InstallWarning(
+          Extension::InstallWarning::FORMAT_TEXT, *it));
+    }
+    const_cast<Extension*>(extension_.get())->AddInstallWarnings(

[Aaron Boodman, 2012/08/01 03:58:54]

You can avoid this const_cast by de-consting extension_ and following that
thread. It ends not to far away at SandboxedUnpacker::Client::OnUnpackSucceeded.
The Extension* that is passed into that is non-const.

None of these actually need to be const at this point in extension's lifetime.
We only need it to be const when it gets added to ExtensionService because at
that point it can be modified on multiple threads concurrently.

[Aaron Boodman, 2012/08/01 04:03:57]

s/modified/accessed/

[eaugusti, 2012/08/03 01:06:26]

Done.

+        install_warnings);
+  }
+
   ReportSuccessFromFileThread();
 }
 
@@ -602,8 +642,10 @@ void CrxInstaller::ReportSuccessFromUIThread() {
 
   // Tell the frontend about the installation and hand off ownership of
   // extension_ to it.
-  frontend_weak_->OnExtensionInstalled(extension_, is_gallery_install(),
-                                       page_ordinal_);
+  frontend_weak_->OnExtensionInstalled(extension_,
+                                       is_gallery_install(),
+                                       page_ordinal_,
+                                       requirement_errors_);
 
   NotifyCrxInstallComplete(extension_.get());