GMC: Truncate text that reaches the artwork

Currently, the MediaNotificationView uses insets for the artwork based
on the message center width. This works for ChromeOS (which uses
message center to show the MediaNotificationView), but not for the
Global Media Controls, which uses its own width. This CL fixes this
issue by letting users of MediaNotificationView specify the width of
the notification.

Bug: 1011178
Change-Id: I267266a154b0bbd4e78ec4fec52734d32c2c9ebf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1864417
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706601}

ash/media/media_notification_container_impl.cc

@@ -5,6 +5,7 @@
 #include "ash/media/media_notification_container_impl.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/native_theme/native_theme_dark_aura.h"
 #include "ui/views/background.h"
@@ -19,11 +20,12 @@ MediaNotificationContainerImpl::MediaNotificationContainerImpl(
       control_buttons_view_(
           std::make_unique<message_center::NotificationControlButtonsView>(
               this)),
-      view_(this,
-            std::move(item),
-            control_buttons_view_.get(),
-            message_center::MessageCenter::Get()
-                ->GetSystemNotificationAppName()) {
+      view_(
+          this,
+          std::move(item),
+          control_buttons_view_.get(),
+          message_center::MessageCenter::Get()->GetSystemNotificationAppName(),
+          message_center::kNotificationWidth) {
   SetLayoutManager(std::make_unique<views::FillLayout>());
 
   // Since we own these, we don't want Views to destroy them when their parent

chrome/browser/ui/views/global_media_controls/media_notification_container_impl_view.cc

@@ -88,7 +88,8 @@ MediaNotificationContainerImplView::MediaNotificationContainerImplView(
   UpdateDismissButtonIcon();
 
   view_ = std::make_unique<media_message_center::MediaNotificationView>(
-      this, std::move(item), dismiss_button_container_.get(), base::string16());
+      this, std::move(item), dismiss_button_container_.get(), base::string16(),
+      kWidth);
   view_->set_owned_by_client();
   ForceExpandedState();
 

components/media_message_center/media_notification_view.cc

@@ -19,7 +19,6 @@
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/font.h"
 #include "ui/gfx/font_list.h"
-#include "ui/message_center/public/cpp/message_center_constants.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"
@@ -32,13 +31,6 @@ using media_session::mojom::MediaSessionAction;
 
 namespace {
 
-// The right padding is 1/5th the size of the notification.
-constexpr int kRightMarginSize = message_center::kNotificationWidth / 5;
-
-// The right padding is 1/3rd the size of the notification when the
-// notification is expanded.
-constexpr int kRightMarginExpandedSize = message_center::kNotificationWidth / 4;
-
 // The number of actions supported when the notification is expanded or not.
 constexpr size_t kMediaNotificationActionsCount = 3;
 constexpr size_t kMediaNotificationExpandedActionsCount = 5;
@@ -107,11 +99,13 @@ MediaNotificationView::MediaNotificationView(
     MediaNotificationContainer* container,
     base::WeakPtr<MediaNotificationItem> item,
     views::View* header_row_controls_view,
-    const base::string16& default_app_name)
+    const base::string16& default_app_name,
+    int notification_width)
     : container_(container),
       item_(std::move(item)),
       header_row_controls_view_(header_row_controls_view),
-      default_app_name_(default_app_name) {
+      default_app_name_(default_app_name),
+      notification_width_(notification_width) {
   DCHECK(container_);
 
   SetLayoutManager(std::make_unique<views::BoxLayout>(
@@ -407,7 +401,9 @@ void MediaNotificationView::UpdateViewForExpandedState() {
             views::BoxLayout::Orientation::kVertical,
             gfx::Insets(
                 kDefaultMarginSize, kDefaultMarginSize, kDefaultMarginSize,
-                has_artwork_ ? kRightMarginExpandedSize : kDefaultMarginSize),
+                has_artwork_
+                    ? (notification_width_ * kMediaImageMaxWidthExpandedPct)
+                    : kDefaultMarginSize),
             kDefaultMarginSize))
         ->SetDefaultFlex(1);
   } else {
@@ -418,7 +414,9 @@ void MediaNotificationView::UpdateViewForExpandedState() {
         ->SetLayoutManager(std::make_unique<views::BoxLayout>(
             views::BoxLayout::Orientation::kHorizontal,
             gfx::Insets(0, kDefaultMarginSize, 14,
-                        has_artwork_ ? kRightMarginSize : kDefaultMarginSize),
+                        has_artwork_
+                            ? (notification_width_ * kMediaImageMaxWidthPct)
+                            : kDefaultMarginSize),
             kDefaultMarginSize, true))
         ->SetFlexForView(title_artist_row_, 1);
   }

components/media_message_center/media_notification_view.h

@@ -65,7 +65,8 @@ class COMPONENT_EXPORT(MEDIA_MESSAGE_CENTER) MediaNotificationView
   MediaNotificationView(MediaNotificationContainer* container,
                         base::WeakPtr<MediaNotificationItem> item,
                         views::View* header_row_controls_view,
-                        const base::string16& default_app_name);
+                        const base::string16& default_app_name,
+                        int notification_width);
   ~MediaNotificationView() override;
 
   void SetExpanded(bool expanded);
@@ -127,6 +128,9 @@ class COMPONENT_EXPORT(MEDIA_MESSAGE_CENTER) MediaNotificationView
   // String to set as the app name of the header when there is no source title.
   base::string16 default_app_name_;
 
+  // Width of the notification in pixels. Used for calculating artwork bounds.
+  int notification_width_;
+
   bool has_artwork_ = false;
 
   // Whether this notification is expanded or not.

components/media_message_center/media_notification_view_unittest.cc

@@ -54,7 +54,10 @@ const char kTestDefaultAppName[] = "default app name";
 const char kTestAppName[] = "app name";
 
 const gfx::Size kWidgetSize(500, 500);
-const gfx::Size kViewSize(400, 400);
+
+constexpr int kViewWidth = 400;
+constexpr int kViewArtworkWidth = kViewWidth * 0.4;
+const gfx::Size kViewSize(kViewWidth, 400);
 
 // Checks if the view class name is used by a media button.
 bool IsMediaButtonType(const char* class_name) {
@@ -303,7 +306,7 @@ class MediaNotificationViewTest : public views::ViewsTestBase {
     auto view = std::make_unique<MediaNotificationView>(
         &container_, item_->GetWeakPtr(),
         nullptr /* header_row_controls_view */,
-        base::ASCIIToUTF16(kTestDefaultAppName));
+        base::ASCIIToUTF16(kTestDefaultAppName), kViewWidth);
     view->SetSize(kViewSize);
     view->set_owned_by_client();
 
@@ -792,6 +795,9 @@ TEST_F(MAYBE_MediaNotificationViewTest, UpdateArtworkFromItem) {
   // have artwork.
   EXPECT_GT(title_artist_width, title_artist_row()->width());
 
+  // Ensure that the title artist row does not extend into the artwork bounds.
+  EXPECT_LE(kViewWidth - kViewArtworkWidth, title_artist_row()->width());
+
   // Ensure that the image is displayed in the background artwork and that the
   // size of the notification was not affected.
   EXPECT_FALSE(GetArtworkImage().isNull());