Extract ResultCatcher from ExtensionApiTest. Use it in ShellApiTest, which is...

Extract ResultCatcher from ExtensionApiTest. Use it in ShellApiTest, which is separated out from AppShellTest.

Move the remaining DnsApiTest to app_shell_browsertests.

BUG=388893

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

Cr-Commit-Position: refs/heads/master@{#293517}

chrome/browser/extensions/api/dns/dns_apitest.cc

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "base/memory/ref_counted.h"
-#include "chrome/browser/extensions/extension_apitest.h"
-#include "extensions/browser/api/dns/host_resolver_wrapper.h"
-#include "extensions/browser/api/dns/mock_host_resolver_creator.h"
-#include "net/dns/mock_host_resolver.h"
-
-class DnsApiTest : public ExtensionApiTest {
- public:
-  DnsApiTest() : resolver_creator_(new extensions::MockHostResolverCreator()) {}
-
- private:
-  virtual void SetUpOnMainThread() OVERRIDE {
-    ExtensionApiTest::SetUpOnMainThread();
-    extensions::HostResolverWrapper::GetInstance()->SetHostResolverForTesting(
-        resolver_creator_->CreateMockHostResolver());
-  }
-
-  virtual void TearDownOnMainThread() OVERRIDE {
-    extensions::HostResolverWrapper::GetInstance()->
-        SetHostResolverForTesting(NULL);
-    resolver_creator_->DeleteMockHostResolver();
-    ExtensionApiTest::TearDownOnMainThread();
-  }
-
-  // The MockHostResolver asserts that it's used on the same thread on which
-  // it's created, which is actually a stronger rule than its real counterpart.
-  // But that's fine; it's good practice.
-  scoped_refptr<extensions::MockHostResolverCreator> resolver_creator_;
-};
-
-IN_PROC_BROWSER_TEST_F(DnsApiTest, DnsExtension) {
-  ASSERT_TRUE(RunExtensionTest("dns/api")) << message_;
-}

chrome/browser/extensions/extension_apitest.h

@@ -69,6 +69,7 @@ class ExtensionApiTest : public ExtensionBrowserTest {
   // GetNextResult() and message() if GetNextResult() return false. If there
   // are no results, this method will pump the UI message loop until one is
   // received.
+  // DEPRECATED: Use extensions/test/result_catcher.h instead.
   class ResultCatcher : public content::NotificationObserver {
    public:
     ResultCatcher();

chrome/chrome_tests.gypi

@@ -1101,7 +1101,6 @@
         'browser/extensions/api/desktop_capture/desktop_capture_apitest.cc',
         'browser/extensions/api/developer_private/developer_private_apitest.cc',
         'browser/extensions/api/dial/dial_apitest.cc',
-        'browser/extensions/api/dns/dns_apitest.cc',
         'browser/extensions/api/downloads/downloads_api_browsertest.cc',
         'browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc',
         'browser/extensions/api/extension_action/browser_action_apitest.cc',

extensions/browser/api/dns/dns_apitest.cc

@@ -5,26 +5,29 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/values.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/test/test_utils.h"
 #include "extensions/browser/api/dns/dns_api.h"
 #include "extensions/browser/api/dns/host_resolver_wrapper.h"
 #include "extensions/browser/api/dns/mock_host_resolver_creator.h"
 #include "extensions/browser/api_test_utils.h"
+#include "extensions/browser/notification_types.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/test_util.h"
-#include "extensions/shell/test/shell_test.h"
+#include "extensions/shell/test/shell_apitest.h"
 #include "net/base/net_errors.h"
 
 using extensions::api_test_utils::RunFunctionAndReturnSingleResult;
 
 namespace extensions {
 
-class DnsApiTest : public AppShellTest {
+class DnsApiTest : public ShellApiTest {
  public:
   DnsApiTest() : resolver_creator_(new MockHostResolverCreator()) {}
 
  private:
   virtual void SetUpOnMainThread() OVERRIDE {
-    AppShellTest::SetUpOnMainThread();
+    ShellApiTest::SetUpOnMainThread();
     HostResolverWrapper::GetInstance()->SetHostResolverForTesting(
         resolver_creator_->CreateMockHostResolver());
   }
@@ -32,7 +35,7 @@ class DnsApiTest : public AppShellTest {
   virtual void TearDownOnMainThread() OVERRIDE {
     HostResolverWrapper::GetInstance()->SetHostResolverForTesting(NULL);
     resolver_creator_->DeleteMockHostResolver();
-    AppShellTest::TearDownOnMainThread();
+    ShellApiTest::TearDownOnMainThread();
   }
 
   // The MockHostResolver asserts that it's used on the same thread on which
@@ -86,4 +89,8 @@ IN_PROC_BROWSER_TEST_F(DnsApiTest, DnsResolveHostname) {
   EXPECT_EQ(MockHostResolverCreator::kAddress, address);
 }
 
+IN_PROC_BROWSER_TEST_F(DnsApiTest, DnsExtension) {
+  ASSERT_TRUE(RunAppTest("api_test/dns/api")) << message_;
+}
+
 }  // namespace extensions

extensions/extensions.gyp

@@ -895,6 +895,8 @@
         'common/value_builder.h',
         'renderer/test_extensions_renderer_client.cc',
         'renderer/test_extensions_renderer_client.h',
+        'test/result_catcher.cc',
+        'test/result_catcher.h',
         'test/test_extensions_client.cc',
         'test/test_extensions_client.h',
         'test/test_permission_message_provider.cc',

extensions/shell/app_shell.gyp

@@ -187,8 +187,10 @@
         '../browser/api/sockets_tcp/sockets_tcp_apitest.cc',
         '../browser/api/sockets_udp/sockets_udp_apitest.cc',
         'browser/shell_browsertest.cc',
-        'test/shell_test.h',
+        'test/shell_apitest.cc',
+        'test/shell_apitest.h',
         'test/shell_test.cc',
+        'test/shell_test.h',
         'test/shell_test_launcher_delegate.cc',
         'test/shell_test_launcher_delegate.h',
         'test/shell_tests_main.cc',

extensions/shell/browser/shell_browsertest.cc

@@ -2,30 +2,17 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/files/file_path.h"
 #include "base/logging.h"
-#include "base/path_service.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/test/test_utils.h"
 #include "extensions/browser/notification_types.h"
-#include "extensions/common/extension_paths.h"
-#include "extensions/shell/test/shell_test.h"
+#include "extensions/shell/test/shell_apitest.h"
 
 namespace extensions {
 
 // Test that we can open an app window and wait for it to load.
-IN_PROC_BROWSER_TEST_F(AppShellTest, Basic) {
-  base::FilePath test_data_dir;
-
-  content::WindowedNotificationObserver test_pass_observer(
-      extensions::NOTIFICATION_EXTENSION_TEST_PASSED,
-      content::NotificationService::AllSources());
-
-  PathService::Get(extensions::DIR_TEST_DATA, &test_data_dir);
-  test_data_dir = test_data_dir.AppendASCII("platform_app");
-  LoadAndLaunchApp(test_data_dir);
-
-  test_pass_observer.Wait();
+IN_PROC_BROWSER_TEST_F(ShellApiTest, Basic) {
+  ASSERT_TRUE(RunAppTest("platform_app")) << message_;
 }
 
 }  // namespace extensions

extensions/shell/test/shell_apitest.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "extensions/shell/test/shell_apitest.h"
+
+#include "base/files/file_path.h"
+#include "base/path_service.h"
+#include "extensions/common/extension_paths.h"
+#include "extensions/shell/browser/shell_extension_system.h"
+#include "extensions/test/result_catcher.h"
+
+namespace extensions {
+
+ShellApiTest::ShellApiTest() {
+}
+
+ShellApiTest::~ShellApiTest() {
+}
+
+bool ShellApiTest::RunAppTest(const std::string& app_dir) {
+  base::FilePath test_data_dir;
+  PathService::Get(extensions::DIR_TEST_DATA, &test_data_dir);
+  test_data_dir = test_data_dir.AppendASCII(app_dir);
+  ResultCatcher catcher;
+
+  bool loaded = extension_system_->LoadApp(test_data_dir);
+  if (!loaded)
+    return false;
+
+  extension_system_->LaunchApp();
+
+  if (!catcher.GetNextResult()) {
+    message_ = catcher.message();
+    return false;
+  }
+
+  return true;
+}
+
+}  // namespace extensions

extensions/shell/test/shell_apitest.h

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef EXTENSIONS_SHELL_TEST_SHELL_API_TEST_H_
+#define EXTENSIONS_SHELL_TEST_SHELL_API_TEST_H_
+
+#include <string>
+
+#include "extensions/shell/test/shell_test.h"
+
+namespace extensions {
+
+// Base class for app shell browser API tests that load an app/extension
+// and wait for a success message from the chrome.test API.
+class ShellApiTest : public AppShellTest {
+ public:
+  ShellApiTest();
+  virtual ~ShellApiTest();
+
+  // Loads an unpacked platform app from a directory using the current
+  // ExtensionSystem, launches it, and waits for a chrome.test success
+  // notification. Returns true if the test succeeds. |app_dir| is a
+  // subpath under extensions/test/data.
+  bool RunAppTest(const std::string& app_dir);
+
+ protected:
+  // If it failed, what was the error message?
+  std::string message_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ShellApiTest);
+};
+
+}  // namespace extensions
+
+#endif  // EXTENSIONS_SHELL_TEST_SHELL_API_TEST_H_

extensions/shell/test/shell_test.cc

@@ -5,7 +5,6 @@
 #include "extensions/shell/test/shell_test.h"
 
 #include "base/command_line.h"
-#include "base/files/file_path.h"
 #include "base/logging.h"
 #include "content/public/common/content_switches.h"
 #include "extensions/browser/extension_system.h"
@@ -47,11 +46,4 @@ void AppShellTest::RunTestOnMainThreadLoop() {
   ShellDesktopController::instance()->CloseAppWindows();
 }
 
-bool AppShellTest::LoadAndLaunchApp(const base::FilePath& app_dir) {
-  bool loaded = extension_system_->LoadApp(app_dir);
-  if (loaded)
-    extension_system_->LaunchApp();
-  return loaded;
-}
-
 }  // namespace extensions

extensions/shell/test/shell_test.h

@@ -32,13 +32,9 @@ class AppShellTest : public content::BrowserTestBase {
   virtual void SetUpOnMainThread() OVERRIDE;
   virtual void RunTestOnMainThreadLoop() OVERRIDE;
 
-  // Loads an unpacked application from a directory using |extension_system_|
-  // and attempts to launch it. Returns true on success.
-  bool LoadAndLaunchApp(const base::FilePath& app_dir);
-
   content::BrowserContext* browser_context() { return browser_context_; }
 
- private:
+ protected:
   content::BrowserContext* browser_context_;
   ShellExtensionSystem* extension_system_;
 };

extensions/test/DEPS

 include_rules = [
+  "+content/public",
   "+mojo/embedder",
 ]

extensions/test/result_catcher.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "extensions/test/result_catcher.h"
+
+#include "content/public/browser/notification_service.h"
+#include "content/public/test/test_utils.h"
+#include "extensions/browser/notification_types.h"
+
+namespace extensions {
+
+ResultCatcher::ResultCatcher()
+    : browser_context_restriction_(NULL), waiting_(false) {
+  registrar_.Add(this,
+                 extensions::NOTIFICATION_EXTENSION_TEST_PASSED,
+                 content::NotificationService::AllSources());
+  registrar_.Add(this,
+                 extensions::NOTIFICATION_EXTENSION_TEST_FAILED,
+                 content::NotificationService::AllSources());
+}
+
+ResultCatcher::~ResultCatcher() {
+}
+
+bool ResultCatcher::GetNextResult() {
+  // Depending on the tests, multiple results can come in from a single call
+  // to RunMessageLoop(), so we maintain a queue of results and just pull them
+  // off as the test calls this, going to the run loop only when the queue is
+  // empty.
+  if (results_.empty()) {
+    waiting_ = true;
+    content::RunMessageLoop();
+    waiting_ = false;
+  }
+
+  if (!results_.empty()) {
+    bool ret = results_.front();
+    results_.pop_front();
+    message_ = messages_.front();
+    messages_.pop_front();
+    return ret;
+  }
+
+  NOTREACHED();
+  return false;
+}
+
+void ResultCatcher::Observe(int type,
+                            const content::NotificationSource& source,
+                            const content::NotificationDetails& details) {
+  if (browser_context_restriction_ &&
+      content::Source<content::BrowserContext>(source).ptr() !=
+          browser_context_restriction_) {
+    return;
+  }
+
+  switch (type) {
+    case extensions::NOTIFICATION_EXTENSION_TEST_PASSED:
+      VLOG(1) << "Got EXTENSION_TEST_PASSED notification.";
+      results_.push_back(true);
+      messages_.push_back(std::string());
+      if (waiting_)
+        base::MessageLoopForUI::current()->Quit();
+      break;
+
+    case extensions::NOTIFICATION_EXTENSION_TEST_FAILED:
+      VLOG(1) << "Got EXTENSION_TEST_FAILED notification.";
+      results_.push_back(false);
+      messages_.push_back(*(content::Details<std::string>(details).ptr()));
+      if (waiting_)
+        base::MessageLoopForUI::current()->Quit();
+      break;
+
+    default:
+      NOTREACHED();
+  }
+}
+
+}  // namespace extensions

extensions/test/result_catcher.h

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef EXTENSIONS_TEST_RESULT_CATCHER_H_
+#define EXTENSIONS_TEST_RESULT_CATCHER_H_
+
+#include <deque>
+#include <string>
+
+#include "base/compiler_specific.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
+
+namespace content {
+class BrowserContext;
+}  // namespace content
+
+namespace extensions {
+
+// Helper class that observes tests failing or passing. Observation starts
+// when the class is constructed. Get the next result by calling
+// GetNextResult() and message() if GetNextResult() return false. If there
+// are no results, this method will pump the UI message loop until one is
+// received.
+// TODO(yoz): Replace the version in ExtensionApiTest with this.
+class ResultCatcher : public content::NotificationObserver {
+ public:
+  ResultCatcher();
+  virtual ~ResultCatcher();
+
+  // Pumps the UI loop until a notification is received that an API test
+  // succeeded or failed. Returns true if the test succeeded, false otherwise.
+  bool GetNextResult();
+
+  void RestrictToBrowserContext(content::BrowserContext* context) {
+    browser_context_restriction_ = context;
+  }
+
+  const std::string& message() { return message_; }
+
+ private:
+  // content::NotificationObserver:
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
+
+  content::NotificationRegistrar registrar_;
+
+  // A sequential list of pass/fail notifications from the test extension(s).
+  std::deque<bool> results_;
+
+  // If it failed, what was the error message?
+  std::deque<std::string> messages_;
+  std::string message_;
+
+  // If non-NULL, we will listen to events from this BrowserContext only.
+  content::BrowserContext* browser_context_restriction_;
+
+  // True if we're in a nested message loop waiting for results from
+  // the extension.
+  bool waiting_;
+};
+
+}  // namespace extensions
+
+#endif  // EXTENSIONS_TEST_RESULT_CATCHER_H_