[Media Notification] Add previous and next track

Add previous and next track buttons to the media
notification.

BUG=893296

Change-Id: I2918e82bfa3ef11ce65e0fd8d21783e683cd3203
Reviewed-on: https://chromium-review.googlesource.com/c/1300114
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606289}

ash/media/media_notification_controller.cc

@@ -108,8 +108,22 @@ void MediaNotificationController::FlushForTesting() {
 
 void MediaNotificationController::OnNotificationClicked(
     base::Optional<int> button_id) {
-  DCHECK_EQ(0, *button_id);
-  media_controller_ptr_->ToggleSuspendResume();
+  DCHECK(button_id.has_value());
+
+  // TODO(beccahughes): Replace with MediaSessionAction enum when moved.
+  switch (*button_id) {
+    case 0:
+      media_controller_ptr_->PreviousTrack();
+      break;
+    case 1:
+      media_controller_ptr_->ToggleSuspendResume();
+      break;
+    case 2:
+      media_controller_ptr_->NextTrack();
+      break;
+    default:
+      NOTREACHED();
+  }
 }
 
 }  // namespace ash

ash/media/media_notification_view.cc

@@ -6,12 +6,13 @@
 
 #include "ash/media/media_notification_constants.h"
 #include "components/vector_icons/vector_icons.h"
-#include "third_party/skia/include/core/SkColor.h"
 #include "ui/message_center/message_center.h"
 #include "ui/message_center/public/cpp/message_center_constants.h"
 #include "ui/message_center/views/notification_control_buttons_view.h"
 #include "ui/message_center/views/notification_header_view.h"
+#include "ui/views/controls/button/image_button_factory.h"
 #include "ui/views/layout/box_layout.h"
+#include "ui/views/style/typography.h"
 
 namespace ash {
 
@@ -19,10 +20,12 @@ namespace {
 
 // Dimensions.
 constexpr gfx::Insets kButtonRowPadding(0, 12, 16, 12);
-constexpr gfx::Size kMediaButtonSize(64, 64);
 constexpr int kMediaButtonIconSize = 32;
 
-constexpr SkColor kControlIconColor = SK_ColorBLACK;
+SkColor GetMediaNotificationColor(const views::View& view) {
+  return views::style::GetColor(view, views::style::CONTEXT_LABEL,
+                                views::style::STYLE_PRIMARY);
+}
 
 }  // namespace
 
@@ -49,28 +52,28 @@ MediaNotificationView::MediaNotificationView(
   button_row_ = new views::View();
   auto* button_row_layout =
       button_row_->SetLayoutManager(std::make_unique<views::BoxLayout>(
-          views::BoxLayout::kHorizontal, kButtonRowPadding, 0));
+          views::BoxLayout::kHorizontal, kButtonRowPadding, 16));
   button_row_layout->set_main_axis_alignment(
       views::BoxLayout::MAIN_AXIS_ALIGNMENT_CENTER);
   button_row_layout->set_cross_axis_alignment(
-      views::BoxLayout::CROSS_AXIS_ALIGNMENT_CENTER);
+      views::BoxLayout::CROSS_AXIS_ALIGNMENT_STRETCH);
   AddChildView(button_row_);
 
+  CreateMediaButton(vector_icons::kMediaPreviousTrackIcon);
+
   // |play_pause_button_| toggles playback.
-  play_pause_button_ = new views::ToggleImageButton(this);
-  play_pause_button_->SetImageAlignment(views::ImageButton::ALIGN_CENTER,
-                                        views::ImageButton::ALIGN_MIDDLE);
-  play_pause_button_->SetSize(kMediaButtonSize);
-  play_pause_button_->SetImage(
-      views::Button::STATE_NORMAL,
-      gfx::CreateVectorIcon(vector_icons::kPlayArrowIcon, kMediaButtonIconSize,
-                            kControlIconColor));
-  const gfx::ImageSkia pause_image = gfx::CreateVectorIcon(
-      vector_icons::kPauseIcon, kMediaButtonIconSize, kControlIconColor);
-  play_pause_button_->SetToggledImage(views::Button::STATE_NORMAL,
-                                      &pause_image);
+  play_pause_button_ = views::CreateVectorToggleImageButton(this);
+  SkColor play_button_color = GetMediaNotificationColor(*play_pause_button_);
+  views::SetImageFromVectorIcon(play_pause_button_,
+                                vector_icons::kPlayArrowIcon,
+                                kMediaButtonIconSize, play_button_color);
+  views::SetToggledImageFromVectorIcon(play_pause_button_,
+                                       vector_icons::kPauseIcon,
+                                       kMediaButtonIconSize, play_button_color);
   button_row_->AddChildView(play_pause_button_);
 
+  CreateMediaButton(vector_icons::kMediaNextTrackIcon);
+
   // TODO(beccahughes): Update |play_pause_button_| based on state.
 
   // TODO(beccahughes): Add remaining UI for notification.
@@ -120,9 +123,9 @@ void MediaNotificationView::OnMouseEvent(ui::MouseEvent* event) {
 
 void MediaNotificationView::ButtonPressed(views::Button* sender,
                                           const ui::Event& event) {
-  if (sender == play_pause_button_) {
+  if (sender->parent() == button_row_) {
     message_center::MessageCenter::Get()->ClickOnNotificationButton(
-        notification_id(), 0);
+        notification_id(), sender->parent()->GetIndexOf(sender));
   }
 }
 
@@ -136,4 +139,11 @@ void MediaNotificationView::UpdateControlButtonsVisibilityWithNotification(
   UpdateControlButtonsVisibility();
 }
 
+void MediaNotificationView::CreateMediaButton(const gfx::VectorIcon& icon) {
+  views::ImageButton* button = views::CreateVectorImageButton(this);
+  views::SetImageFromVectorIcon(button, icon, kMediaButtonIconSize,
+                                GetMediaNotificationColor(*button));
+  button_row_->AddChildView(button);
+}
+
 }  // namespace ash

ash/media/media_notification_view.h

@@ -51,6 +51,9 @@ class ASH_EXPORT MediaNotificationView : public message_center::MessageView,
   void UpdateControlButtonsVisibilityWithNotification(
       const message_center::Notification& notification);
 
+  // Creates an image button with |icon| and adds it to |button_row_|.
+  void CreateMediaButton(const gfx::VectorIcon& icon);
+
   // View containing close and settings buttons.
   std::unique_ptr<message_center::NotificationControlButtonsView>
       control_buttons_view_;

ash/media/media_notification_view_unittest.cc

@@ -31,7 +31,9 @@ using media_session::test::TestMediaController;
 
 namespace {
 
-const int kMediaButtonIconSize = 32;
+// The icons size is 32 and INSETS_VECTOR_IMAGE_BUTTON will add padding around
+// the image.
+const int kMediaButtonIconSize = 40;
 
 }  // namespace
 
@@ -104,7 +106,6 @@ class MediaNotificationViewTest : public AshTestBase {
   }
 
   views::View* button_row() const { return view_->button_row_; }
-  views::View* play_pause_button() const { return view_->play_pause_button_; }
 
  private:
   std::unique_ptr<message_center::MessageView> CreateAndCaptureCustomView(
@@ -150,16 +151,37 @@ TEST_F(MediaNotificationViewTest, ButtonsSanityCheck) {
   EXPECT_GT(button_row()->width(), 0);
   EXPECT_GT(button_row()->height(), 0);
 
-  EXPECT_TRUE(play_pause_button()->visible());
-  EXPECT_EQ(kMediaButtonIconSize, play_pause_button()->width());
-  EXPECT_EQ(kMediaButtonIconSize, play_pause_button()->height());
+  EXPECT_EQ(3, button_row()->child_count());
+
+  for (int i = 0; i < button_row()->child_count(); ++i) {
+    const views::Button* child =
+        views::Button::AsButton(button_row()->child_at(i));
+
+    EXPECT_TRUE(child->visible());
+    EXPECT_EQ(kMediaButtonIconSize, child->width());
+    EXPECT_EQ(kMediaButtonIconSize, child->height());
+  }
+}
+
+TEST_F(MediaNotificationViewTest, NextTrackButtonClick) {
+  EXPECT_EQ(0, media_controller()->next_track_count());
+
+  gfx::Point cursor_location(1, 1);
+  views::View::ConvertPointToScreen(button_row()->child_at(2),
+                                    &cursor_location);
+  GetEventGenerator()->MoveMouseTo(cursor_location.x(), cursor_location.y());
+  GetEventGenerator()->ClickLeftButton();
+  Shell::Get()->media_notification_controller()->FlushForTesting();
+
+  EXPECT_EQ(1, media_controller()->next_track_count());
 }
 
 TEST_F(MediaNotificationViewTest, PlayPauseButtonClick) {
   EXPECT_EQ(0, media_controller()->toggle_suspend_resume_count());
 
   gfx::Point cursor_location(1, 1);
-  views::View::ConvertPointToScreen(play_pause_button(), &cursor_location);
+  views::View::ConvertPointToScreen(button_row()->child_at(1),
+                                    &cursor_location);
   GetEventGenerator()->MoveMouseTo(cursor_location.x(), cursor_location.y());
   GetEventGenerator()->ClickLeftButton();
   Shell::Get()->media_notification_controller()->FlushForTesting();
@@ -167,6 +189,19 @@ TEST_F(MediaNotificationViewTest, PlayPauseButtonClick) {
   EXPECT_EQ(1, media_controller()->toggle_suspend_resume_count());
 }
 
+TEST_F(MediaNotificationViewTest, PreviousTrackButtonClick) {
+  EXPECT_EQ(0, media_controller()->previous_track_count());
+
+  gfx::Point cursor_location(1, 1);
+  views::View::ConvertPointToScreen(button_row()->child_at(0),
+                                    &cursor_location);
+  GetEventGenerator()->MoveMouseTo(cursor_location.x(), cursor_location.y());
+  GetEventGenerator()->ClickLeftButton();
+  Shell::Get()->media_notification_controller()->FlushForTesting();
+
+  EXPECT_EQ(1, media_controller()->previous_track_count());
+}
+
 TEST_F(MediaNotificationViewTest, ClickNotification) {
   EXPECT_EQ(0, media_controller()->toggle_suspend_resume_count());
 

components/vector_icons/BUILD.gn

@@ -28,6 +28,8 @@ aggregate_vector_icons("components_vector_icons") {
     "lightbulb_outline.icon",
     "location_on.icon",
     "lock.icon",
+    "media_next_track.icon",
+    "media_previous_track.icon",
     "media_router_active.icon",
     "media_router_error.icon",
     "media_router_idle.icon",

components/vector_icons/media_next_track.icon

+// Copyright 2018 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.
+
+CANVAS_DIMENSIONS, 24,
+MOVE_TO, 6, 18,
+R_LINE_TO, 8.5f, -6,
+LINE_TO, 6, 6,
+R_V_LINE_TO, 12,
+CLOSE,
+MOVE_TO, 16, 6,
+R_V_LINE_TO, 12,
+R_H_LINE_TO, 2,
+V_LINE_TO, 6,
+R_H_LINE_TO, -2,
+CLOSE

components/vector_icons/media_previous_track.icon

+// Copyright 2018 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.
+
+CANVAS_DIMENSIONS, 24,
+MOVE_TO, 6, 6,
+R_H_LINE_TO, 2,
+R_V_LINE_TO, 12,
+H_LINE_TO, 6,
+CLOSE,
+R_MOVE_TO, 3.5f, 6,
+R_LINE_TO, 8.5f, 6,
+V_LINE_TO, 6,
+CLOSE

ui/views/controls/button/image_button_factory.cc

@@ -14,8 +14,9 @@
 
 namespace views {
 
-ImageButton* CreateVectorImageButton(ButtonListener* listener) {
-  ImageButton* button = new ImageButton(listener);
+namespace {
+
+void ConfigureVectorImageButton(ImageButton* button) {
   button->SetInkDropMode(Button::InkDropMode::ON);
   button->set_has_ink_drop_action_on_click(true);
   button->SetImageAlignment(ImageButton::ALIGN_CENTER,
@@ -23,21 +24,62 @@ ImageButton* CreateVectorImageButton(ButtonListener* listener) {
   button->SetFocusPainter(nullptr);
   button->SetBorder(CreateEmptyBorder(
       LayoutProvider::Get()->GetInsetsMetric(INSETS_VECTOR_IMAGE_BUTTON)));
+}
+
+}  // namespace
+
+ImageButton* CreateVectorImageButton(ButtonListener* listener) {
+  ImageButton* button = new ImageButton(listener);
+  ConfigureVectorImageButton(button);
+  return button;
+}
+
+ToggleImageButton* CreateVectorToggleImageButton(ButtonListener* listener) {
+  ToggleImageButton* button = new ToggleImageButton(listener);
+  ConfigureVectorImageButton(button);
   return button;
 }
 
 void SetImageFromVectorIcon(ImageButton* button,
                             const gfx::VectorIcon& icon,
                             SkColor related_text_color) {
+  SetImageFromVectorIcon(button, icon, GetDefaultSizeOfVectorIcon(icon),
+                         related_text_color);
+}
+
+void SetImageFromVectorIcon(ImageButton* button,
+                            const gfx::VectorIcon& icon,
+                            int dip_size,
+                            SkColor related_text_color) {
   const SkColor icon_color =
       color_utils::DeriveDefaultIconColor(related_text_color);
   const SkColor disabled_color =
       SkColorSetA(icon_color, gfx::kDisabledControlAlpha);
-  button->SetImage(Button::STATE_NORMAL,
-                   gfx::CreateVectorIcon(icon, icon_color));
-  button->SetImage(Button::STATE_DISABLED,
-                   gfx::CreateVectorIcon(icon, disabled_color));
+  const gfx::ImageSkia& normal_image =
+      gfx::CreateVectorIcon(icon, dip_size, icon_color);
+  const gfx::ImageSkia& disabled_image =
+      gfx::CreateVectorIcon(icon, dip_size, disabled_color);
+
+  button->SetImage(Button::STATE_NORMAL, normal_image);
+  button->SetImage(Button::STATE_DISABLED, disabled_image);
   button->set_ink_drop_base_color(icon_color);
 }
 
+void SetToggledImageFromVectorIcon(ToggleImageButton* button,
+                                   const gfx::VectorIcon& icon,
+                                   int dip_size,
+                                   SkColor related_text_color) {
+  const SkColor icon_color =
+      color_utils::DeriveDefaultIconColor(related_text_color);
+  const SkColor disabled_color =
+      SkColorSetA(icon_color, gfx::kDisabledControlAlpha);
+  const gfx::ImageSkia normal_image =
+      gfx::CreateVectorIcon(icon, dip_size, icon_color);
+  const gfx::ImageSkia disabled_image =
+      gfx::CreateVectorIcon(icon, dip_size, disabled_color);
+
+  button->SetToggledImage(Button::STATE_NORMAL, &normal_image);
+  button->SetToggledImage(Button::STATE_DISABLED, &disabled_image);
+}
+
 }  // views

ui/views/controls/button/image_button_factory.h

@@ -17,11 +17,18 @@ namespace views {
 
 class ButtonListener;
 class ImageButton;
+class ToggleImageButton;
 
 // Creates an ImageButton with an ink drop and a centered image in preparation
 // for applying a vector icon with SetImageFromVectorIcon below.
 VIEWS_EXPORT ImageButton* CreateVectorImageButton(ButtonListener* listener);
 
+// Creates a ToggleImageButton with an ink drop and a centered image in
+// preperation for applying a vector icon from SetImageFromVectorIcon and
+// SetToggledImageFromVectorIcon below.
+VIEWS_EXPORT ToggleImageButton* CreateVectorToggleImageButton(
+    ButtonListener* listener);
+
 // Sets images on |button| for STATE_NORMAL and STATE_DISABLED from the given
 // vector icon and color. |related_text_color| is normally the main text color
 // used in the parent view, and the actual color used is derived from that. Call
@@ -31,6 +38,20 @@ VIEWS_EXPORT void SetImageFromVectorIcon(
     const gfx::VectorIcon& icon,
     SkColor related_text_color = gfx::kGoogleGrey900);
 
+// As above, but creates the images at the given size.
+VIEWS_EXPORT void SetImageFromVectorIcon(
+    ImageButton* button,
+    const gfx::VectorIcon& icon,
+    int dip_size,
+    SkColor related_text_color = gfx::kGoogleGrey900);
+
+// As above, but sets the toggled images for a toggled image button.
+VIEWS_EXPORT void SetToggledImageFromVectorIcon(
+    ToggleImageButton* button,
+    const gfx::VectorIcon& icon,
+    int dip_size,
+    SkColor related_text_color = gfx::kGoogleGrey900);
+
 }  // namespace views
 
 #endif  // UI_VIEWS_CONTROLS_BUTTON_IMAGE_BUTTON_FACTORY_H_