sync: Refactor themes sync integration tests

chrome/browser/sync/test/integration/themes_helper.cc

Index: chrome/browser/sync/test/integration/themes_helper.cc
diff --git a/chrome/browser/sync/test/integration/themes_helper.cc b/chrome/browser/sync/test/integration/themes_helper.cc
index 8fa75bc659e772359235377858cc892a6e7fcc60..720f6cbaae7f4df5a71f650fd7f92999a9616f25 100644
--- a/chrome/browser/sync/test/integration/themes_helper.cc
+++ b/chrome/browser/sync/test/integration/themes_helper.cc
@@ -4,12 +4,19 @@
 
 #include "chrome/browser/sync/test/integration/themes_helper.h"
 
+#include "base/callback.h"
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/strings/stringprintf.h"
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/sync/test/integration/status_change_checker.h"
 #include "chrome/browser/sync/test/integration/sync_datatype_helper.h"
 #include "chrome/browser/sync/test/integration/sync_extension_helper.h"
 #include "chrome/browser/themes/theme_service.h"
 #include "chrome/browser/themes/theme_service_factory.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
+#include "content/public/browser/notification_source.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/id_util.h"
 #include "extensions/common/manifest.h"
@@ -56,10 +63,6 @@ bool ThemeIsPendingInstall(Profile* profile, const std::string& id) {
       IsExtensionPendingInstallForSync(profile, id);
 }
 
-bool HasOrWillHaveCustomTheme(Profile* profile, const std::string& id) {
-  return (GetThemeID(profile) == id) || ThemeIsPendingInstall(profile, id);
-}
-
 void UseCustomTheme(Profile* profile, int index) {
   SyncExtensionHelper::GetInstance()->InstallExtension(
       profile, MakeName(index), extensions::Manifest::TYPE_THEME);
@@ -73,4 +76,190 @@ void UseSystemTheme(Profile* profile) {
   GetThemeService(profile)->UseSystemTheme();
 }
 
+namespace {
+
+// Helper to wait until the specified theme is pending for install on the
+// specified profile.
+//
+// The themes sync integration tests don't actually install any custom themes,
+// but they do occasionally check that the ThemeService is attempts to install

[pval...(no longer on Chromium), 2014/07/15 20:34:47]

s/attempts/attempting/ ?

[rlarocque, 2014/07/15 20:47:01]

I removed the preceding "is".

+// synced themes.
+class ThemePendingInstallChecker : public StatusChangeChecker,
+                                   public content::NotificationObserver {
+ public:
+  ThemePendingInstallChecker(Profile* profile, const std::string& theme);
+  virtual ~ThemePendingInstallChecker();
+
+  // Implementation of StatusChangeChecker.
+  virtual std::string GetDebugMessage() const OVERRIDE;
+  virtual bool IsExitConditionSatisfied() OVERRIDE;
+
+  // Implementation of content::NotificationObserver.
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
+
+  // Waits until the condition to be met or a timeout occurs.
+  void Wait();
+
+ private:
+  Profile* profile_;
+  const std::string& theme_;
+
+  content::NotificationRegistrar registrar_;
+};
+
+ThemePendingInstallChecker::ThemePendingInstallChecker(Profile* profile,
+                                                       const std::string& theme)
+    : profile_(profile), theme_(theme) {
+}
+
+ThemePendingInstallChecker::~ThemePendingInstallChecker() {
+}
+
+std::string ThemePendingInstallChecker::GetDebugMessage() const {
+  return base::StringPrintf("Waiting for pending theme to be '%s'",
+                            theme_.c_str());
+}
+
+bool ThemePendingInstallChecker::IsExitConditionSatisfied() {
+  return ThemeIsPendingInstall(profile_, theme_);
+}
+
+void ThemePendingInstallChecker::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  DCHECK_EQ(chrome::NOTIFICATION_EXTENSION_UPDATING_STARTED, type);
+  CheckExitCondition();
+}
+
+void ThemePendingInstallChecker::Wait() {
+  // We'll check to see if the condition is met whenever the extension system
+  // tries to contact the web store.
+  registrar_.Add(this,
+                 chrome::NOTIFICATION_EXTENSION_UPDATING_STARTED,
+                 content::Source<Profile>(profile_));
+
+  if (IsExitConditionSatisfied()) {
+    return;
+  }
+
+  StartBlockingWait();
+}
+
+}  // namespace
+
+bool AwaitThemeIsPendingInstall(Profile* profile, const std::string& theme) {
+  ThemePendingInstallChecker checker(profile, theme);
+  checker.Wait();
+  return !checker.TimedOut();
+}
+
+namespace {
+
+// Helper to wait until a given condition is met, checking every time the
+// current theme changes.
+//
+// The |exit_condition_| closure may be invoked zero or more times.
+class ThemeConditionChecker : public StatusChangeChecker,
+                              public content::NotificationObserver {
+ public:
+  ThemeConditionChecker(Profile* profile,
+                        const std::string& debug_message_,
+                        base::Callback<bool(ThemeService*)> exit_condition);
+  virtual ~ThemeConditionChecker();
+
+  // Implementation of StatusChangeChecker.
+  virtual std::string GetDebugMessage() const OVERRIDE;
+  virtual bool IsExitConditionSatisfied() OVERRIDE;
+
+  // Implementation of content::NotificationObserver.
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
+
+  // Waits until the condition to be met or a timeout occurs.
+  void Wait();
+
+ private:
+  Profile* profile_;
+  const std::string debug_message_;
+  base::Callback<bool(ThemeService*)> exit_condition_;
+
+  content::NotificationRegistrar registrar_;
+};
+
+ThemeConditionChecker::ThemeConditionChecker(
+    Profile* profile,
+    const std::string& debug_message,
+    base::Callback<bool(ThemeService*)> exit_condition)
+    : profile_(profile),
+      debug_message_(debug_message),
+      exit_condition_(exit_condition) {
+}
+
+ThemeConditionChecker::~ThemeConditionChecker() {
+}
+
+std::string ThemeConditionChecker::GetDebugMessage() const {
+  return debug_message_;
+}
+
+bool ThemeConditionChecker::IsExitConditionSatisfied() {
+  return exit_condition_.Run(GetThemeService(profile_));
+}
+
+void ThemeConditionChecker::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  DCHECK_EQ(chrome::NOTIFICATION_BROWSER_THEME_CHANGED, type);
+  CheckExitCondition();
+}
+
+void ThemeConditionChecker::Wait() {
+  registrar_.Add(this,
+                 chrome::NOTIFICATION_BROWSER_THEME_CHANGED,
+                 content::Source<ThemeService>(GetThemeService(profile_)));
+
+  if (IsExitConditionSatisfied()) {
+    return;
+  }
+
+  StartBlockingWait();
+}
+
+// Helper function to let us bind this functionality into a base::Callback.
+bool UsingSystemThemeFunc(ThemeService* theme_service) {
+  return theme_service->UsingSystemTheme();
+}
+
+// Helper function to let us bind this functionality into a base::Callback.
+bool UsingDefaultThemeFunc(ThemeService* theme_service) {
+  return theme_service->UsingDefaultTheme();
+}
+
+}  // namespace
+
+bool AwaitUsingSystemTheme(Profile* profile) {
+  ThemeConditionChecker checker(
+      profile,
+      std::string("Waiting until profile is using system theme"),
+      base::Bind(&UsingSystemThemeFunc));
+  VLOG(1) << "Current theme ID is: " << GetThemeID(profile);

[pval...(no longer on Chromium), 2014/07/15 20:34:47]

should this be *D*VLOG?

[rlarocque, 2014/07/15 20:47:01]

Actually, I should just remove them.  I was using them to debug an issue in the
themes code itself.

+  checker.Wait();
+  return !checker.TimedOut();
+}
+
+bool AwaitUsingDefaultTheme(Profile* profile) {
+  ThemeConditionChecker checker(
+      profile,
+      std::string("Waiting until profile is using default theme"),
+      base::Bind(&UsingDefaultThemeFunc));
+  VLOG(1) << "Current theme ID is: " << GetThemeID(profile);
+  checker.Wait();
+  return !checker.TimedOut();
+}
+
 }  // namespace themes_helper