Commit b87349ae authored by Marc Treib's avatar Marc Treib Committed by Commit Bot

Revert "DNR tests: Use ExtensionTestMessageListener to detect background page load."

This reverts commit 6e6741a1.

Reason for revert: DeclarativeNetRequestBrowserTest_Packed.PageWhitelistingAPI became flaky on ChromeOS debug and ASan/LSan:
https://ci.chromium.org/buildbot/chromium.memory/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/?limit=100
https://ci.chromium.org/buildbot/chromium.chromiumos/linux-chromeos-dbg/?limit=100
https://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=DeclarativeNetRequestBrowserTest_Packed.*PageWhitelist

Original change's description:
> DNR tests: Use ExtensionTestMessageListener to detect background page load.
>
> This CL changes declarative net request browsertests to use
> ExtensionTestMessageListener to detect background page load instead of relying
> on the notification system, which is deprecated. Also, add a
> extension_id_for_message() method to ExtensionTestMessageListener. This is
> useful to detect the extension id, the message was received from.
>
> BUG=696822
>
> Change-Id: Ie1fc74d547d4ee417c5f6b2d5a7082adbc9d164a
> Reviewed-on: https://chromium-review.googlesource.com/1034155
> Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
> Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#555005}

TBR=lazyboy@chromium.org,karandeepb@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 696822, 838536
Change-Id: Ic714040d602db9e0a4abb266c0f84adb10029625
Reviewed-on: https://chromium-review.googlesource.com/1041946
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555691}
parent 674171d9
......@@ -31,6 +31,7 @@
#include "components/proxy_config/proxy_config_pref_names.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/notification_source.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/test_utils.h"
......@@ -43,12 +44,13 @@
#include "extensions/browser/extension_registry.h"
#include "extensions/browser/extension_system.h"
#include "extensions/browser/extension_util.h"
#include "extensions/browser/notification_types.h"
#include "extensions/browser/runtime_data.h"
#include "extensions/common/api/declarative_net_request/constants.h"
#include "extensions/common/api/declarative_net_request/test_utils.h"
#include "extensions/common/constants.h"
#include "extensions/common/extension_id.h"
#include "extensions/common/url_pattern.h"
#include "extensions/test/extension_test_message_listener.h"
#include "net/dns/mock_host_resolver.h"
#include "net/test/spawned_test_server/spawned_test_server.h"
#include "net/test/test_data_directory.h"
......@@ -129,9 +131,6 @@ class DeclarativeNetRequestBrowserTest
host_resolver()->AddRule("*", "127.0.0.1");
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
background_page_ready_listener_ =
std::make_unique<ExtensionTestMessageListener>("ready",
false /*will_reply*/);
}
protected:
......@@ -185,7 +184,6 @@ class DeclarativeNetRequestBrowserTest
kJSONRulesFilename, rules, hosts,
has_background_script_);
background_page_ready_listener_->Reset();
const Extension* extension = nullptr;
switch (GetParam()) {
case ExtensionLoadType::PACKED:
......@@ -203,10 +201,6 @@ class DeclarativeNetRequestBrowserTest
// Ensure the ruleset is also loaded on the IO thread.
content::RunAllTasksUntilIdle();
// Wait for the background page to load if needed.
if (has_background_script_)
WaitForBackgroundScriptToLoad(extension->id());
// Ensure no load errors were reported.
EXPECT_TRUE(LoadErrorReporter::GetInstance()->GetErrors()->empty());
......@@ -226,16 +220,9 @@ class DeclarativeNetRequestBrowserTest
{URLPattern::kAllUrlsPattern});
}
void WaitForBackgroundScriptToLoad(const ExtensionId& extension_id) {
ASSERT_TRUE(background_page_ready_listener_->WaitUntilSatisfied());
ASSERT_EQ(extension_id,
background_page_ready_listener_->extension_id_for_message());
}
private:
base::ScopedTempDir temp_dir_;
bool has_background_script_ = false;
std::unique_ptr<ExtensionTestMessageListener> background_page_ready_listener_;
DISALLOW_COPY_AND_ASSIGN(DeclarativeNetRequestBrowserTest);
};
......@@ -1150,6 +1137,15 @@ IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest_Packed,
ASSERT_TRUE(dnr_extension);
EXPECT_EQ("Test extension", dnr_extension->name());
// Ensure the background page is ready before dispatching the script to it.
if (!ExtensionSystem::Get(profile())->runtime_data()->IsBackgroundPageReady(
dnr_extension)) {
content::WindowedNotificationObserver(
NOTIFICATION_EXTENSION_BACKGROUND_PAGE_READY,
content::Source<Extension>(dnr_extension))
.Wait();
}
// Whitelist "https://www.google.com/".
const char* script1 = R"(
chrome.declarativeNetRequest.addWhitelistedPages(
......@@ -1195,7 +1191,13 @@ IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest_Packed,
ASSERT_TRUE(dnr_extension);
// Ensure the background page is ready before dispatching the script to it.
WaitForBackgroundScriptToLoad(dnr_extension->id());
if (!ExtensionSystem::Get(profile())->runtime_data()->IsBackgroundPageReady(
dnr_extension)) {
content::WindowedNotificationObserver(
NOTIFICATION_EXTENSION_BACKGROUND_PAGE_READY,
content::Source<Extension>(dnr_extension))
.Wait();
}
const char* script1 = R"(
chrome.declarativeNetRequest.getWhitelistedPages(function(patterns) {
......
......@@ -156,9 +156,9 @@ void WriteManifestAndRuleset(
JSONFileValueSerializer(extension_dir.Append(json_rules_filepath))
.Serialize(rules);
// Persists a background script if needed.
// Persists an empty background script if needed.
if (has_background_script) {
std::string content = "chrome.test.sendMessage('ready');";
std::string content;
CHECK_EQ(static_cast<int>(content.length()),
base::WriteFile(extension_dir.Append(kBackgroundScriptFilepath),
content.c_str(), content.length()));
......
......@@ -84,10 +84,8 @@ std::unique_ptr<base::ListValue> ToListValue(
// Writes the declarative |rules| in the given |extension_dir| together with the
// manifest file. |hosts| specifies the host permissions, the extensions should
// have. If |has_background_script| is true, a background script
// ("background.js") will also be persisted for the extension. Clients can
// listen in to the "ready" message from the background page to detect its
// loading.
// have. If |has_background_script| is true, an empty background script
// ("background.js") will also be persisted for the extension.
void WriteManifestAndRuleset(
const base::FilePath& extension_dir,
const base::FilePath::CharType* json_rules_filepath,
......
......@@ -76,7 +76,6 @@ void ExtensionTestMessageListener::Reset() {
satisfied_ = false;
failed_ = false;
message_.clear();
extension_id_for_message_.clear();
replied_ = false;
}
......@@ -90,19 +89,13 @@ void ExtensionTestMessageListener::Observe(
// extension.
extensions::TestSendMessageFunction* function =
content::Source<extensions::TestSendMessageFunction>(source).ptr();
std::string sender_extension_id;
if (function->extension())
sender_extension_id = function->extension_id();
if (satisfied_ ||
(!extension_id_.empty() && sender_extension_id != extension_id_)) {
(!extension_id_.empty() && function->extension_id() != extension_id_)) {
return;
}
// We should have an empty message if we're not already satisfied.
CHECK(message_.empty());
CHECK(extension_id_for_message_.empty());
std::pair<std::string, bool*>* message_details =
content::Details<std::pair<std::string, bool*>>(details).ptr();
......@@ -113,7 +106,6 @@ void ExtensionTestMessageListener::Observe(
// empty string.
*message_details->second = true;
message_ = message;
extension_id_for_message_ = sender_extension_id;
satisfied_ = true;
failed_ = (message_ == failure_message_);
......
......@@ -11,7 +11,6 @@
#include "base/memory/ref_counted.h"
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
#include "extensions/common/extension_id.h"
namespace extensions {
class TestSendMessageFunction;
......@@ -95,8 +94,7 @@ class ExtensionTestMessageListener : public content::NotificationObserver {
~ExtensionTestMessageListener() override;
// This returns true immediately if we've already gotten the expected
// message, or waits until it arrives. Once this returns true, message() and
// extension_id_for_message() accessors can be used.
// message, or waits until it arrives.
// Returns false if the wait is interrupted and we still haven't gotten the
// message, or if the message was equal to |failure_message_|.
bool WaitUntilSatisfied() WARN_UNUSED_RESULT;
......@@ -129,10 +127,6 @@ class ExtensionTestMessageListener : public content::NotificationObserver {
const std::string& message() const { return message_; }
const extensions::ExtensionId& extension_id_for_message() const {
return extension_id_for_message_;
}
private:
// Implements the content::NotificationObserver interface.
void Observe(int type,
......@@ -173,9 +167,6 @@ class ExtensionTestMessageListener : public content::NotificationObserver {
// If we received a message that was the failure message.
bool failed_;
// The extension id from which |message_| was received.
extensions::ExtensionId extension_id_for_message_;
// The function we need to reply to.
scoped_refptr<extensions::TestSendMessageFunction> function_;
};
......
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