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

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

Issue 1442593002: The AllUrlsApiTest.WhitelistedExtension test should no longer flake. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: The WhitelistedExtensionRunsOnExtensionPages test is enabled. Created 5 years, 1 month 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 | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/extensions/all_urls_apitest.cc
diff --git a/chrome/browser/extensions/all_urls_apitest.cc b/chrome/browser/extensions/all_urls_apitest.cc
index ef01f6a57222b0d1d5902087cd9bea1c33a70e0a..1989d4ef7785b916e5c05159f8c57049b21fd86a 100644
--- a/chrome/browser/extensions/all_urls_apitest.cc
+++ b/chrome/browser/extensions/all_urls_apitest.cc
@@ -22,23 +22,50 @@ namespace {
const std::string kAllUrlsTarget = "/extensions/api_test/all_urls/index.html";
}
-class AllUrlsApiTest : public ExtensionApiTest {
+class AllUrlsApiTest : public ExtensionApiTest,
+ public ExtensionRegistryObserver {
protected:
- AllUrlsApiTest() {}
+ AllUrlsApiTest() : wait_until_reload_(false),
+ content_script_is_reloaded_(false),
+ execute_script_is_reloaded_(false) {}
~AllUrlsApiTest() override {}
const Extension* content_script() const { return content_script_.get(); }
const Extension* execute_script() const { return execute_script_.get(); }
+ // ExtensionRegistryObserver implementation
+ void OnExtensionLoaded(
+ content::BrowserContext*,
+ const Extension* extension) override {
+ if (!wait_until_reload_)
+ return;
+
+ if (extension->id() == content_script_->id())
+ content_script_is_reloaded_ = true;
+ else if (extension->id() == execute_script_->id())
+ execute_script_is_reloaded_ = true;
+ if (content_script_is_reloaded_ && execute_script_is_reloaded_) {
+ base::MessageLoop::current()->QuitWhenIdle();
+ wait_until_reload_ = false;
+ }
+ }
+
void WhitelistExtensions() {
+ ScopedObserver<ExtensionRegistry, ExtensionRegistryObserver> observer(this);
+ observer.Add(ExtensionRegistry::Get(browser()->profile()));
+
ExtensionsClient::ScriptingWhitelist whitelist;
whitelist.push_back(content_script_->id());
whitelist.push_back(execute_script_->id());
ExtensionsClient::Get()->SetScriptingWhitelist(whitelist);
// Extensions will have certain permissions withheld at initialization if
// they aren't whitelisted, so we need to reload them.
+ content_script_is_reloaded_ = false;
+ execute_script_is_reloaded_ = false;
+ wait_until_reload_ = true;
extension_service()->ReloadExtension(content_script_->id());
extension_service()->ReloadExtension(execute_script_->id());
+ base::MessageLoop::current()->Run();
}
private:
@@ -54,17 +81,14 @@ class AllUrlsApiTest : public ExtensionApiTest {
scoped_refptr<const Extension> content_script_;
scoped_refptr<const Extension> execute_script_;
+ bool wait_until_reload_;
+ bool content_script_is_reloaded_;
+ bool execute_script_is_reloaded_;
+
DISALLOW_COPY_AND_ASSIGN(AllUrlsApiTest);
};
-#if (defined(OS_WIN) && !defined(NDEBUG)) || defined(OS_CHROMEOS) || \
- (defined(OS_MACOSX) && defined(ADDRESS_SANITIZER))
-// http://crbug.com/174341
-#define MAYBE_WhitelistedExtension DISABLED_WhitelistedExtension
-#else
-#define MAYBE_WhitelistedExtension WhitelistedExtension
-#endif
-IN_PROC_BROWSER_TEST_F(AllUrlsApiTest, MAYBE_WhitelistedExtension) {
+IN_PROC_BROWSER_TEST_F(AllUrlsApiTest, WhitelistedExtension) {
#if defined(OS_WIN) && defined(USE_ASH)
// Disable this test in Metro+Ash for now (http://crbug.com/262796).
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
@@ -134,18 +158,14 @@ IN_PROC_BROWSER_TEST_F(AllUrlsApiTest, RegularExtensions) {
}
// Disabled because sometimes bystander doesn't load.
-// TODO(devlin): Why?
IN_PROC_BROWSER_TEST_F(AllUrlsApiTest,
- DISABLED_WhitelistedExtensionRunsOnExtensionPages) {
+ WhitelistedExtensionRunsOnExtensionPages) {
WhitelistExtensions();
const Extension* bystander =
LoadExtension(test_data_dir_.AppendASCII("all_urls")
.AppendASCII("bystander"));
ASSERT_TRUE(bystander);
- // TODO(devlin): This test should probably go in the WhitelistedExtension test
- // above, but that one has a bunch of disableds, so it wouldn't be very
- // useful.
GURL url(bystander->GetResourceURL("page.html"));
ExtensionTestMessageListener listenerA(
"content script: " + url.spec(), false);
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698