Make TrayDetailsView a ViewClickListener and ButtonListener

ash/common/system/tray/tray_details_view.h

 // 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.
 
 #ifndef ASH_COMMON_SYSTEM_TRAY_TRAY_DETAILS_VIEW_H_
 #define ASH_COMMON_SYSTEM_TRAY_TRAY_DETAILS_VIEW_H_
 
 #include "ash/ash_export.h"
 #include "ash/common/system/tray/special_popup_row.h"
+#include "ash/common/system/tray/view_click_listener.h"
 #include "base/macros.h"
+#include "ui/views/controls/button/button.h"
 #include "ui/views/view.h"
 
 namespace views {
 class ScrollView;
-}
+}  // namespace views
 
 namespace ash {
+namespace test {
+class TrayDetailsViewTest;
+}  // namespace test
+
 class FixedSizedScrollView;
 class ScrollBorder;
 class SystemTrayItem;
-class ViewClickListener;
 
-class ASH_EXPORT TrayDetailsView : public views::View {
+class ASH_EXPORT TrayDetailsView : public views::View,
+                                   public ViewClickListener,
+                                   public views::ButtonListener {
  public:
   explicit TrayDetailsView(SystemTrayItem* owner);
   ~TrayDetailsView() override;
 
-  // Creates a row with special highlighting etc. This is typically the
-  // bottom-most row in the popup.
-  void CreateSpecialRow(int string_id, ViewClickListener* listener);
+  // ViewClickListener:
+  void OnViewClicked(views::View* sender) override;
+
+  // views::ButtonListener:
+  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
+
+  SystemTrayItem* owner() const { return owner_; }
+  SpecialPopupRow* title_row() const { return title_row_; }
+  FixedSizedScrollView* scroller() const { return scroller_; }
+  views::View* scroll_content() const { return scroll_content_; }

[oshima, 2016/09/01 18:00:58]

ash uses the pattern where const method should return const internal object.
Does this have to be const (or can return value be const)?

[tdanderson, 2016/09/01 19:48:00]

Sorry, I don't understand your question - It seems OK as-is (an accessor should
be prevented from mutating any members). Can you please clarify?

[oshima, 2016/09/01 20:23:17]

const accessor shouldn't return non const internal object. Consider the
following:

class B {
   void modify() {
     value_ = 100;
   }
   int value_;
}

class A {
  void ModifyStateB() {
     b_->modify();
  }
  B* b() const {};

  B* b_;
};
  

method(const A* a) {
   a->ModifyStateB(); // won't compile.

   a->b()->modify();  // shouldn't work, but works.
}

+
+ protected:
+  // views::View:
+  void Layout() override;
+  void OnPaintBorder(gfx::Canvas* canvas) override;
+
+  // Creates the row containing the back button and title. For material design
+  // this appears at the top of the view, for non-material design it appears
+  // at the bottom.
+  void CreateTitleRow(int string_id);
 
   // Creates a scrollable list. The list has a border at the bottom if there is
   // any other view between the list and the footer row at the bottom.
   void CreateScrollableList();
 
   // Adds a separator in scrollable list.
   void AddScrollSeparator();
 
   // Removes (and destroys) all child views.
   void Reset();
 
-  // Transition to default view from details view. If |footer_| has focus before
-  // transition, the default view should focus on the owner of this details
-  // view.
+ private:
+  friend class test::TrayDetailsViewTest;
+
+  // Overridden to handle clicks on subclass-specific views.
+  virtual void HandleViewClicked(views::View* view);
+
+  // Overridden to handle button presses on subclass-specific buttons.
+  virtual void HandleButtonPressed(views::Button* sender,
+                                   const ui::Event& event);
+
+  // Creates and adds subclass-specific buttons to the title row.
+  virtual void CreateExtraTitleRowButtons();
+
+  // Transition to default view from details view. If |title_row_| has focus
+  // before transition, the default view should focus on the owner of this
+  // details view.
   void TransitionToDefaultView();
 
-  SystemTrayItem* owner() const { return owner_; }
-  SpecialPopupRow* footer() const { return footer_; }
-  FixedSizedScrollView* scroller() const { return scroller_; }
-  views::View* scroll_content() const { return scroll_content_; }
-
- protected:
-  // Overridden from views::View.
-  void Layout() override;
-  void OnPaintBorder(gfx::Canvas* canvas) override;
-
- private:
   SystemTrayItem* owner_;
-  SpecialPopupRow* footer_;
+  SpecialPopupRow* title_row_;
   FixedSizedScrollView* scroller_;
   views::View* scroll_content_;
   ScrollBorder* scroll_border_;  // Weak reference
 
   DISALLOW_COPY_AND_ASSIGN(TrayDetailsView);
 };
 
 }  // namespace ash
 
 #endif  // ASH_COMMON_SYSTEM_TRAY_TRAY_DETAILS_VIEW_H_