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}

chrome/browser/extensions/api/declarative_net_request/declarative_net_request_browsertest.cc

@@ -31,7 +31,6 @@
 #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"
@@ -44,13 +43,12 @@
 #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"
@@ -131,6 +129,9 @@ 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:
@@ -184,6 +185,7 @@ class DeclarativeNetRequestBrowserTest
                             kJSONRulesFilename, rules, hosts,
                             has_background_script_);
 
+    background_page_ready_listener_->Reset();
     const Extension* extension = nullptr;
     switch (GetParam()) {
       case ExtensionLoadType::PACKED:
@@ -201,6 +203,10 @@ 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());
 
@@ -220,9 +226,16 @@ 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);
 };
@@ -1137,15 +1150,6 @@ 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(
@@ -1191,13 +1195,7 @@ IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest_Packed,
   ASSERT_TRUE(dnr_extension);
 
   // 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();
-  }
+  WaitForBackgroundScriptToLoad(dnr_extension->id());
 
   const char* script1 = R"(
     chrome.declarativeNetRequest.getWhitelistedPages(function(patterns) {

extensions/common/api/declarative_net_request/test_utils.cc

@@ -156,9 +156,9 @@ void WriteManifestAndRuleset(
   JSONFileValueSerializer(extension_dir.Append(json_rules_filepath))
       .Serialize(rules);
 
-  // Persists an empty background script if needed.
+  // Persists a background script if needed.
   if (has_background_script) {
-    std::string content;
+    std::string content = "chrome.test.sendMessage('ready');";
     CHECK_EQ(static_cast<int>(content.length()),
              base::WriteFile(extension_dir.Append(kBackgroundScriptFilepath),
                              content.c_str(), content.length()));

extensions/common/api/declarative_net_request/test_utils.h

@@ -84,8 +84,10 @@ 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, an empty background script
-// ("background.js") will also be persisted for the extension.
+// 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.
 void WriteManifestAndRuleset(
     const base::FilePath& extension_dir,
     const base::FilePath::CharType* json_rules_filepath,

extensions/test/extension_test_message_listener.cc

@@ -77,6 +77,7 @@ void ExtensionTestMessageListener::Reset() {
   satisfied_ = false;
   failed_ = false;
   message_.clear();
+  extension_id_for_message_.clear();
   replied_ = false;
 }
 
@@ -90,13 +91,19 @@ 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() && function->extension_id() != extension_id_)) {
+      (!extension_id_.empty() && sender_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();
@@ -107,6 +114,7 @@ void ExtensionTestMessageListener::Observe(
     // empty string.
     *message_details->second = true;
     message_ = message;
+    extension_id_for_message_ = sender_extension_id;
     satisfied_ = true;
     failed_ = (message_ == failure_message_);
 

extensions/test/extension_test_message_listener.h

@@ -11,6 +11,7 @@
 #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;
@@ -94,7 +95,8 @@ class ExtensionTestMessageListener : public content::NotificationObserver {
   ~ExtensionTestMessageListener() override;
 
   // This returns true immediately if we've already gotten the expected
-  // message, or waits until it arrives.
+  // message, or waits until it arrives. Once this returns true, message() and
+  // extension_id_for_message() accessors can be used.
   // 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;
@@ -127,6 +129,10 @@ 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,
@@ -167,6 +173,9 @@ 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_;
 };