[Profiles] Remove the NotificationService from the ProfileInfoCache

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

 // Copyright (c) 2012 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/ui/views/frame/glass_browser_frame_view.h"
 
 #include "base/prefs/pref_service.h"
 #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"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/profiles/avatar_menu_button.h"
 #include "chrome/browser/ui/views/profiles/new_avatar_button.h"
 #include "chrome/browser/ui/views/tabs/tab.h"
 #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"
 #include "ui/base/theme_provider.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/icon_util.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/win/dpi.h"
 #include "ui/resources/grit/ui_resources.h"
 #include "ui/views/controls/label.h"
@@ skipping 57 matching lines @@
 // GlassBrowserFrameView, public:
 
 GlassBrowserFrameView::GlassBrowserFrameView(BrowserFrame* frame,
                                              BrowserView* browser_view)
     : BrowserNonClientFrameView(frame, browser_view),
       throbber_running_(false),
       throbber_frame_(0) {
   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() {
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // GlassBrowserFrameView, BrowserNonClientFrameView implementation:
 
 gfx::Rect GlassBrowserFrameView::GetBoundsForTabStrip(
     views::View* tabstrip) const {
@@ skipping 468 matching lines @@
   }
 }
 
 void GlassBrowserFrameView::DisplayNextThrobberFrame() {
   throbber_frame_ = (throbber_frame_ + 1) % kThrobberIconCount;
   SendMessage(views::HWNDForWidget(frame()), WM_SETICON,
               static_cast<WPARAM>(ICON_SMALL),
               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
 void GlassBrowserFrameView::InitThrobberIcons() {
   static bool initialized = false;
   if (!initialized) {
     for (int i = 0; i < kThrobberIconCount; ++i) {
       throbber_icons_[i] =
           ui::LoadThemeIconFromResourcesDataDLL(IDI_THROBBER_01 + i);
       DCHECK(throbber_icons_[i]);
     }
     initialized = true;
   }
 }