Pick more adequate time for showing locale change notification.

(without this fix users complain that notification is shown too early).

BUG=chromium-os:11839
TEST=Manual

Review URL: http://codereview.chromium.org/6591067

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@76740 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 69 insertions, 59 deletions

diff --git a/chrome/browser/chromeos/locale_change_guard.cc b/chrome/browser/chromeos/locale_change_guard.cc
index d65c6cf..abe52906 100644
--- a/chrome/browser/chromeos/locale_change_guard.cc
+++ b/chrome/browser/chromeos/locale_change_guard.cc
@@ -7,24 +7,19 @@
 #include "base/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/browser_process.h"
-#include "chrome/browser/chromeos/notifications/system_notification.h"
 #include "chrome/browser/metrics/user_metrics.h"
 #include "chrome/browser/notifications/notification_delegate.h"
 #include "chrome/browser/prefs/pref_service.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/common/notification_service.h"
+#include "chrome/common/notification_source.h"
 #include "chrome/common/pref_names.h"
 #include "content/browser/tab_contents/tab_contents.h"
 #include "grit/app_resources.h"
 #include "grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 
-namespace {
-
-base::LazyInstance<chromeos::LocaleChangeGuard> g_locale_change_guard(
-    base::LINKER_INITIALIZED);
-
-}  // namespace
-
 namespace chromeos {
 
 class LocaleChangeGuard::Delegate : public NotificationDelegate {
@@ -42,16 +37,18 @@ class LocaleChangeGuard::Delegate : public NotificationDelegate {
   DISALLOW_COPY_AND_ASSIGN(Delegate);
 };
 
-LocaleChangeGuard::LocaleChangeGuard()
-    : profile_id_(Profile::kInvalidProfileId),
-      tab_contents_(NULL),
+LocaleChangeGuard::LocaleChangeGuard(Profile* profile)
+    : profile_(profile),
       note_(NULL),
       reverted_(false) {
+  DCHECK(profile_);
+  registrar_.Add(this, NotificationType::LOAD_COMPLETED_MAIN_FRAME,
+                 NotificationService::AllSources());
 }
 
 void LocaleChangeGuard::RevertLocaleChange(const ListValue* list) {
   if (note_ == NULL ||
-      tab_contents_ == NULL ||
+      profile_ == NULL ||
       from_locale_.empty() ||
       to_locale_.empty()) {
     NOTREACHED();
@@ -60,35 +57,40 @@ void LocaleChangeGuard::RevertLocaleChange(const ListValue* list) {
   if (reverted_)
     return;
 
-  PrefService* prefs = tab_contents_->profile()->GetPrefs();
+  PrefService* prefs = profile_->GetPrefs();
   if (prefs == NULL)
     return;
 
   reverted_ = true;
   UserMetrics::RecordAction(UserMetricsAction("LanguageChange_Revert"));
-  tab_contents_->profile()->ChangeAppLocale(
+  profile_->ChangeAppLocale(
       from_locale_, Profile::APP_LOCALE_CHANGED_VIA_REVERT);
 
-  Browser* browser = Browser::GetBrowserForController(
-      &tab_contents_->controller(), NULL);
+  Browser* browser = Browser::GetTabbedBrowser(profile_, false);
   if (browser)
     browser->ExecuteCommand(IDC_EXIT);
 }
 
-void LocaleChangeGuard::CheckLocaleChange(TabContents* tab_contents) {
-  // We want notification to be shown no more than once per session.
-  if (note_ != NULL)
+void LocaleChangeGuard::Observe(NotificationType type,
+                                const NotificationSource& source,
+                                const NotificationDetails& details) {
+  if (type != NotificationType::LOAD_COMPLETED_MAIN_FRAME) {
+    NOTREACHED();
+    return;
+  }
+  if (profile_ == NULL) {
+    NOTREACHED();
     return;
-  DCHECK(note_ == NULL && tab_contents_ == NULL);
-  DCHECK(from_locale_.empty() && to_locale_.empty());
+  }
+
+  // We need to perform locale change check only once: so we want to
+  // unsubscribe from notifications in any case.
+  registrar_.RemoveAll();
 
-  // We check profile Id because:
-  // (1) we want to exit fast in common case when nothing should be done.
-  // (2) on ChromeOS this guard may be invoked for a dummy profile first time.
-  ProfileId cur_profile_id = tab_contents->profile()->GetRuntimeId();
-  if (cur_profile_id == profile_id_)
-    return;  // We have already checked this profile, exiting fast.
-  profile_id_ = cur_profile_id;
+  if (note_ != NULL || !from_locale_.empty() || !to_locale_.empty()) {
+    // Somehow we are notified more than once. Once is enough.
+    return;
+  }
 
   std::string cur_locale = g_browser_process->GetApplicationLocale();
   if (cur_locale.empty()) {
@@ -96,7 +98,7 @@ void LocaleChangeGuard::CheckLocaleChange(TabContents* tab_contents) {
     return;
   }
 
-  PrefService* prefs = tab_contents->profile()->GetPrefs();
+  PrefService* prefs = profile_->GetPrefs();
   if (prefs == NULL)
     return;
 
@@ -118,9 +120,8 @@ void LocaleChangeGuard::CheckLocaleChange(TabContents* tab_contents) {
   // Locale change detected, showing notification.
   from_locale_ = from_locale;
   to_locale_ = to_locale;
-  tab_contents_ = tab_contents;
   note_.reset(new chromeos::SystemNotification(
-      tab_contents->profile(),
+      profile_,
       new Delegate(this),
       IDR_DEFAULT_FAVICON,
       l10n_util::GetStringUTF16(
@@ -138,7 +139,7 @@ void LocaleChangeGuard::CheckLocaleChange(TabContents* tab_contents) {
 
 void LocaleChangeGuard::AcceptLocaleChange() {
   if (note_ == NULL ||
-      tab_contents_ == NULL ||
+      profile_ == NULL ||
       from_locale_.empty() ||
       to_locale_.empty()) {
     NOTREACHED();
@@ -149,7 +150,7 @@ void LocaleChangeGuard::AcceptLocaleChange() {
   // If not: mark current locale as accepted.
   if (reverted_)
     return;
-  PrefService* prefs = tab_contents_->profile()->GetPrefs();
+  PrefService* prefs = profile_->GetPrefs();
   if (prefs == NULL)
     return;
   if (prefs->GetString(prefs::kApplicationLocale) != to_locale_)
@@ -160,11 +161,6 @@ void LocaleChangeGuard::AcceptLocaleChange() {
   prefs->ScheduleSavePersistentPrefs();
 }
 
-// static
-void LocaleChangeGuard::Check(TabContents* tab_contents) {
-  g_locale_change_guard.Get().CheckLocaleChange(tab_contents);
-}
-
 void LocaleChangeGuard::Delegate::Close(bool by_user) {
   if (by_user)
     master_->AcceptLocaleChange();
diff --git a/chrome/browser/chromeos/locale_change_guard.h b/chrome/browser/chromeos/locale_change_guard.h
index 0391761..efddc99 100644
--- a/chrome/browser/chromeos/locale_change_guard.h
+++ b/chrome/browser/chromeos/locale_change_guard.h
@@ -6,41 +6,46 @@
 #define CHROME_BROWSER_CHROMEOS_LOCALE_CHANGE_GUARD_H_
 #pragma once
 
+#include <string>
+
 #include "base/lazy_instance.h"
 #include "base/scoped_ptr.h"
-#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/chromeos/notifications/system_notification.h"
+#include "chrome/common/notification_registrar.h"
+#include "chrome/common/notification_observer.h"
+#include "chrome/common/notification_type.h"
 
 class ListValue;
-class TabContents;
+class NotificationDetails;
+class NotificationSource;
+class Profile;
 
 namespace chromeos {
 
-class SystemNotification;
-
-class LocaleChangeGuard {
+// Performs check whether locale has been changed automatically recently
+// (based on synchronized user preference).  If so: shows notification that
+// allows user to revert change.
+class LocaleChangeGuard : public NotificationObserver {
  public:
-  // When called first time for user profile: performs check whether
-  // locale has been changed automatically recently (based on synchronized user
-  // preference).  If so: shows notification that allows user to revert change.
-  // On subsequent calls: does nothing (hopefully fast).
-  static void Check(TabContents* tab_contents);
+  explicit LocaleChangeGuard(Profile* profile);
 
  private:
   class Delegate;
 
-  LocaleChangeGuard();
-  void CheckLocaleChange(TabContents* tab_contents);
   void RevertLocaleChange(const ListValue* list);
   void AcceptLocaleChange();
 
+  // NotificationObserver implementation.
+  virtual void Observe(NotificationType type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details);
+
   std::string from_locale_;
   std::string to_locale_;
-  ProfileId profile_id_;
-  TabContents* tab_contents_;
+  Profile* profile_;
   scoped_ptr<chromeos::SystemNotification> note_;
   bool reverted_;
-
-  friend struct base::DefaultLazyInstanceTraits<LocaleChangeGuard>;
+  NotificationRegistrar registrar_;
 };
 
 }  // namespace chromeos
diff --git a/chrome/browser/chromeos/login/login_utils.cc b/chrome/browser/chromeos/login/login_utils.cc
index 8694f8a..e10f01b 100644
--- a/chrome/browser/chromeos/login/login_utils.cc
+++ b/chrome/browser/chromeos/login/login_utils.cc
@@ -334,6 +334,7 @@ void LoginUtilsImpl::CompleteLogin(
     pref_service->SetBoolean(prefs::kEnableScreenLock, true);
   }
 
+  profile->OnLogin();
   DoBrowserLaunch(profile);
 }
 
diff --git a/chrome/browser/chromeos/notifications/system_notification.h b/chrome/browser/chromeos/notifications/system_notification.h
index 5573f2b..12fa718 100644
--- a/chrome/browser/chromeos/notifications/system_notification.h
+++ b/chrome/browser/chromeos/notifications/system_notification.h
@@ -9,7 +9,6 @@
 #include <string>
 
 #include "base/basictypes.h"
-#include "base/ref_counted.h"
 #include "base/string16.h"
 #include "chrome/browser/chromeos/notifications/balloon_collection_impl.h"
 #include "chrome/browser/notifications/notification_delegate.h"
diff --git a/chrome/browser/profiles/profile.h b/chrome/browser/profiles/profile.h
index 8c80164..bf3807a 100644
--- a/chrome/browser/profiles/profile.h
+++ b/chrome/browser/profiles/profile.h
@@ -528,6 +528,9 @@ class Profile {
   virtual void ChangeAppLocale(
       const std::string& locale, AppLocaleChangedVia via) = 0;
 
+  // Called after login.
+  virtual void OnLogin() {}
+
   // Returns ChromeOS's ProxyConfigServiceImpl, creating if not yet created.
   virtual chromeos::ProxyConfigServiceImpl*
       GetChromeOSProxyConfigServiceImpl() = 0;
diff --git a/chrome/browser/profiles/profile_impl.cc b/chrome/browser/profiles/profile_impl.cc
index 22e3911..0d4d077 100644
--- a/chrome/browser/profiles/profile_impl.cc
+++ b/chrome/browser/profiles/profile_impl.cc
@@ -124,6 +124,7 @@
 #endif
 
 #if defined(OS_CHROMEOS)
+#include "chrome/browser/chromeos/locale_change_guard.h"
 #include "chrome/browser/chromeos/login/user_manager.h"
 #include "chrome/browser/chromeos/preferences.h"
 #endif
@@ -1501,6 +1502,10 @@ void ProfileImpl::ChangeAppLocale(
   local_state->ScheduleSavePersistentPrefs();
 }
 
+void ProfileImpl::OnLogin() {
+  locale_change_guard_.reset(new chromeos::LocaleChangeGuard(this));
+}
+
 chromeos::ProxyConfigServiceImpl*
     ProfileImpl::GetChromeOSProxyConfigServiceImpl() {
   if (!chromeos_proxy_config_service_impl_) {
diff --git a/chrome/browser/profiles/profile_impl.h b/chrome/browser/profiles/profile_impl.h
index b2565b9..80b19a8 100644
--- a/chrome/browser/profiles/profile_impl.h
+++ b/chrome/browser/profiles/profile_impl.h
@@ -27,6 +27,7 @@ class PrefService;
 #if defined(OS_CHROMEOS)
 namespace chromeos {
 class Preferences;
+class LocaleChangeGuard;
 }
 #endif
 
@@ -137,6 +138,7 @@ class ProfileImpl : public Profile,
 
 #if defined(OS_CHROMEOS)
   virtual void ChangeAppLocale(const std::string& locale, AppLocaleChangedVia);
+  virtual void OnLogin();
   virtual chromeos::ProxyConfigServiceImpl* GetChromeOSProxyConfigServiceImpl();
   virtual void SetupChromeOSEnterpriseExtensionObserver();
   virtual void InitChromeOSPreferences();
@@ -306,6 +308,8 @@ class ProfileImpl : public Profile,
 
   scoped_ptr<chromeos::EnterpriseExtensionObserver>
       chromeos_enterprise_extension_observer_;
+
+  scoped_ptr<chromeos::LocaleChangeGuard> locale_change_guard_;
 #endif
 
   scoped_refptr<PrefProxyConfigTracker> pref_proxy_config_tracker_;
diff --git a/content/browser/tab_contents/tab_contents.cc b/content/browser/tab_contents/tab_contents.cc
index 4069494..213697f 100644
--- a/content/browser/tab_contents/tab_contents.cc
+++ b/content/browser/tab_contents/tab_contents.cc
@@ -319,7 +319,7 @@ TabContents::TabContents(Profile* profile,
   registrar_.Add(this, NotificationType::EXTENSION_UNLOADED,
                  NotificationService::AllSources());
 
-  // Listen for Google URL changes
+  // Listen for Google URL changes.
   registrar_.Add(this, NotificationType::GOOGLE_URL_UPDATED,
                  NotificationService::AllSources());
 
@@ -658,9 +658,6 @@ void TabContents::DidBecomeSelected() {
 
   WebCacheManager::GetInstance()->ObserveActivity(GetRenderProcessHost()->id());
   last_selected_time_ = base::TimeTicks::Now();
-#if defined(OS_CHROMEOS)
-  chromeos::LocaleChangeGuard::Check(this);
-#endif
 }
 
 void TabContents::FadeForInstant(bool animate) {