Move on-demand update checks from the FILE thread to the blocking pool.

chrome/browser/google/google_update_win_unittest.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 "chrome/browser/google/google_update_win.h"
 
 #include <windows.h>
 #include <atlbase.h>
 #include <atlcom.h>
 
+#include <map>
 #include <memory>
 #include <queue>
 
 #include "base/base_paths.h"
 #include "base/bind.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/path_service.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/scoped_path_override.h"
 #include "base/test/test_reg_util_win.h"
 #include "base/test/test_simple_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/version.h"
 #include "base/win/registry.h"
+#include "base/win/scoped_com_initializer.h"
 #include "base/win/scoped_comptr.h"
 #include "chrome/common/chrome_version.h"
 #include "chrome/installer/util/browser_distribution.h"
 #include "chrome/installer/util/google_update_settings.h"
 #include "chrome/installer/util/helper.h"
 #include "google_update/google_update_idl.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/win/atl_module.h"
 
 using ::testing::DoAll;
 using ::testing::HasSubstr;
 using ::testing::InSequence;
+using ::testing::InvokeWithoutArgs;
 using ::testing::IsEmpty;
 using ::testing::Return;
 using ::testing::Sequence;
 using ::testing::SetArgPointee;
 using ::testing::StrEq;
 using ::testing::StrictMock;
 using ::testing::Values;
 using ::testing::_;
 
 namespace {
@@ skipping 12 matching lines @@
   MOCK_METHOD3(OnError, void(GoogleUpdateErrorCode,
                              const base::string16&,
                              const base::string16&));
 
  private:
   base::WeakPtrFactory<UpdateCheckDelegate> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(MockUpdateCheckDelegate);
 };
 
+// A fake implementation of the COM IGlobalInterfaceTable that holds and hands
+// out object pointers.
+class FakeGit : public CComObjectRootEx<CComSingleThreadModel>,
+                public IGlobalInterfaceTable {
+ public:
+  BEGIN_COM_MAP(FakeGit)
+    COM_INTERFACE_ENTRY(IGlobalInterfaceTable)
+  END_COM_MAP()
+
+  FakeGit() = default;
+
+  HRESULT STDMETHODCALLTYPE RegisterInterfaceInGlobal(IUnknown* object,
+                                                      REFIID,
+                                                      DWORD* cookie) override {
+    objects_[++last_cookie_] = object;
+    *cookie = last_cookie_;
+    return S_OK;
+  }
+
+  HRESULT STDMETHODCALLTYPE RevokeInterfaceFromGlobal(DWORD cookie) override {
+    return objects_.erase(cookie) ? S_OK : E_INVALIDARG;
+  }
+
+  HRESULT STDMETHODCALLTYPE GetInterfaceFromGlobal(DWORD cookie,
+                                                   REFIID,
+                                                   void** object) override {
+    auto it = objects_.find(cookie);
+    if (it == objects_.end())
+      return E_INVALIDARG;
+    it->second.get()->AddRef();
+    *object = it->second.get();
+    return S_OK;
+  }
+
+  size_t size() const { return objects_.size(); }
+
+ private:
+  using CookieToObjectMap = std::map<DWORD, base::win::ScopedComPtr<IUnknown>>;
+
+  CookieToObjectMap objects_;
+  DWORD last_cookie_ = 0;
+};

[Peter Kasting, 2016/08/03 01:24:34]

Nit: DISALLOW_COPY_AND_ASSIGN?

[grt (UTC plus 2), 2016/08/03 05:36:31]

Done.

+
 // An interface that exposes a factory method for creating an IGoogleUpdate3Web
 // instance.
 class GoogleUpdateFactory {
  public:
   virtual ~GoogleUpdateFactory() {}
   virtual HRESULT Create(
       base::win::ScopedComPtr<IGoogleUpdate3Web>* google_update) = 0;
 };
 
 class MockCurrentState : public CComObjectRootEx<CComSingleThreadModel>,
@@ skipping 329 matching lines @@
   // in setting up expectations until a consumer obtains it via get_appWeb, at
   // which time it is owned by the consumer.
   CComObject<MockApp>* MakeApp(const base::char16* app_guid) {
     // The bundle will be called on to create the installed app.
     EXPECT_CALL(*this, createInstalledApp(StrEq(app_guid)))
         .WillOnce(Return(S_OK));
 
     CComObject<MockApp>* mock_app = nullptr;
     EXPECT_EQ(S_OK, CComObject<MockApp>::CreateInstance(&mock_app));
 
-    // Give mock_app_bundle a ref to the app which it will return when asked.
     // Note: to support multiple apps, get_appWeb expectations should use
     // successive indices.
-    mock_app->AddRef();
+    app_ = mock_app;
     EXPECT_CALL(*this, get_appWeb(0, _))
-        .WillOnce(DoAll(SetArgPointee<1>(mock_app),
-                        Return(S_OK)));
+        .WillRepeatedly(
+            DoAll(SetArgPointee<1>(mock_app),
+                  InvokeWithoutArgs(mock_app, &CComObject<MockApp>::AddRef)));
 
     return mock_app;
   }
 
  private:
+  base::win::ScopedComPtr<IAppWeb> app_;
+
   DISALLOW_COPY_AND_ASSIGN(MockAppBundle);
 };
 
 // A mock IGoogleUpdate3Web that can handle a call to initialize and
 // createAppBundleWeb by consumers.
 class MockGoogleUpdate : public CComObjectRootEx<CComSingleThreadModel>,
                          public IGoogleUpdate3Web {
  public:
   BEGIN_COM_MAP(MockGoogleUpdate)
     COM_INTERFACE_ENTRY(IGoogleUpdate3Web)
@@ skipping 27 matching lines @@
   // at which time it is owned by the consumer.
   CComObject<MockAppBundle>* MakeAppBundle() {
     CComObject<MockAppBundle>* mock_app_bundle = nullptr;
     EXPECT_EQ(S_OK,
               CComObject<MockAppBundle>::CreateInstance(&mock_app_bundle));
     EXPECT_CALL(*mock_app_bundle, initialize())
         .WillOnce(Return(S_OK));
     // Give this instance a ref to the bundle which it will return when created.
     mock_app_bundle->AddRef();
     EXPECT_CALL(*this, createAppBundleWeb(_))
+        .InSequence(sequence_)
         .WillOnce(DoAll(SetArgPointee<0>(mock_app_bundle), Return(S_OK)));
     return mock_app_bundle;
   }
 
  private:
+  Sequence sequence_;
+
   DISALLOW_COPY_AND_ASSIGN(MockGoogleUpdate);
 };
 
 // A mock factory for creating an IGoogleUpdate3Web instance.
 class MockGoogleUpdateFactory : public GoogleUpdateFactory {
  public:
   MockGoogleUpdateFactory() {}
   MOCK_METHOD1(Create, HRESULT(base::win::ScopedComPtr<IGoogleUpdate3Web>*));
 
   // Returns a mock IGoogleUpdate3Web object that will be returned by the
@@ skipping 29 matching lines @@
     // Configure all mock functions that return HRESULT to return failure.
     ::testing::DefaultValue<HRESULT>::Set(E_FAIL);
   }
 
   static void TearDownTestCase() { ::testing::DefaultValue<HRESULT>::Clear(); }
 
  protected:
   GoogleUpdateWinTest()
       : task_runner_(new base::TestSimpleTaskRunner()),
         task_runner_handle_(task_runner_),
-        system_level_install_(GetParam()) {}
+        system_level_install_(GetParam()),
+        fake_git_(nullptr) {}
 
   void SetUp() override {
     ::testing::TestWithParam<bool>::SetUp();
 
     // Override FILE_EXE so that it looks like the test is running from the
     // standard install location for this mode (system-level or user-level).
     base::FilePath file_exe;
     ASSERT_TRUE(PathService::Get(base::FILE_EXE, &file_exe));
     base::FilePath install_dir(installer::GetChromeInstallPath(
         system_level_install_, BrowserDistribution::GetDistribution()));
@@ skipping 30 matching lines @@
     ASSERT_EQ(ERROR_SUCCESS,
               key.Create(root, kClientState, KEY_WRITE | KEY_WOW64_32KEY));
     ASSERT_EQ(ERROR_SUCCESS,
               key.CreateKey(kChromeGuid, KEY_WRITE | KEY_WOW64_32KEY));
     ASSERT_EQ(ERROR_SUCCESS,
               key.WriteValue(L"UninstallArguments",
                              L"--uninstall --multi-install --chrome"));
 
     // Provide an IGoogleUpdate3Web class factory so that this test can provide
     // a mocked-out instance.
-    SetGoogleUpdateFactoryForTesting(
+    SetUpdateCheckFactoriesForTesting(
+        base::Bind(&GoogleUpdateWinTest::GetFakeGit, base::Unretained(this)),
         base::Bind(&GoogleUpdateFactory::Create,
                    base::Unretained(&mock_google_update_factory_)));
 
     // Compute a newer version.
     base::Version current_version(CHROME_VERSION_STRING);
     new_version_ = base::StringPrintf(L"%u.%u.%u.%u",
                                       current_version.components()[0],
                                       current_version.components()[1],
                                       current_version.components()[2] + 1,
                                       current_version.components()[3]);
   }
 
   // Creates app bundle and app mocks that will be used to simulate Google
   // Update.
   void MakeGoogleUpdateMocks(CComObject<MockAppBundle>** mock_app_bundle,
                              CComObject<MockApp>** mock_app) {
     CComObject<MockGoogleUpdate>* google_update =
         mock_google_update_factory_.MakeServerMock();
     CComObject<MockAppBundle>* app_bundle = google_update->MakeAppBundle();
     CComObject<MockApp>* app = app_bundle->MakeApp(kChromeBinariesGuid);
 
     if (mock_app_bundle)
       *mock_app_bundle = app_bundle;
     if (mock_app)
       *mock_app = app;
   }
 
   void TearDown() override {
+    // Be sure all objects were removed from the Global Interface Table.
+    if (fake_git_) {
+      ASSERT_EQ(0U, fake_git_->size());

[Peter Kasting, 2016/08/03 01:24:34]

Nit: You could also elect to provide .empty() in place of .size(), which would
make this assert slightly shorter, at the cost of omitting the precise count
from the logs in cases where the assert fails.  (I'd probably
ASSERT_TRUE(fake_git_->empty()) if it were an STL class.)

[grt (UTC plus 2), 2016/08/03 05:36:31]

Done.

+      git_.Release();
+      fake_git_ = nullptr;
+    }
+
     // Remove the test's IGoogleUpdate on-demand update class factory.
-    SetGoogleUpdateFactoryForTesting(GoogleUpdate3ClassFactory());
+    SetUpdateCheckFactoriesForTesting(GlobalInterfaceTableClassFactory(),
+                                      GoogleUpdate3ClassFactory());
     ::testing::TestWithParam<bool>::TearDown();
   }
 
+  HRESULT GetFakeGit(base::win::ScopedComPtr<IGlobalInterfaceTable>* git) {

[Peter Kasting, 2016/08/03 01:24:34]

Seems like this should return void.

[grt (UTC plus 2), 2016/08/03 05:36:31]

This is bound into a GlobalInterfaceTableClassFactory callback, which returns
HRESULT.

+    if (!fake_git_) {
+      EXPECT_EQ(S_OK, CComObject<FakeGit>::CreateInstance(&fake_git_));
+      EXPECT_NE(nullptr, fake_git_);
+      git_ = fake_git_;
+    }
+    *git = git_;
+    return S_OK;
+  }
+
   static const base::char16 kClients[];
   static const base::char16 kClientState[];
   static const base::char16 kChromeGuid[];
   static const base::char16 kChromeBinariesGuid[];
 
+  base::win::ScopedCOMInitializer com_initializer_;
   scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
   base::ThreadTaskRunnerHandle task_runner_handle_;
   bool system_level_install_;
   std::unique_ptr<base::ScopedPathOverride> file_exe_override_;
   std::unique_ptr<base::ScopedPathOverride> program_files_override_;
   std::unique_ptr<base::ScopedPathOverride> program_files_x86_override_;
   std::unique_ptr<base::ScopedPathOverride> local_app_data_override_;
   registry_util::RegistryOverrideManager registry_override_manager_;
 
+  CComObject<FakeGit>* fake_git_;
+  base::win::ScopedComPtr<IGlobalInterfaceTable> git_;
+
   // A mock object, the OnUpdateCheckCallback method of which will be invoked
   // each time the update check machinery invokes the given UpdateCheckCallback.
   StrictMock<MockUpdateCheckDelegate> mock_update_check_delegate_;
 
   // A mock object that provides a GoogleUpdate3ClassFactory by which the test
   // fixture's IGoogleUpdate3Web simulator is provided to the update check
   // machinery.
   StrictMock<MockGoogleUpdateFactory> mock_google_update_factory_;
 
   // The new version that the fixture will pretend is available.
@@ skipping 264 matching lines @@
   }
   BeginUpdateCheck(task_runner_, std::string(), true, 0,
                    mock_update_check_delegate_.AsWeakPtr());
   task_runner_->RunUntilIdle();
 }
 
 // Test that a retry after a USING_EXTERNAL_UPDATER failure succeeds.
 TEST_P(GoogleUpdateWinTest, RetryAfterExternalUpdaterError) {
   static const HRESULT GOOPDATE_E_APP_USING_EXTERNAL_UPDATER = 0xa043081d;
 
-  CComObject<MockAppBundle>* mock_app_bundle =
-      mock_google_update_factory_.MakeServerMock()->MakeAppBundle();
+  CComObject<MockGoogleUpdate>* google_update =
+      mock_google_update_factory_.MakeServerMock();
+  CComObject<MockAppBundle>* mock_app_bundle = google_update->MakeAppBundle();
 
   // The first attempt will fail in createInstalledApp indicating that an update
   // is already in progress.
-  Sequence bundle_seq;
   EXPECT_CALL(*mock_app_bundle, createInstalledApp(StrEq(kChromeBinariesGuid)))
-      .InSequence(bundle_seq)
       .WillOnce(Return(GOOPDATE_E_APP_USING_EXTERNAL_UPDATER));
 
-  // Expect a retry on the same instance.
-  EXPECT_CALL(*mock_app_bundle, createInstalledApp(StrEq(kChromeBinariesGuid)))
-      .InSequence(bundle_seq)
-      .WillOnce(Return(S_OK));
-
-  // See MakeApp() for an explanation of this:
-  CComObject<MockApp>* mock_app = nullptr;
-  EXPECT_EQ(S_OK, CComObject<MockApp>::CreateInstance(&mock_app));
-  mock_app->AddRef();
-  EXPECT_CALL(*mock_app_bundle, get_appWeb(0, _))
-      .WillOnce(DoAll(SetArgPointee<1>(mock_app), Return(S_OK)));
+  // Expect a retry on a new bundle.
+  mock_app_bundle = google_update->MakeAppBundle();
+  CComObject<MockApp>* mock_app = mock_app_bundle->MakeApp(kChromeBinariesGuid);
 
   // Expect the bundle to be called on to start the update.
   EXPECT_CALL(*mock_app_bundle, checkForUpdate()).WillOnce(Return(S_OK));
 
   mock_app->PushState(STATE_INIT);
   mock_app->PushState(STATE_CHECKING_FOR_UPDATE);
   mock_app->PushState(STATE_NO_UPDATE);
 
   // Expect the update check to succeed.
   EXPECT_CALL(mock_update_check_delegate_,
@@ skipping 61 matching lines @@
   BeginUpdateCheck(task_runner_, std::string(), true, 0,
                    mock_update_check_delegate_.AsWeakPtr());
   BeginUpdateCheck(task_runner_, std::string(), true, 0,
                    mock_update_check_delegate_2.AsWeakPtr());
   task_runner_->RunUntilIdle();
 }
 
 INSTANTIATE_TEST_CASE_P(UserLevel, GoogleUpdateWinTest, Values(false));
 
 INSTANTIATE_TEST_CASE_P(SystemLevel, GoogleUpdateWinTest, Values(true));