Settings reset prompt: Add a unit test for the default settings fetcher.
The test ensures that DefaultSettingsFetcher never returns null even when fetching default settings fails.
BUG=716372
Review-Url: https://codereview.chromium.org/2850103002
Cr-Commit-Position: refs/heads/master@{#468424}
diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn
index 8d66725c..9cfc638 100644
--- a/chrome/browser/BUILD.gn
+++ b/chrome/browser/BUILD.gn
@@ -1787,6 +1787,8 @@
"renderer_host/chrome_extension_message_filter.h",
"safe_browsing/chrome_cleaner_dialog_controller.cc",
"safe_browsing/chrome_cleaner_dialog_controller.h",
+ "safe_browsing/settings_reset_prompt/default_settings_fetcher.cc",
+ "safe_browsing/settings_reset_prompt/default_settings_fetcher.h",
"safe_browsing/settings_reset_prompt/extension_info.cc",
"safe_browsing/settings_reset_prompt/extension_info.h",
"safe_browsing/settings_reset_prompt/settings_reset_prompt_config.cc",
diff --git a/chrome/browser/safe_browsing/settings_reset_prompt/default_settings_fetcher.cc b/chrome/browser/safe_browsing/settings_reset_prompt/default_settings_fetcher.cc
new file mode 100644
index 0000000..ea8490e
--- /dev/null
+++ b/chrome/browser/safe_browsing/settings_reset_prompt/default_settings_fetcher.cc
@@ -0,0 +1,95 @@
+// Copyright 2017 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/safe_browsing/settings_reset_prompt/default_settings_fetcher.h"
+
+#include <string>
+#include <utility>
+
+#include "base/bind_helpers.h"
+#include "base/logging.h"
+#include "base/memory/ptr_util.h"
+#include "chrome/browser/google/google_brand.h"
+#include "chrome/browser/profile_resetter/brandcode_config_fetcher.h"
+#include "chrome/browser/profile_resetter/brandcoded_default_settings.h"
+#include "content/public/browser/browser_thread.h"
+#include "url/gurl.h"
+
+namespace safe_browsing {
+
+namespace {
+
+#if defined(GOOGLE_CHROME_BUILD)
+constexpr char kOmahaUrl[] = "https://tools.google.com/service/update2";
+#endif // defined(GOOGLE_CHROME_BUILD)
+
+} // namespace
+
+// static
+void DefaultSettingsFetcher::FetchDefaultSettings(SettingsCallback callback) {
+ DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+ // |settings_fetcher| will delete itself when default settings have been
+ // fetched after the call to |Start()|.
+ DefaultSettingsFetcher* settings_fetcher =
+ new DefaultSettingsFetcher(std::move(callback));
+ settings_fetcher->Start();
+}
+
+// static
+void DefaultSettingsFetcher::FetchDefaultSettingsForTesting(
+ SettingsCallback callback,
+ std::unique_ptr<BrandcodedDefaultSettings> fetched_settings) {
+ DefaultSettingsFetcher* settings_fetcher =
+ new DefaultSettingsFetcher(std::move(callback));
+ // Inject the given settings directly by passing them to
+ // |PostCallbackAndDeleteSelf()|.
+ settings_fetcher->PostCallbackAndDeleteSelf(std::move(fetched_settings));
+}
+
+DefaultSettingsFetcher::DefaultSettingsFetcher(SettingsCallback callback)
+ : callback_(std::move(callback)) {}
+
+DefaultSettingsFetcher::~DefaultSettingsFetcher() {}
+
+void DefaultSettingsFetcher::Start() {
+ DCHECK(!config_fetcher_);
+
+#if defined(GOOGLE_CHROME_BUILD)
+ std::string brandcode;
+ if (google_brand::GetBrand(&brandcode) && !brandcode.empty()) {
+ config_fetcher_.reset(new BrandcodeConfigFetcher(
+ base::Bind(&DefaultSettingsFetcher::OnSettingsFetched,
+ base::Unretained(this)),
+ GURL(kOmahaUrl), brandcode));
+ return;
+ }
+#endif // defined(GOOGLE_CHROME_BUILD)
+
+ // For non Google Chrome builds and cases with an empty |brandcode|, we create
+ // a default-constructed |BrandcodedDefaultSettings| object and post the
+ // callback immediately.
+ PostCallbackAndDeleteSelf(base::MakeUnique<BrandcodedDefaultSettings>());
+}
+
+void DefaultSettingsFetcher::OnSettingsFetched() {
+ DCHECK(config_fetcher_);
+ DCHECK(!config_fetcher_->IsActive());
+
+ PostCallbackAndDeleteSelf(config_fetcher_->GetSettings());
+}
+
+void DefaultSettingsFetcher::PostCallbackAndDeleteSelf(
+ std::unique_ptr<BrandcodedDefaultSettings> default_settings) {
+ // Use default settings if fetching of BrandcodedDefaultSettings failed.
+ if (!default_settings)
+ default_settings.reset(new BrandcodedDefaultSettings());
+
+ content::BrowserThread::PostTask(
+ content::BrowserThread::UI, FROM_HERE,
+ base::BindOnce(std::move(callback_), base::Passed(&default_settings)));
+ delete this;
+}
+
+} // namespace safe_browsing
diff --git a/chrome/browser/safe_browsing/settings_reset_prompt/default_settings_fetcher.h b/chrome/browser/safe_browsing/settings_reset_prompt/default_settings_fetcher.h
new file mode 100644
index 0000000..ffd5e9eaa
--- /dev/null
+++ b/chrome/browser/safe_browsing/settings_reset_prompt/default_settings_fetcher.h
@@ -0,0 +1,64 @@
+// Copyright 2017 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 CHROME_BROWSER_SAFE_BROWSING_SETTINGS_RESET_PROMPT_DEFAULT_SETTINGS_FETCHER_H_
+#define CHROME_BROWSER_SAFE_BROWSING_SETTINGS_RESET_PROMPT_DEFAULT_SETTINGS_FETCHER_H_
+
+#include <memory>
+
+#include "base/callback.h"
+
+class BrandcodedDefaultSettings;
+class BrandcodeConfigFetcher;
+
+namespace safe_browsing {
+
+// Class that fetches default settings to be used for the settings reset
+// prompt. The static |FetchDefaultSettings()| function will create and manage
+// the lifetime of |DefaultSettingsFetcher| instances.
+class DefaultSettingsFetcher {
+ public:
+ using SettingsCallback =
+ base::Callback<void(std::unique_ptr<BrandcodedDefaultSettings>)>;
+
+ // Fetches default settings and passes the corresponding
+ // |BrandcodedDefaultSettings| object to |callback| on the UI thread. This
+ // function must be called on the UI thread as well.
+ static void FetchDefaultSettings(SettingsCallback callback);
+ // Allows tests to specify the default settings that were fetched in
+ // |fetched_settings|. Passing nullptr as |fetched_settings| corresponds to
+ // the case when fetching default settings over the network fails.
+ static void FetchDefaultSettingsForTesting(
+ SettingsCallback callback,
+ std::unique_ptr<BrandcodedDefaultSettings> fetched_settings);
+
+ private:
+ // Instances of |DefaultSettingsFetcher| own themselves and will delete
+ // themselves once default settings have been fetched and |callback| has been
+ // posted on the UI thread.
+ //
+ // The main reason for this design is that |BrandcodeConfigFetcher| takes a
+ // callback and initiates the fetching process in its constructor, and we need
+ // to hold on to the instance of the fetcher until settings have been
+ // fetched. This design saves us from having to explicitly manage global
+ // |BrandcodeConfigFetcher| instances.
+ explicit DefaultSettingsFetcher(SettingsCallback callback);
+ ~DefaultSettingsFetcher();
+
+ // Starts the process of fetching default settings and will ensure that
+ // |PostCallbackAndDeleteSelf| is called once settings have been fetched.
+ void Start();
+ void OnSettingsFetched();
+ // Posts a call to |callback_| on the UI thread, passing to it
+ // |default_settings|, and deletes |this|.
+ void PostCallbackAndDeleteSelf(
+ std::unique_ptr<BrandcodedDefaultSettings> default_settings);
+
+ std::unique_ptr<BrandcodeConfigFetcher> config_fetcher_;
+ SettingsCallback callback_;
+};
+
+} // namespace safe_browsing
+
+#endif // CHROME_BROWSER_SAFE_BROWSING_SETTINGS_RESET_PROMPT_DEFAULT_SETTINGS_FETCHER_H_
diff --git a/chrome/browser/safe_browsing/settings_reset_prompt/default_settings_fetcher_browsertest.cc b/chrome/browser/safe_browsing/settings_reset_prompt/default_settings_fetcher_browsertest.cc
new file mode 100644
index 0000000..4b39921
--- /dev/null
+++ b/chrome/browser/safe_browsing/settings_reset_prompt/default_settings_fetcher_browsertest.cc
@@ -0,0 +1,69 @@
+// Copyright 2017 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/safe_browsing/settings_reset_prompt/default_settings_fetcher.h"
+
+#include <memory>
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/callback.h"
+#include "base/memory/ptr_util.h"
+#include "base/run_loop.h"
+#include "chrome/browser/profile_resetter/brandcoded_default_settings.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace safe_browsing {
+namespace {
+
+class DefaultSettingsFetcherTest : public InProcessBrowserTest {
+ public:
+ void FetchedSettings(std::unique_ptr<BrandcodedDefaultSettings> settings) {
+ EXPECT_FALSE(settings_);
+ EXPECT_FALSE(fetched_settings_called);
+
+ fetched_settings_called = true;
+ settings_ = std::move(settings);
+ }
+
+ protected:
+ bool fetched_settings_called = false;
+ std::unique_ptr<BrandcodedDefaultSettings> settings_;
+};
+
+IN_PROC_BROWSER_TEST_F(DefaultSettingsFetcherTest, FetchingSettingsSucceeded) {
+ // The default settings that we will pretend were fetched. Keep the raw
+ // pointer here so that we can compare it to what was passed to the callback.
+ BrandcodedDefaultSettings* default_settings = new BrandcodedDefaultSettings();
+
+ DefaultSettingsFetcher::FetchDefaultSettingsForTesting(
+ base::Bind(&DefaultSettingsFetcherTest::FetchedSettings,
+ base::Unretained(this)),
+ base::WrapUnique(default_settings));
+
+ base::RunLoop().RunUntilIdle();
+
+ EXPECT_TRUE(fetched_settings_called);
+ EXPECT_EQ(default_settings, settings_.get());
+}
+
+IN_PROC_BROWSER_TEST_F(DefaultSettingsFetcherTest, FetchingSettingsFailed) {
+ // Pretend that fetching default settings failed by passing in a nullptr to
+ // |FetchDefaultSettingsForTesting()|. The callback should still receive
+ // default-constructed default settings.
+ DefaultSettingsFetcher::FetchDefaultSettingsForTesting(
+ base::Bind(&DefaultSettingsFetcherTest::FetchedSettings,
+ base::Unretained(this)),
+ nullptr);
+
+ base::RunLoop().RunUntilIdle();
+
+ EXPECT_TRUE(fetched_settings_called);
+ EXPECT_TRUE(settings_);
+}
+
+} // namespace
+} // namespace safe_browsing
diff --git a/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model.cc b/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model.cc
index f93e81e6..3a8daef 100644
--- a/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model.cc
+++ b/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model.cc
@@ -10,12 +10,11 @@
#include "base/callback.h"
#include "base/metrics/histogram_macros.h"
#include "chrome/browser/extensions/extension_service.h"
-#include "chrome/browser/google/google_brand.h"
#include "chrome/browser/prefs/session_startup_pref.h"
-#include "chrome/browser/profile_resetter/brandcode_config_fetcher.h"
#include "chrome/browser/profile_resetter/brandcoded_default_settings.h"
#include "chrome/browser/profile_resetter/resettable_settings_snapshot.h"
#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/safe_browsing/settings_reset_prompt/default_settings_fetcher.h"
#include "chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config.h"
#include "chrome/browser/search_engines/template_url_service_factory.h"
#include "chrome/common/extensions/manifest_handlers/settings_overrides_handler.h"
@@ -34,10 +33,6 @@
namespace {
-#if defined(GOOGLE_CHROME_BUILD)
-constexpr char kOmahaUrl[] = "https://tools.google.com/service/update2";
-#endif // defined(GOOGLE_CHROME_BUILD)
-
// These values are used for UMA metrics reporting. New enum values can be
// added, but existing enums must never be renumbered or deleted and reused.
enum SettingsReset {
@@ -57,103 +52,6 @@
SETTINGS_TYPE_STARTUP_URLS,
};
-// A helper class that fetches default settings to be used for the settings
-// reset prompt. The static |FetchDefaultSettings()| function will create and
-// manage the lifetime of |DefaultSettingsFetcher| instances.
-class DefaultSettingsFetcher {
- public:
- using SettingsCallback =
- base::Callback<void(std::unique_ptr<BrandcodedDefaultSettings>)>;
-
- // Fetches default settings and passes the corresponding
- // |BrandcodedDefaultSettings| object to |callback| on the UI thread. The
- // function should be called on the UI thread as well.
- static void FetchDefaultSettings(SettingsCallback callback);
-
- private:
- // Instances of |DefaultSettingsFetcher| own themselves and will delete
- // themselves once default settings have been fetched and |callback| has been
- // posted on the UI thread.
- //
- // The main reason for this design is that |BrandcodeConfigFetcher| takes a
- // callback and initiates the fetching process in its constructor, and we need
- // to hold on to the instance of the fetcher until settings have been
- // fetched. This design saves us from having to explicitly manage global
- // |BrandcodeConfigFetcher| instances.
- explicit DefaultSettingsFetcher(SettingsCallback callback);
- ~DefaultSettingsFetcher();
-
- // Starts the process of fetching default settings and will ensure that
- // |PostCallbackAndDeleteSelf| is called once settings have been fetched.
- void Start();
- void OnSettingsFetched();
- // Posts a call to |callback_| on the UI thread, passing to it
- // |default_settings|, and deletes |this|.
- void PostCallbackAndDeleteSelf(
- std::unique_ptr<BrandcodedDefaultSettings> default_settings);
-
- std::unique_ptr<BrandcodeConfigFetcher> config_fetcher_;
- SettingsCallback callback_;
-};
-
-// static
-void DefaultSettingsFetcher::FetchDefaultSettings(SettingsCallback callback) {
- DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-
- // |settings_fetcher| will delete itself when default settings have been
- // fetched after the call to |Start()|.
- DefaultSettingsFetcher* settings_fetcher =
- new DefaultSettingsFetcher(std::move(callback));
- settings_fetcher->Start();
-}
-
-DefaultSettingsFetcher::DefaultSettingsFetcher(SettingsCallback callback)
- : callback_(std::move(callback)) {}
-
-DefaultSettingsFetcher::~DefaultSettingsFetcher() {}
-
-void DefaultSettingsFetcher::Start() {
- DCHECK(!config_fetcher_);
-
-#if defined(GOOGLE_CHROME_BUILD)
- std::string brandcode;
- if (google_brand::GetBrand(&brandcode) && !brandcode.empty()) {
- config_fetcher_.reset(new BrandcodeConfigFetcher(
- base::Bind(&DefaultSettingsFetcher::OnSettingsFetched,
- base::Unretained(this)),
- GURL(kOmahaUrl), brandcode));
- return;
- }
-#endif // defined(GOOGLE_CHROME_BUILD)
-
- // For non Google Chrome builds and cases with an empty |brandcode|, we create
- // a default-constructed |BrandcodedDefaultSettings| object and post the
- // callback immediately.
- PostCallbackAndDeleteSelf(base::MakeUnique<BrandcodedDefaultSettings>());
-}
-
-void DefaultSettingsFetcher::OnSettingsFetched() {
- DCHECK(config_fetcher_);
- DCHECK(!config_fetcher_->IsActive());
-
- std::unique_ptr<BrandcodedDefaultSettings> settings(
- config_fetcher_->GetSettings());
- // Use default settings if fetching of BrandcodedDefaultSettings fails.
- if (!settings)
- settings.reset(new BrandcodedDefaultSettings());
-
- PostCallbackAndDeleteSelf(std::move(settings));
-}
-
-void DefaultSettingsFetcher::PostCallbackAndDeleteSelf(
- std::unique_ptr<BrandcodedDefaultSettings> default_settings) {
- DCHECK(default_settings);
- content::BrowserThread::PostTask(
- content::BrowserThread::UI, FROM_HERE,
- base::BindOnce(std::move(callback_), base::Passed(&default_settings)));
- delete this;
-}
-
const extensions::Extension* GetExtension(
Profile* profile,
const extensions::ExtensionId& extension_id) {
