Replace base::CallbackList with base::ObserverList in CastConfigDelegate

ash/system/cast/tray_cast.cc

 // Copyright 2015 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 "ash/system/cast/tray_cast.h"
 
 #include "ash/session/session_state_delegate.h"
 #include "ash/shelf/shelf_types.h"
 #include "ash/shell.h"
 #include "ash/system/chromeos/screen_security/screen_tray_item.h"
@@ skipping 24 matching lines @@
 
 namespace ash {
 
 namespace {
 
 const int kMaximumStatusStringLength = 100;
 const int kStopButtonRightPadding = 18;
 
 // Returns the active CastConfigDelegate instance.
 ash::CastConfigDelegate* GetCastConfigDelegate() {
+  if (!ash::Shell::GetInstance() ||

[achuithb, 2016/01/13 09:21:25]

Could you please add a comment here explaining when this happens? (I believe
shutdown?)

[jdufault, 2016/01/13 19:43:26]

Done.

+      !ash::Shell::GetInstance()->system_tray_delegate()) {
+    return nullptr;
+  }
+
   return ash::Shell::GetInstance()
       ->system_tray_delegate()
       ->GetCastConfigDelegate();
 }
 
 // Helper method to elide the given string to the maximum length. If a string is
 // contains user-input and is displayed, we should elide it.
 // TODO(jdufault): This does not properly trim unicode characters. We should
 // implement this properly by using views::Label::SetElideBehavior(...). See
 // crbug.com/532496.
@@ skipping 63 matching lines @@
   // Overridden from views::ButtonListener.
   void ButtonPressed(views::Button* sender, const ui::Event& event) override;
 
   // The cast activity id that we are displaying. If the user stops a cast, we
   // send this value to the config delegate so that we stop the right cast.
   std::string displayed_activity_id_;
 
   views::ImageView* icon_;
   views::Label* label_;
   TrayPopupLabelButton* stop_button_;
-  base::WeakPtrFactory<CastCastView> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(CastCastView);
 };
 
-CastCastView::CastCastView() : weak_ptr_factory_(this) {
+CastCastView::CastCastView() {
   // We will initialize the primary tray view which shows a stop button here.
 
   set_background(views::Background::CreateSolidBackground(kBackgroundColor));
   ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
   SetLayoutManager(new views::BoxLayout(views::BoxLayout::kHorizontal,
                                         kTrayPopupPaddingHorizontal, 0,
                                         kTrayPopupPaddingBetweenItems));
   icon_ = new FixedSizedImageView(0, kTrayPopupItemHeight);
   icon_->SetImage(
       bundle.GetImageNamed(IDR_AURA_UBER_TRAY_CAST_ENABLED).ToImageSkia());
@@ skipping 261 matching lines @@
   // Overridden from ViewClickListener.
   void OnViewClicked(views::View* sender) override;
 
   user::LoginStatus login_;
   views::View* options_ = nullptr;
   // A mapping from the receiver id to the receiver/activity data.
   std::map<std::string, CastConfigDelegate::ReceiverAndActivity>
       receivers_and_activities_;
   // A mapping from the view pointer to the associated activity id.
   std::map<views::View*, std::string> receiver_activity_map_;
-  base::WeakPtrFactory<CastDetailedView> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(CastDetailedView);
 };
 
 CastDetailedView::CastDetailedView(
     SystemTrayItem* owner,
     user::LoginStatus login,
     const CastConfigDelegate::ReceiversAndActivities& receivers_and_activities)
-    : TrayDetailsView(owner), login_(login), weak_ptr_factory_(this) {
+    : TrayDetailsView(owner), login_(login) {
   CreateItems();
   UpdateReceiverList(receivers_and_activities);
 }
 
 CastDetailedView::~CastDetailedView() {
 }
 
 void CastDetailedView::SimulateViewClickedForTest(
     const std::string& receiver_id) {
   for (auto& it : receiver_activity_map_) {
@@ skipping 109 matching lines @@
     if (it != receiver_activity_map_.end()) {
       cast_config_delegate->CastToReceiver(it->second);
       Shell::GetInstance()->metrics()->RecordUserMetricsAction(
           ash::UMA_STATUS_AREA_DETAILED_CAST_VIEW_LAUNCH_CAST);
     }
   }
 }
 
 }  // namespace tray
 
-TrayCast::TrayCast(SystemTray* system_tray)
-    : SystemTrayItem(system_tray),
-      weak_ptr_factory_(this) {
+TrayCast::TrayCast(SystemTray* system_tray) : SystemTrayItem(system_tray) {
   Shell::GetInstance()->AddShellObserver(this);
 }
 
 TrayCast::~TrayCast() {
   Shell::GetInstance()->RemoveShellObserver(this);

[achuithb, 2016/01/13 09:21:25]

Do we need to check Shell::GetInstance here?

[jdufault, 2016/01/13 19:43:26]

Done.

+
+  if (GetCastConfigDelegate())

[achuithb, 2016/01/13 09:21:25]

Using a temporary is an option here so we don't call GetCastConfigDelegate
twice.

Also, should we be checking added_observer_?

[jdufault, 2016/01/13 19:43:26]

Done.

+    GetCastConfigDelegate()->RemoveObserver(this);
 }
 
 void TrayCast::StartCastForTest(const std::string& receiver_id) {
   if (detailed_ != nullptr)
     detailed_->SimulateViewClickedForTest(receiver_id);
 }
 
 void TrayCast::StopCastForTest() {
   default_->cast_view()->StopCasting();
 }
@@ skipping 12 matching lines @@
   tray_->SetVisible(is_casting_);
   return tray_;
 }
 
 views::View* TrayCast::CreateDefaultView(user::LoginStatus status) {
   CHECK(default_ == nullptr);
 
   if (HasCastExtension()) {
     ash::CastConfigDelegate* cast_config_delegate = GetCastConfigDelegate();
 
     // We add the cast listener here instead of in the ctor for two reasons:

[achuithb, 2016/01/13 09:21:25]

Doesn't this comment needs updating since we now have an observer? Also, the
comment about the extension seems to not be entirely true  in the media router
case?

[jdufault, 2016/01/13 19:43:26]

Done.

     // - The ctor gets called too early in the initialization cycle (at least
     //   for the tests); the correct profile hasn't been setup yet.
     // - The listener is only added if there is a cast extension. If the call
     //   below were in the ctor, then the cast tray item would not appear if the
     //   user installed the extension in an existing session.
-    if (!device_update_subscription_) {
-      device_update_subscription_ =
-          cast_config_delegate->RegisterDeviceUpdateObserver(base::Bind(
-              &TrayCast::OnReceiversUpdated, weak_ptr_factory_.GetWeakPtr()));
+    if (!added_observer_) {
+      cast_config_delegate->AddObserver(this);
+      added_observer_ = true;
     }
 
     // The extension updates its view model whenever the popup is opened, so we
     // probably should as well.
     cast_config_delegate->RequestDeviceRefresh();
   }
 
   default_ = new tray::CastDuplexView(this, status != user::LOGGED_IN_LOCKED,
                                       receivers_and_activities_);
   default_->set_id(TRAY_VIEW);
@@ skipping 24 matching lines @@
 void TrayCast::DestroyDetailedView() {
   detailed_ = nullptr;
 }
 
 bool TrayCast::HasCastExtension() {
   ash::CastConfigDelegate* cast_config_delegate = GetCastConfigDelegate();
   return cast_config_delegate != nullptr &&
          cast_config_delegate->HasCastExtension();
 }
 
-void TrayCast::OnReceiversUpdated(
+void TrayCast::OnDevicesUpdated(
     const CastConfigDelegate::ReceiversAndActivities& receivers_activities) {
   receivers_and_activities_ = receivers_activities;
 
   if (default_) {
     bool has_receivers = !receivers_and_activities_.empty();
     default_->SetVisible(has_receivers);
     default_->cast_view()->UpdateLabel(receivers_and_activities_);
   }
   if (detailed_)
     detailed_->UpdateReceiverList(receivers_and_activities_);
@@ skipping 22 matching lines @@
   is_casting_ = started;
   UpdatePrimaryView();
 }
 
 void TrayCast::UpdateAfterShelfAlignmentChange(ShelfAlignment alignment) {
   if (tray_)
     tray_->UpdateAlignment(alignment);
 }
 
 }  // namespace ash