[Profiles] Remove the NotificationService from the ProfileInfoCache

chrome/browser/ui/views/frame/glass_browser_frame_view.cc

Index: chrome/browser/ui/views/frame/glass_browser_frame_view.cc
diff --git a/chrome/browser/ui/views/frame/glass_browser_frame_view.cc b/chrome/browser/ui/views/frame/glass_browser_frame_view.cc
index 4eaaf0647767725b64d2b1ea8db8a70f7f093b33..61f03855ea12bc38c7804625b12d26a9990cafb9 100644
--- a/chrome/browser/ui/views/frame/glass_browser_frame_view.cc
+++ b/chrome/browser/ui/views/frame/glass_browser_frame_view.cc
@@ -8,7 +8,6 @@
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/app/chrome_dll_resource.h"
-#include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/signin_header_helper.h"
 #include "chrome/browser/themes/theme_properties.h"
@@ -19,7 +18,6 @@
 #include "chrome/browser/ui/views/tabs/tab_strip.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "components/signin/core/common/profile_management_switches.h"
-#include "content/public/browser/notification_service.h"
 #include "grit/theme_resources.h"
 #include "skia/ext/image_operations.h"
 #include "ui/base/resource/resource_bundle_win.h"
@@ -97,15 +95,7 @@ GlassBrowserFrameView::GlassBrowserFrameView(BrowserFrame* frame,
   if (browser_view->ShouldShowWindowIcon())
     InitThrobberIcons();
 
-  if (browser_view->IsRegularOrGuestSession() && switches::IsNewAvatarMenu())
-    UpdateNewStyleAvatarInfo(this, NewAvatarButton::NATIVE_BUTTON);
-  else
-    UpdateAvatarInfo();
-
-  if (!browser_view->IsOffTheRecord()) {
-    registrar_.Add(this, chrome::NOTIFICATION_PROFILE_CACHED_INFO_CHANGED,
-                   content::NotificationService::AllSources());
-  }
+  UpdateAvatar(this, NewAvatarButton::NATIVE_BUTTON);
 }
 
 GlassBrowserFrameView::~GlassBrowserFrameView() {
@@ -594,23 +584,27 @@ void GlassBrowserFrameView::DisplayNextThrobberFrame() {
               reinterpret_cast<LPARAM>(throbber_icons_[throbber_frame_]));
 }
 
-void GlassBrowserFrameView::Observe(
-    int type,
-    const content::NotificationSource& source,
-    const content::NotificationDetails& details) {
-  switch (type) {
-    case chrome::NOTIFICATION_PROFILE_CACHED_INFO_CHANGED:
-      if (browser_view()->IsRegularOrGuestSession() &&
-          switches::IsNewAvatarMenu()) {
-        UpdateNewStyleAvatarInfo(this, NewAvatarButton::NATIVE_BUTTON);
-      } else {
-        UpdateAvatarInfo();
-      }
-      break;
-    default:
-      NOTREACHED() << "Got a notification we didn't register for!";
-      break;
-  }
+void GlassBrowserFrameView::OnProfileAdded(const base::FilePath& profile_path) {
+  UpdateAvatar(this, NewAvatarButton::NATIVE_BUTTON);

[sky, 2015/02/19 15:59:36]

The new code for all this looks quite a bit different than the old. I'm assuming
you know what you're doing;)

[noms (inactive), 2015/02/19 16:07:19]

Oof, that's some misplaced confidence :)

Jokes aside, very little has actually changed. Before we were updating the
button any time the ProfileInfoCache changed, now we're only updating it when
things affect it (the active name changes, profiles get added/deleted). A lot of
actions (changing the name of a different profile), don't really affect *this*
browser window.

And UpdateAvatar just engulfed lines 603-608.

+}
+
+void GlassBrowserFrameView::OnProfileWasRemoved(
+    const base::FilePath& profile_path,
+    const base::string16& profile_name) {
+  UpdateAvatar(this, NewAvatarButton::NATIVE_BUTTON);
+}
+
+void GlassBrowserFrameView::OnProfileNameChanged(
+    const base::FilePath& profile_path,
+    const base::string16& old_profile_name) {
+  UpdateAvatar(this, NewAvatarButton::NATIVE_BUTTON);
+}
+
+void GlassBrowserFrameView::OnProfileAvatarChanged(
+    const base::FilePath& profile_path) {
+  // Profile avatars are only displayed in the old UI or incognito.
+  if (browser_view()->IsOffTheRecord() || !switches::IsNewAvatarMenu())

[sky, 2015/02/19 15:59:36]

Seems silly to duplicate code like this between glass and opaque. Is there a way
to refactor common code like this? Maybe a new superclass that glass/opaque
share?

[noms (inactive), 2015/02/19 16:07:19]

Do you think BrowserNonClientFrameView would be a good place? It's the one that
actually defines the listener (glass and opaque inherit it by magic), and this
UpdateAvatarInfo() function. I will try that.

[sky, 2015/02/19 16:09:18]

I was assuming you didn't want this for ash, in which case promoting doesn't
make sense. Would it make more sense to move the profile/avatar functionality
off into it's own class that both glass and opaque browser frame view own?

[noms (inactive), 2015/02/19 16:11:49]

Oh Ash has this button too (Metro mode, not ChromeOS)

[noms, 2015/02/20 21:24:34]

Ok, done. I still had to override a method, as the avatar button needs to know
what kind of button it is (and the BrowserNonClientFrameView doesn't know), but
I think it looks much cleaner now.

+    UpdateAvatarInfo();
 }
 
 // static