Fix content verifier problem with content scripts

chrome/browser/extensions/content_verifier_browsertest.cc

Index: chrome/browser/extensions/content_verifier_browsertest.cc
diff --git a/chrome/browser/extensions/content_verifier_browsertest.cc b/chrome/browser/extensions/content_verifier_browsertest.cc
index a7b9ac8acf28c4b02be802f4f6e0ae78758c6c27..aac5cb55a13f4042b931959368748cbe78550025 100644
--- a/chrome/browser/extensions/content_verifier_browsertest.cc
+++ b/chrome/browser/extensions/content_verifier_browsertest.cc
@@ -6,6 +6,7 @@
 #include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/common/chrome_switches.h"
 #include "content/public/test/test_utils.h"
+#include "extensions/browser/content_verifier.h"
 #include "extensions/browser/content_verify_job.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
@@ -90,10 +91,12 @@ class JobObserver : public ContentVerifyJob::TestObserver {
   JobObserver();
   virtual ~JobObserver();
 
+  enum class Result { SUCCESS, FAILURE };
+
   // Call this to add an expected job result.
   void ExpectJobResult(const std::string& extension_id,
                        const base::FilePath& relative_path,
-                       bool expected_to_fail);
+                       Result expected_result);
 
   // Wait to see expected jobs. Returns true if we saw all jobs finish as
   // expected, or false if any job completed with non-expected success/failure
@@ -110,31 +113,38 @@ class JobObserver : public ContentVerifyJob::TestObserver {
 
  private:
   typedef std::pair<std::string, base::FilePath> ExtensionFile;
-  typedef std::map<ExtensionFile, bool> ExpectedJobs;
+  typedef std::map<ExtensionFile, Result> ExpectedJobs;
   ExpectedJobs expected_jobs_;
   scoped_refptr<content::MessageLoopRunner> loop_runner_;
-  bool saw_expected_job_results_;
+  // Used to track when jobs unexpectedly fail when expected to succeed, or
+  // vice versa.
+  int unexpected_job_results_;
+  content::BrowserThread::ID creation_thread_;
 };
 
 void JobObserver::ExpectJobResult(const std::string& extension_id,
                                   const base::FilePath& relative_path,
-                                  bool expected_to_fail) {
+                                  Result expected_result) {
   expected_jobs_.insert(std::make_pair(
-      ExtensionFile(extension_id, relative_path), expected_to_fail));
+      ExtensionFile(extension_id, relative_path), expected_result));
 }
 
-JobObserver::JobObserver() : saw_expected_job_results_(false) {
+JobObserver::JobObserver() : unexpected_job_results_(0) {
+  EXPECT_TRUE(
+      content::BrowserThread::GetCurrentThreadIdentifier(&creation_thread_));
 }
 
 JobObserver::~JobObserver() {
 }
 
 bool JobObserver::WaitForExpectedJobs() {
+  EXPECT_TRUE(content::BrowserThread::CurrentlyOn(creation_thread_));
   if (!expected_jobs_.empty()) {
     loop_runner_ = new content::MessageLoopRunner();
     loop_runner_->Run();
+    loop_runner_ = nullptr;
   }
-  return saw_expected_job_results_;
+  return unexpected_job_results_ == 0;
 }
 
 void JobObserver::JobStarted(const std::string& extension_id,
@@ -144,23 +154,74 @@ void JobObserver::JobStarted(const std::string& extension_id,
 void JobObserver::JobFinished(const std::string& extension_id,
                               const base::FilePath& relative_path,
                               bool failed) {
+  if (!content::BrowserThread::CurrentlyOn(creation_thread_)) {
+    content::BrowserThread::PostTask(
+        creation_thread_, FROM_HERE,
+        base::Bind(&JobObserver::JobFinished, base::Unretained(this),
+                   extension_id, relative_path, failed));
+    return;
+  }
   ExpectedJobs::iterator i = expected_jobs_.find(ExtensionFile(
       extension_id, relative_path.NormalizePathSeparatorsTo('/')));
   if (i != expected_jobs_.end()) {
-    if (failed != i->second) {
-      saw_expected_job_results_ = false;
+    if ((failed && i->second != Result::FAILURE) ||
+        (!failed && i->second != Result::SUCCESS)) {
+      unexpected_job_results_ += 1;
       if (loop_runner_.get())
         loop_runner_->Quit();
     }
     expected_jobs_.erase(i);
     if (expected_jobs_.empty()) {
-      saw_expected_job_results_ = true;
       if (loop_runner_.get())
         loop_runner_->Quit();
     }
   }
 }
 
+class VerifierObserver : public ContentVerifier::TestObserver {
+ public:
+  VerifierObserver();
+  virtual ~VerifierObserver();
+
+  const std::set<std::string>& completed_fetches() {
+    return completed_fetches_;
+  }
+
+  // Returns when we've seen OnFetchComplete for |extension_id|.
+  void WaitForFetchComplete(const std::string& extension_id);
+
+  // ContentVerifier::TestObserver
+  void OnFetchComplete(const std::string& extension_id, bool success) override;
+
+ private:
+  std::set<std::string> completed_fetches_;
+  std::string id_to_wait_for_;
+  scoped_refptr<content::MessageLoopRunner> loop_runner_;
+};
+
+VerifierObserver::VerifierObserver() {
+}
+
+VerifierObserver::~VerifierObserver() {
+}
+
+void VerifierObserver::WaitForFetchComplete(const std::string& extension_id) {
+  EXPECT_TRUE(id_to_wait_for_.empty());
+  EXPECT_EQ(loop_runner_.get(), nullptr);
+  id_to_wait_for_ = extension_id;
+  loop_runner_ = new content::MessageLoopRunner();
+  loop_runner_->Run();
+  id_to_wait_for_.clear();
+  loop_runner_ = nullptr;
+}
+
+void VerifierObserver::OnFetchComplete(const std::string& extension_id,
+                                       bool success) {
+  completed_fetches_.insert(extension_id);
+  if (extension_id == id_to_wait_for_)
+    loop_runner_->Quit();
+}
+
 }  // namespace
 
 class ContentVerifierTest : public ExtensionBrowserTest {
@@ -175,12 +236,12 @@ class ContentVerifierTest : public ExtensionBrowserTest {
         switches::kExtensionContentVerificationEnforce);
   }
 
-  // Setup our unload observer and JobDelegate, and install a test extension.
   void SetUpOnMainThread() override {
     ExtensionBrowserTest::SetUpOnMainThread();
   }
 
   void TearDownOnMainThread() override {
+    ContentVerifier::SetObserverForTests(NULL);
     ContentVerifyJob::SetDelegateForTests(NULL);
     ContentVerifyJob::SetObserverForTests(NULL);
     ExtensionBrowserTest::TearDownOnMainThread();
@@ -237,15 +298,19 @@ IN_PROC_BROWSER_TEST_F(ContentVerifierTest, DotSlashPaths) {
   std::string id = "hoipipabpcoomfapcecilckodldhmpgl";
 
   job_observer.ExpectJobResult(
-      id, base::FilePath(FILE_PATH_LITERAL("background.js")), false);
-  job_observer.ExpectJobResult(
-      id, base::FilePath(FILE_PATH_LITERAL("page.html")), false);
-  job_observer.ExpectJobResult(
-      id, base::FilePath(FILE_PATH_LITERAL("page.js")), false);
+      id, base::FilePath(FILE_PATH_LITERAL("background.js")),
+      JobObserver::Result::SUCCESS);
+  job_observer.ExpectJobResult(id,
+                               base::FilePath(FILE_PATH_LITERAL("page.html")),
+                               JobObserver::Result::SUCCESS);
+  job_observer.ExpectJobResult(id, base::FilePath(FILE_PATH_LITERAL("page.js")),
+                               JobObserver::Result::SUCCESS);
   job_observer.ExpectJobResult(
-      id, base::FilePath(FILE_PATH_LITERAL("dir/page2.html")), false);
-  job_observer.ExpectJobResult(
-      id, base::FilePath(FILE_PATH_LITERAL("page2.js")), false);
+      id, base::FilePath(FILE_PATH_LITERAL("dir/page2.html")),
+      JobObserver::Result::SUCCESS);
+  job_observer.ExpectJobResult(id,
+                               base::FilePath(FILE_PATH_LITERAL("page2.js")),
+                               JobObserver::Result::SUCCESS);
 
   // Install a test extension we copied from the webstore that has actual
   // signatures, and contains image paths with leading "./".
@@ -260,4 +325,52 @@ IN_PROC_BROWSER_TEST_F(ContentVerifierTest, DotSlashPaths) {
   ContentVerifyJob::SetObserverForTests(NULL);
 }
 
+IN_PROC_BROWSER_TEST_F(ContentVerifierTest, ContentScripts) {
+  VerifierObserver verifier_observer;
+  ContentVerifier::SetObserverForTests(&verifier_observer);
+
+  // Install an extension with content scripts. The initial read of the content
+  // scripts will fail verification because they are read before the content
+  // verification system has completed a one-time processing of the expected
+  // hashes. (The extension only contains the root level hashes of the merkle
+  // tree, but the content verification system builds the entire tree and
+  // caches it in the extension install directory - see ContentHashFetcher for
+  // more details).
+  std::string id = "jmllhlobpjcnnomjlipadejplhmheiif";
+  const Extension* extension = InstallExtensionFromWebstore(
+      test_data_dir_.AppendASCII("content_verifier/content_script.crx"), 1);
+  ASSERT_TRUE(extension);
+  ASSERT_EQ(extension->id(), id);
+
+  // Wait for the content verification code to finish processing the hashes.
+  if (!ContainsKey(verifier_observer.completed_fetches(), id))
+    verifier_observer.WaitForFetchComplete(id);
+
+  // Now disable the extension, since content scripts are read at enable time,
+  // set up our job observer, and re-enable, expecting a success this time.
+  DisableExtension(id);
+  JobObserver job_observer;
+  ContentVerifyJob::SetObserverForTests(&job_observer);
+  job_observer.ExpectJobResult(id,
+                               base::FilePath(FILE_PATH_LITERAL("script.js")),
+                               JobObserver::Result::SUCCESS);
+  EnableExtension(id);
+  EXPECT_TRUE(job_observer.WaitForExpectedJobs());
+
+  // Now alter the contents of the content script, reload the extension, and
+  // expect to see a job failure due to the content script content hash not
+  // being what was signed by the webstore.
+  base::FilePath scriptfile = extension->path().AppendASCII("script.js");
+  std::string extra = "some_extra_function_call();";
+  ASSERT_TRUE(base::AppendToFile(scriptfile, extra.data(), extra.size()));
+  DisableExtension(id);
+  job_observer.ExpectJobResult(id,
+                               base::FilePath(FILE_PATH_LITERAL("script.js")),
+                               JobObserver::Result::FAILURE);
+  EnableExtension(id);
+  EXPECT_TRUE(job_observer.WaitForExpectedJobs());
+
+  ContentVerifyJob::SetObserverForTests(NULL);
+}
+
 }  // namespace extensions