Commit 0f33acbb authored by magjed's avatar magjed Committed by Commit bot

Revert of The AllUrlsApiTest.WhitelistedExtension test should no longer flake....

Revert of The AllUrlsApiTest.WhitelistedExtension test should no longer flake. (patchset #3 id:40001 of https://codereview.chromium.org/1442593002/ )

Reason for revert:
The WhitelistedExtension is still flaky on the XP Tests bot, see e.g. https://build.chromium.org/p/chromium.win/builders/XP%20Tests%20%281%29/builds/41174
stdio output:
[ RUN      ] AllUrlsApiTest.WhitelistedExtensionRunsOnExtensionPages
[1636:4084:1120/124530:WARNING:raw_channel.cc(208)] Shutting down RawChannel with write buffer nonempty
[139/464] AllUrlsApiTest.WhitelistedExtensionRunsOnExtensionPages (TIMED OUT)

Original issue's description:
> The AllUrlsApiTest.WhitelistedExtension test should no longer flake.
>
> The test had begun to hang after the refactoring that changed the way the extensions are loaded and reloaded. As a result of that refactoring, the test loaded the pages and tried to get some response from the extensions before they had time to reload.
>
> This change introduces waiting for the extensions reload, so the test should no longer flake.
>
> BUG=174341
>
> R=finnur@chromium.org
>
> Committed: https://crrev.com/454083864ede93660c2f08cf7d0e06ca10013c0d
> Cr-Commit-Position: refs/heads/master@{#359537}

TBR=finnur@chromium.org,voodoo@yandex-team.ru
NOPRESUBMIT=true
NOTREECHECKS=true
BUG=174341

Review URL: https://codereview.chromium.org/1469813003

Cr-Commit-Position: refs/heads/master@{#361090}
parent b2e15d28
...@@ -22,50 +22,23 @@ namespace { ...@@ -22,50 +22,23 @@ namespace {
const std::string kAllUrlsTarget = "/extensions/api_test/all_urls/index.html"; const std::string kAllUrlsTarget = "/extensions/api_test/all_urls/index.html";
} }
class AllUrlsApiTest : public ExtensionApiTest, class AllUrlsApiTest : public ExtensionApiTest {
public ExtensionRegistryObserver {
protected: protected:
AllUrlsApiTest() : wait_until_reload_(false), AllUrlsApiTest() {}
content_script_is_reloaded_(false),
execute_script_is_reloaded_(false) {}
~AllUrlsApiTest() override {} ~AllUrlsApiTest() override {}
const Extension* content_script() const { return content_script_.get(); } const Extension* content_script() const { return content_script_.get(); }
const Extension* execute_script() const { return execute_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() { void WhitelistExtensions() {
ScopedObserver<ExtensionRegistry, ExtensionRegistryObserver> observer(this);
observer.Add(ExtensionRegistry::Get(browser()->profile()));
ExtensionsClient::ScriptingWhitelist whitelist; ExtensionsClient::ScriptingWhitelist whitelist;
whitelist.push_back(content_script_->id()); whitelist.push_back(content_script_->id());
whitelist.push_back(execute_script_->id()); whitelist.push_back(execute_script_->id());
ExtensionsClient::Get()->SetScriptingWhitelist(whitelist); ExtensionsClient::Get()->SetScriptingWhitelist(whitelist);
// Extensions will have certain permissions withheld at initialization if // Extensions will have certain permissions withheld at initialization if
// they aren't whitelisted, so we need to reload them. // 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(content_script_->id());
extension_service()->ReloadExtension(execute_script_->id()); extension_service()->ReloadExtension(execute_script_->id());
base::MessageLoop::current()->Run();
} }
private: private:
...@@ -81,14 +54,17 @@ class AllUrlsApiTest : public ExtensionApiTest, ...@@ -81,14 +54,17 @@ class AllUrlsApiTest : public ExtensionApiTest,
scoped_refptr<const Extension> content_script_; scoped_refptr<const Extension> content_script_;
scoped_refptr<const Extension> execute_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); DISALLOW_COPY_AND_ASSIGN(AllUrlsApiTest);
}; };
IN_PROC_BROWSER_TEST_F(AllUrlsApiTest, WhitelistedExtension) { #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) {
#if defined(OS_WIN) && defined(USE_ASH) #if defined(OS_WIN) && defined(USE_ASH)
// Disable this test in Metro+Ash for now (http://crbug.com/262796). // Disable this test in Metro+Ash for now (http://crbug.com/262796).
if (base::CommandLine::ForCurrentProcess()->HasSwitch( if (base::CommandLine::ForCurrentProcess()->HasSwitch(
...@@ -158,14 +134,18 @@ IN_PROC_BROWSER_TEST_F(AllUrlsApiTest, RegularExtensions) { ...@@ -158,14 +134,18 @@ IN_PROC_BROWSER_TEST_F(AllUrlsApiTest, RegularExtensions) {
} }
// Disabled because sometimes bystander doesn't load. // Disabled because sometimes bystander doesn't load.
// TODO(devlin): Why?
IN_PROC_BROWSER_TEST_F(AllUrlsApiTest, IN_PROC_BROWSER_TEST_F(AllUrlsApiTest,
WhitelistedExtensionRunsOnExtensionPages) { DISABLED_WhitelistedExtensionRunsOnExtensionPages) {
WhitelistExtensions(); WhitelistExtensions();
const Extension* bystander = const Extension* bystander =
LoadExtension(test_data_dir_.AppendASCII("all_urls") LoadExtension(test_data_dir_.AppendASCII("all_urls")
.AppendASCII("bystander")); .AppendASCII("bystander"));
ASSERT_TRUE(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")); GURL url(bystander->GetResourceURL("page.html"));
ExtensionTestMessageListener listenerA( ExtensionTestMessageListener listenerA(
"content script: " + url.spec(), false); "content script: " + url.spec(), false);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment