Ensure tests have an active task runner

chrome/browser/background/background_mode_manager_unittest.cc

 // Copyright (c) 2011 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/bind.h"
 #include "base/command_line.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/test_simple_task_runner.h"
+#include "base/thread_task_runner_handle.h"
 #include "chrome/browser/background/background_mode_manager.h"
 #include "chrome/browser/browser_shutdown.h"
 #include "chrome/browser/extensions/extension_function_test_utils.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/test_extension_system.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/browser/profiles/profile_info_cache.h"
 #include "chrome/browser/status_icons/status_icon_menu_model.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/base/testing_browser_process.h"
@@ skipping 17 matching lines @@
 
 using testing::_;
 using testing::AtMost;
 using testing::Exactly;
 using testing::InSequence;
 using testing::Mock;
 using testing::StrictMock;
 
 namespace {
 
+scoped_ptr<TestingProfileManager> CreateTestingProfileManager() {
+  scoped_ptr<TestingProfileManager> profile_manager(
+      new TestingProfileManager(TestingBrowserProcess::GetGlobal()));
+  EXPECT_TRUE(profile_manager->SetUp());
+  return profile_manager.Pass();
+}
+
 // Helper class that tracks state transitions in BackgroundModeManager and
 // exposes them via getters (or gmock for EnableLaunchOnStartup).
 class TestBackgroundModeManager : public StrictMock<BackgroundModeManager> {
  public:
   TestBackgroundModeManager(const base::CommandLine& command_line,
                             ProfileInfoCache* cache)
       : StrictMock<BackgroundModeManager>(command_line, cache),
         have_status_tray_(false),
         has_shown_balloon_(false) {
     ResumeBackgroundMode();
@@ skipping 82 matching lines @@
 
  private:
   bool enabled_;
   std::map<Profile*, int> profile_app_counts_;
 
   DISALLOW_COPY_AND_ASSIGN(AdvancedTestBackgroundModeManager);
 };
 
 class BackgroundModeManagerTest : public testing::Test {
  public:
-  BackgroundModeManagerTest() {}
+  BackgroundModeManagerTest()
+      : dummy_task_runner_(new base::TestSimpleTaskRunner()) {}
   ~BackgroundModeManagerTest() override {}
 
   void SetUp() override {
     command_line_.reset(new base::CommandLine(base::CommandLine::NO_PROGRAM));
     profile_manager_ = CreateTestingProfileManager();
     profile_ = profile_manager_->CreateTestingProfile("p1");
   }
 
  protected:
-  scoped_refptr<extensions::Extension> CreateExtension(
-      extensions::Manifest::Location location,
-      const std::string& data,
-      const std::string& id) {
-    scoped_ptr<base::DictionaryValue> parsed_manifest(
-        extensions::api_test_utils::ParseDictionary(data));
-    return extensions::api_test_utils::CreateExtension(
-        location, parsed_manifest.get(), id);
-  }
-
-  // From views::MenuModelAdapter::IsCommandEnabled with modification.
-  bool IsCommandEnabled(ui::MenuModel* model, int id) const {
-    int index = 0;
-    if (ui::MenuModel::GetModelAndIndexForCommandId(id, &model, &index))
-      return model->IsEnabledAt(index);
-
-    return false;
-  }
-
+  // We use a dummy task runner instead of the full test browser thread bundle
+  // to avoid having the browser's shutdown logic try to access a non-existent

[Andrew T Wilson (Slow), 2015/04/22 14:48:03]

I guess I'm not sure I understand - are you saying that there are shutdown tasks
that this dummy_task_runner_ won't execute? I'm not 100% clear why this class
gets a dummy_task_runner_ but BackgroundModeManagerWithExtensionTest does not?

[Sami, 2015/04/22 18:06:45]

Disclaimer: that was just the first thing I managed to make work -- I don't
actually know anything about these tests.

I've now done some more digging and the underlying problem is that
chrome::DecrementKeepAliveCount() uses the absence of a MessageLoop to infer
that it's in a unit test and shouldn't do a real browser shutdown. Since my
patch is adding a MessageLoop, things go wrong when the shutdown sequence is
trying to touch things that aren't there.

Unfortunately I can't seem to find the set of objects that actually need to be
around for the shutdown to work (MessageCenter is enough for everything except
CrOS which wants an ash::Shell). BackgroundModeManagerWithExtensionTest works
around this by artificially incrementing the browser keepalive count to prevent
the shutdown, but we can't really do that in BackgroundModeManagerTest because
it is explicitly checking what the current keepalive count is.

The best solution I could think of is to add a
chrome::DisableShutdownForTesting() that this test can use to keep its existing
behavior. WDYT?

+  // objects such as the MessageCenter during teardown.
+  base::ThreadTaskRunnerHandle dummy_task_runner_;
   scoped_ptr<base::CommandLine> command_line_;
 
   scoped_ptr<TestingProfileManager> profile_manager_;
   // Test profile used by all tests - this is owned by profile_manager_.
   TestingProfile* profile_;
 
  private:
-  scoped_ptr<TestingProfileManager> CreateTestingProfileManager() {
-    scoped_ptr<TestingProfileManager> profile_manager
-        (new TestingProfileManager(TestingBrowserProcess::GetGlobal()));
-    EXPECT_TRUE(profile_manager->SetUp());
-    return profile_manager.Pass();
-  }
-
   DISALLOW_COPY_AND_ASSIGN(BackgroundModeManagerTest);
 };
 
-class BackgroundModeManagerWithExtensionsTest
-    : public BackgroundModeManagerTest {
+class BackgroundModeManagerWithExtensionsTest : public testing::Test {
  public:
   BackgroundModeManagerWithExtensionsTest() {}
   ~BackgroundModeManagerWithExtensionsTest() override {}
 
   void SetUp() override {
-    BackgroundModeManagerTest::SetUp();
+    command_line_.reset(new base::CommandLine(base::CommandLine::NO_PROGRAM));
+    profile_manager_ = CreateTestingProfileManager();
+    profile_ = profile_manager_->CreateTestingProfile("p1");
 
     // Aura clears notifications from the message center at shutdown.
     message_center::MessageCenter::Initialize();
 
     // BackgroundModeManager actually affects Chrome start/stop state,
     // tearing down our thread bundle before we've had chance to clean
     // everything up. Keeping Chrome alive prevents this.
     // We aren't interested in if the keep alive works correctly in this test.
     chrome::IncrementKeepAliveCount();
 
@@ skipping 46 matching lines @@
     // Message Center shutdown must occur after the DecrementKeepAliveCount
     // because DecrementKeepAliveCount will end up referencing the message
     // center during cleanup.
     message_center::MessageCenter::Shutdown();
 
     // Clear the shutdown flag to isolate the remaining effect of this test.
     browser_shutdown::SetTryingToQuit(false);
   }
 
  protected:
+  scoped_refptr<extensions::Extension> CreateExtension(
+      extensions::Manifest::Location location,
+      const std::string& data,
+      const std::string& id) {
+    scoped_ptr<base::DictionaryValue> parsed_manifest(
+        extensions::api_test_utils::ParseDictionary(data));
+    return extensions::api_test_utils::CreateExtension(
+        location, parsed_manifest.get(), id);
+  }
+
+  // From views::MenuModelAdapter::IsCommandEnabled with modification.
+  bool IsCommandEnabled(ui::MenuModel* model, int id) const {
+    int index = 0;
+    if (ui::MenuModel::GetModelAndIndexForCommandId(id, &model, &index))
+      return model->IsEnabledAt(index);
+
+    return false;
+  }
+
   void AddEphemeralApp(const extensions::Extension* extension,
                        ExtensionService* service) {
     extensions::ExtensionPrefs* prefs =
         extensions::ExtensionPrefs::Get(service->profile());
     ASSERT_TRUE(prefs);
     prefs->OnExtensionInstalled(extension,
                                 extensions::Extension::ENABLED,
                                 syncer::StringOrdinal(),
                                 extensions::kInstallFlagIsEphemeral,
                                 std::string());
 
     service->AddExtension(extension);
   }
 
   scoped_ptr<TestBackgroundModeManager> manager_;
 
+  scoped_ptr<base::CommandLine> command_line_;
+
+  scoped_ptr<TestingProfileManager> profile_manager_;
+  // Test profile used by all tests - this is owned by profile_manager_.
+  TestingProfile* profile_;
+
  private:
   // Required for extension service.
   content::TestBrowserThreadBundle thread_bundle_;
 
 #if defined(OS_CHROMEOS)
   // ChromeOS needs extra services to run in the following order.
   chromeos::ScopedTestDeviceSettingsService test_device_settings_service_;
   chromeos::ScopedTestCrosSettings test_cros_settings_;
   chromeos::ScopedTestUserManager test_user_manager_;
 #endif
@@ skipping 613 matching lines @@
   }
   service->AddExtension(upgraded_bg_ext.get());
   Mock::VerifyAndClearExpectations(manager_.get());
   EXPECT_FALSE(manager_->HasShownBalloon());
 
   // Upgrading an extension that didn't have background to one that does should
   // show the balloon.
   service->AddExtension(upgraded_no_bg_ext_has_bg.get());
   EXPECT_TRUE(manager_->HasShownBalloon());
 }