Fix content verifier problem with content scripts (reland)

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..91c6629b242a5f6d80ad2c3db12ce850cdeaafda 100644
--- a/chrome/browser/extensions/content_verifier_browsertest.cc
+++ b/chrome/browser/extensions/content_verifier_browsertest.cc
@@ -2,10 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <list>
+#include <set>
+#include <string>
+
 #include "base/scoped_observer.h"
 #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,14 +95,15 @@ 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
-  // status.
+  // Wait to see expected jobs. Returns true when we've seen all expected jobs
+  // finish, or false if there was an error or timeout.
   bool WaitForExpectedJobs();
 
   // ContentVerifyJob::TestObserver interface
@@ -109,32 +115,47 @@ class JobObserver : public ContentVerifyJob::TestObserver {
                    bool failed) override;
 
  private:
-  typedef std::pair<std::string, base::FilePath> ExtensionFile;
-  typedef std::map<ExtensionFile, bool> ExpectedJobs;
-  ExpectedJobs expected_jobs_;
+  struct ExpectedResult {
+   public:
+    std::string extension_id;
+    base::FilePath path;
+    Result result;
+
+    ExpectedResult(const std::string& extension_id, const base::FilePath& path,
+                   Result result) {
+      this->extension_id = extension_id;
+      this->path = path;
+      this->result = result;
+    }
+  };
+  std::list<ExpectedResult> expectations_;
+  content::BrowserThread::ID creation_thread_;
   scoped_refptr<content::MessageLoopRunner> loop_runner_;
-  bool saw_expected_job_results_;
 };
 
 void JobObserver::ExpectJobResult(const std::string& extension_id,
                                   const base::FilePath& relative_path,
-                                  bool expected_to_fail) {
-  expected_jobs_.insert(std::make_pair(
-      ExtensionFile(extension_id, relative_path), expected_to_fail));
+                                  Result expected_result) {
+  expectations_.push_back(ExpectedResult(
+      extension_id, relative_path, expected_result));
 }
 
-JobObserver::JobObserver() : saw_expected_job_results_(false) {
+JobObserver::JobObserver() {
+  EXPECT_TRUE(
+      content::BrowserThread::GetCurrentThreadIdentifier(&creation_thread_));
 }
 
 JobObserver::~JobObserver() {
 }
 
 bool JobObserver::WaitForExpectedJobs() {
-  if (!expected_jobs_.empty()) {
+  EXPECT_TRUE(content::BrowserThread::CurrentlyOn(creation_thread_));
+  if (!expectations_.empty()) {
     loop_runner_ = new content::MessageLoopRunner();
     loop_runner_->Run();
+    loop_runner_ = nullptr;
   }
-  return saw_expected_job_results_;
+  return expectations_.empty();
 }
 
 void JobObserver::JobStarted(const std::string& extension_id,
@@ -144,21 +165,75 @@ void JobObserver::JobStarted(const std::string& extension_id,
 void JobObserver::JobFinished(const std::string& extension_id,
                               const base::FilePath& relative_path,
                               bool failed) {
-  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 (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();
+  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;
+  }
+  Result result = failed ? Result::FAILURE : Result::SUCCESS;
+  bool found = false;
+  for (std::list<ExpectedResult>::iterator i = expectations_.begin();
+       i != expectations_.end(); ++i) {
+    if (i->extension_id == extension_id && i->path == relative_path &&
+        i->result == result) {
+      found = true;
+      expectations_.erase(i);
+      break;
     }
   }
+  if (found) {
+    if (expectations_.empty() && loop_runner_.get())
+      loop_runner_->Quit();
+  } else {
+    LOG(WARNING) << "Ignoring unexpected JobFinished " << extension_id << "/"
+                 << relative_path.value() << " failed:" << failed;
+  }
+}
+
+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
@@ -175,12 +250,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 +312,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 +339,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