Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1113)

Unified Diff: chrome/browser/extensions/content_verifier_browsertest.cc

Issue 1243533005: Revert of Fix content verifier problem with content scripts (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | chrome/browser/extensions/extension_browsertest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 aac5cb55a13f4042b931959368748cbe78550025..a7b9ac8acf28c4b02be802f4f6e0ae78758c6c27 100644
--- a/chrome/browser/extensions/content_verifier_browsertest.cc
+++ b/chrome/browser/extensions/content_verifier_browsertest.cc
@@ -6,7 +6,6 @@
#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"
@@ -91,12 +90,10 @@
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,
- Result expected_result);
+ bool expected_to_fail);
// 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
@@ -113,38 +110,31 @@
private:
typedef std::pair<std::string, base::FilePath> ExtensionFile;
- typedef std::map<ExtensionFile, Result> ExpectedJobs;
+ typedef std::map<ExtensionFile, bool> ExpectedJobs;
ExpectedJobs expected_jobs_;
scoped_refptr<content::MessageLoopRunner> loop_runner_;
- // Used to track when jobs unexpectedly fail when expected to succeed, or
- // vice versa.
- int unexpected_job_results_;
- content::BrowserThread::ID creation_thread_;
+ bool saw_expected_job_results_;
};
void JobObserver::ExpectJobResult(const std::string& extension_id,
const base::FilePath& relative_path,
- Result expected_result) {
+ bool expected_to_fail) {
expected_jobs_.insert(std::make_pair(
- ExtensionFile(extension_id, relative_path), expected_result));
-}
-
-JobObserver::JobObserver() : unexpected_job_results_(0) {
- EXPECT_TRUE(
- content::BrowserThread::GetCurrentThreadIdentifier(&creation_thread_));
+ ExtensionFile(extension_id, relative_path), expected_to_fail));
+}
+
+JobObserver::JobObserver() : saw_expected_job_results_(false) {
}
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 unexpected_job_results_ == 0;
+ }
+ return saw_expected_job_results_;
}
void JobObserver::JobStarted(const std::string& extension_id,
@@ -154,72 +144,21 @@
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 != Result::FAILURE) ||
- (!failed && i->second != Result::SUCCESS)) {
- unexpected_job_results_ += 1;
+ 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();
}
}
-}
-
-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
@@ -236,12 +175,12 @@
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();
@@ -298,19 +237,15 @@
std::string id = "hoipipabpcoomfapcecilckodldhmpgl";
job_observer.ExpectJobResult(
- 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")),
- JobObserver::Result::SUCCESS);
- job_observer.ExpectJobResult(id,
- base::FilePath(FILE_PATH_LITERAL("page2.js")),
- JobObserver::Result::SUCCESS);
+ 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);
+ 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);
// Install a test extension we copied from the webstore that has actual
// signatures, and contains image paths with leading "./".
@@ -325,52 +260,4 @@
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
« no previous file with comments | « no previous file | chrome/browser/extensions/extension_browsertest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698