[Media Notification] Fix crash in background

This crash is still appearing and I think |owner_|
might be going away and cause the seg fault.

This removes |owner_| from MediaNotificationBackground.

BUG=994395

Change-Id: I59364ea14e9e7614728dac1e227133bcf76733c2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1772375Reviewed-by: Tommy Steimel <steimel@chromium.org>
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690768}

components/media_message_center/media_notification_background.cc

@@ -233,16 +233,12 @@ base::Optional<SkColor> GetNotificationForegroundColor(
 }  // namespace
 
 MediaNotificationBackground::MediaNotificationBackground(
-    views::View* owner,
     int top_radius,
     int bottom_radius,
     double artwork_max_width_pct)
-    : owner_(owner),
-      top_radius_(top_radius),
+    : top_radius_(top_radius),
       bottom_radius_(bottom_radius),
-      artwork_max_width_pct_(artwork_max_width_pct) {
-  DCHECK(owner);
-}
+      artwork_max_width_pct_(artwork_max_width_pct) {}
 
 MediaNotificationBackground::~MediaNotificationBackground() = default;
 
@@ -274,7 +270,7 @@ void MediaNotificationBackground::Paint(gfx::Canvas* canvas,
     // maintaining the aspect ratio.
     gfx::Rect source_bounds =
         gfx::Rect(0, 0, artwork_.width(), artwork_.height());
-    gfx::Rect artwork_bounds = GetArtworkBounds(bounds);
+    gfx::Rect artwork_bounds = GetArtworkBounds(*view);
 
     canvas->DrawImageInt(
         artwork_, source_bounds.x(), source_bounds.y(), source_bounds.width(),
@@ -286,11 +282,11 @@ void MediaNotificationBackground::Paint(gfx::Canvas* canvas,
   // notification. This may cover up some of the artwork.
   const SkColor background_color =
       background_color_.value_or(kMediaNotificationDefaultBackgroundColor);
-  canvas->FillRect(GetFilledBackgroundBounds(bounds), background_color);
+  canvas->FillRect(GetFilledBackgroundBounds(*view), background_color);
 
   {
     // Draw a gradient to fade the color background and the image together.
-    gfx::Rect draw_bounds = GetGradientBounds(bounds);
+    gfx::Rect draw_bounds = GetGradientBounds(*view);
 
     const SkColor colors[2] = {
         background_color, SkColorSetA(background_color, SK_AlphaTRANSPARENT)};
@@ -315,27 +311,25 @@ void MediaNotificationBackground::UpdateArtwork(const gfx::ImageSkia& image) {
   background_color_ = GetNotificationBackgroundColor(artwork_.bitmap());
   foreground_color_ =
       GetNotificationForegroundColor(background_color_, artwork_.bitmap());
-  owner_->SchedulePaint();
 }
 
-void MediaNotificationBackground::UpdateCornerRadius(int top_radius,
+bool MediaNotificationBackground::UpdateCornerRadius(int top_radius,
                                                      int bottom_radius) {
   if (top_radius_ == top_radius && bottom_radius_ == bottom_radius)
-    return;
+    return false;
 
   top_radius_ = top_radius;
   bottom_radius_ = bottom_radius;
-
-  owner_->SchedulePaint();
+  return true;
 }
 
-void MediaNotificationBackground::UpdateArtworkMaxWidthPct(
+bool MediaNotificationBackground::UpdateArtworkMaxWidthPct(
     double max_width_pct) {
   if (artwork_max_width_pct_ == max_width_pct)
-    return;
+    return false;
 
   artwork_max_width_pct_ = max_width_pct;
-  owner_->SchedulePaint();
+  return true;
 }
 
 SkColor MediaNotificationBackground::GetBackgroundColor() const {
@@ -344,11 +338,12 @@ SkColor MediaNotificationBackground::GetBackgroundColor() const {
   return kMediaNotificationDefaultBackgroundColor;
 }
 
-SkColor MediaNotificationBackground::GetForegroundColor() const {
+SkColor MediaNotificationBackground::GetForegroundColor(
+    const views::View& owner) const {
   const SkColor foreground =
       foreground_color_.has_value()
           ? *foreground_color_
-          : views::style::GetColor(*owner_, views::style::CONTEXT_LABEL,
+          : views::style::GetColor(owner, views::style::CONTEXT_LABEL,
                                    views::style::STYLE_PRIMARY);
   return color_utils::BlendForMinContrast(foreground, GetBackgroundColor())
       .color;
@@ -373,31 +368,34 @@ int MediaNotificationBackground::GetArtworkVisibleWidth(
 }
 
 gfx::Rect MediaNotificationBackground::GetArtworkBounds(
-    const gfx::Rect& view_bounds) const {
+    const views::View& owner) const {
+  const gfx::Rect& view_bounds = owner.GetContentsBounds();
   int width = GetArtworkWidth(view_bounds.size());
 
   // The artwork should be positioned on the far right hand side of the
   // notification and be the same height.
-  return owner_->GetMirroredRect(
+  return owner.GetMirroredRect(
       gfx::Rect(view_bounds.right() - width, 0, width, view_bounds.height()));
 }
 
 gfx::Rect MediaNotificationBackground::GetFilledBackgroundBounds(
-    const gfx::Rect& view_bounds) const {
+    const views::View& owner) const {
   // The filled background should take up the full notification except the area
   // taken up by the artwork.
+  const gfx::Rect& view_bounds = owner.GetContentsBounds();
   gfx::Rect bounds = gfx::Rect(view_bounds);
   bounds.Inset(0, 0, GetArtworkVisibleWidth(view_bounds.size()), 0);
-  return owner_->GetMirroredRect(bounds);
+  return owner.GetMirroredRect(bounds);
 }
 
 gfx::Rect MediaNotificationBackground::GetGradientBounds(
-    const gfx::Rect& view_bounds) const {
+    const views::View& owner) const {
   if (artwork_.isNull())
     return gfx::Rect(0, 0, 0, 0);
 
   // The gradient should appear above the artwork on the left.
-  return owner_->GetMirroredRect(gfx::Rect(
+  const gfx::Rect& view_bounds = owner.GetContentsBounds();
+  return owner.GetMirroredRect(gfx::Rect(
       view_bounds.width() - GetArtworkVisibleWidth(view_bounds.size()),
       view_bounds.y(), kMediaImageGradientWidth, view_bounds.height()));
 }

components/media_message_center/media_notification_background.h

@@ -29,8 +29,7 @@ namespace media_message_center {
 class COMPONENT_EXPORT(MEDIA_MESSAGE_CENTER) MediaNotificationBackground
     : public views::Background {
  public:
-  MediaNotificationBackground(views::View* owner,
-                              int top_radius,
+  MediaNotificationBackground(int top_radius,
                               int bottom_radius,
                               double artwork_max_width_pct);
   ~MediaNotificationBackground() override;
@@ -38,12 +37,12 @@ class COMPONENT_EXPORT(MEDIA_MESSAGE_CENTER) MediaNotificationBackground
   // views::Background
   void Paint(gfx::Canvas* canvas, views::View* view) const override;
 
-  void UpdateCornerRadius(int top_radius, int bottom_radius);
   void UpdateArtwork(const gfx::ImageSkia& image);
-  void UpdateArtworkMaxWidthPct(double max_width_pct);
+  bool UpdateCornerRadius(int top_radius, int bottom_radius);
+  bool UpdateArtworkMaxWidthPct(double max_width_pct);
 
   SkColor GetBackgroundColor() const;
-  SkColor GetForegroundColor() const;
+  SkColor GetForegroundColor(const views::View& owner) const;
 
  private:
   friend class MediaNotificationBackgroundTest;
@@ -53,15 +52,12 @@ class COMPONENT_EXPORT(MEDIA_MESSAGE_CENTER) MediaNotificationBackground
 
   int GetArtworkWidth(const gfx::Size& view_size) const;
   int GetArtworkVisibleWidth(const gfx::Size& view_size) const;
-  gfx::Rect GetArtworkBounds(const gfx::Rect& view_bounds) const;
-  gfx::Rect GetFilledBackgroundBounds(const gfx::Rect& view_bounds) const;
-  gfx::Rect GetGradientBounds(const gfx::Rect& view_bounds) const;
+  gfx::Rect GetArtworkBounds(const views::View& owner) const;
+  gfx::Rect GetFilledBackgroundBounds(const views::View& owner) const;
+  gfx::Rect GetGradientBounds(const views::View& owner) const;
   SkPoint GetGradientStartPoint(const gfx::Rect& draw_bounds) const;
   SkPoint GetGradientEndPoint(const gfx::Rect& draw_bounds) const;
 
-  // Reference to the owning view that this is a background for.
-  views::View* owner_;
-
   int top_radius_;
   int bottom_radius_;
 

components/media_message_center/media_notification_background_unittest.cc

@@ -73,16 +73,13 @@ class MediaNotificationBackgroundTest : public testing::Test {
   ~MediaNotificationBackgroundTest() override = default;
 
   void SetUp() override {
-    owner_ = std::make_unique<views::StaticSizedView>();
-    background_ = std::make_unique<MediaNotificationBackground>(owner_.get(),
-                                                                10, 10, 0.1);
+    background_ = std::make_unique<MediaNotificationBackground>(10, 10, 0.1);
 
     EXPECT_FALSE(GetBackgroundColor().has_value());
   }
 
   void TearDown() override {
     background_.reset();
-    owner_.reset();
   }
 
   MediaNotificationBackground* background() const { return background_.get(); }
@@ -96,7 +93,6 @@ class MediaNotificationBackgroundTest : public testing::Test {
   }
 
  private:
-  std::unique_ptr<views::StaticSizedView> owner_;
   std::unique_ptr<MediaNotificationBackground> background_;
 
   DISALLOW_COPY_AND_ASSIGN(MediaNotificationBackgroundTest);
@@ -322,6 +318,8 @@ class MediaNotificationBackgroundRTLTest
                                                 : switches::kForceDirectionLTR);
 
     MediaNotificationBackgroundTest::SetUp();
+
+    ASSERT_EQ(IsRTL(), base::i18n::IsRTL());
   }
 
   bool IsRTL() const { return GetParam(); }
@@ -336,15 +334,19 @@ INSTANTIATE_TEST_SUITE_P(, MediaNotificationBackgroundRTLTest, testing::Bool());
 TEST_P(MediaNotificationBackgroundRTLTest, BoundsSanityCheck) {
   // The test notification will have a width of 200 and a height of 50.
   gfx::Rect bounds(0, 0, 200, 50);
+  auto owner = std::make_unique<views::StaticSizedView>();
+  owner->SetBoundsRect(bounds);
+  ASSERT_EQ(bounds, owner->GetContentsBounds());
 
   // Check the artwork is not visible by default.
   EXPECT_EQ(0, background()->GetArtworkWidth(bounds.size()));
   EXPECT_EQ(0, background()->GetArtworkVisibleWidth(bounds.size()));
-  EXPECT_EQ(gfx::Rect(IsRTL() ? -200 : 200, 0, 0, 50),
-            background()->GetArtworkBounds(bounds));
-  EXPECT_EQ(gfx::Rect(IsRTL() ? -200 : 0, 0, 200, 50),
-            background()->GetFilledBackgroundBounds(bounds));
-  EXPECT_EQ(gfx::Rect(0, 0, 0, 0), background()->GetGradientBounds(bounds));
+  EXPECT_EQ(gfx::Rect(IsRTL() ? 0 : 200, 0, 0, 50),
+            background()->GetArtworkBounds(*owner.get()));
+  EXPECT_EQ(gfx::Rect(IsRTL() ? 0 : 0, 0, 200, 50),
+            background()->GetFilledBackgroundBounds(*owner.get()));
+  EXPECT_EQ(gfx::Rect(0, 0, 0, 0),
+            background()->GetGradientBounds(*owner.get()));
 
   // The background artwork image will have an aspect ratio of 2:1.
   SkBitmap bitmap;
@@ -362,22 +364,22 @@ TEST_P(MediaNotificationBackgroundRTLTest, BoundsSanityCheck) {
   EXPECT_EQ(100, background()->GetArtworkVisibleWidth(bounds.size()));
 
   // Check the artwork is positioned to the right.
-  EXPECT_EQ(gfx::Rect(IsRTL() ? -200 : 100, 0, 100, 50),
-            background()->GetArtworkBounds(bounds));
+  EXPECT_EQ(gfx::Rect(IsRTL() ? 0 : 100, 0, 100, 50),
+            background()->GetArtworkBounds(*owner.get()));
 
   // Check the filled background is to the left of the image.
-  EXPECT_EQ(gfx::Rect(IsRTL() ? -100 : 0, 0, 100, 50),
-            background()->GetFilledBackgroundBounds(bounds));
+  EXPECT_EQ(gfx::Rect(IsRTL() ? 100 : 0, 0, 100, 50),
+            background()->GetFilledBackgroundBounds(*owner.get()));
 
   // Check the gradient is positioned above the artwork.
-  const gfx::Rect gradient_bounds = background()->GetGradientBounds(bounds);
-  EXPECT_EQ(gfx::Rect(IsRTL() ? -140 : 100, 0, 40, 50), gradient_bounds);
+  const gfx::Rect gradient_bounds =
+      background()->GetGradientBounds(*owner.get());
+  EXPECT_EQ(gfx::Rect(IsRTL() ? 60 : 100, 0, 40, 50), gradient_bounds);
 
   // Check the gradient point X-values are the start and end of
   // |gradient_bounds|.
-  EXPECT_EQ(IsRTL() ? -100 : 100,
-            background()->GetGradientStartPoint(gradient_bounds).x());
-  EXPECT_EQ(IsRTL() ? -140 : 140,
+  EXPECT_EQ(100, background()->GetGradientStartPoint(gradient_bounds).x());
+  EXPECT_EQ(IsRTL() ? 60 : 140,
             background()->GetGradientEndPoint(gradient_bounds).x());
 
   // Check both of the gradient point Y-values are half the height.

components/media_message_center/media_notification_view.cc

@@ -203,7 +203,7 @@ MediaNotificationView::MediaNotificationView(
           IDS_MEDIA_MESSAGE_CENTER_MEDIA_NOTIFICATION_ACTION_NEXT_TRACK));
 
   SetBackground(std::make_unique<MediaNotificationBackground>(
-      this, message_center::kNotificationCornerRadius,
+      message_center::kNotificationCornerRadius,
       message_center::kNotificationCornerRadius, kMediaImageMaxWidthPct));
 
   UpdateForegroundColor();
@@ -235,8 +235,10 @@ void MediaNotificationView::SetExpanded(bool expanded) {
 
 void MediaNotificationView::UpdateCornerRadius(int top_radius,
                                                int bottom_radius) {
-  GetMediaNotificationBackground()->UpdateCornerRadius(top_radius,
-                                                       bottom_radius);
+  if (GetMediaNotificationBackground()->UpdateCornerRadius(top_radius,
+                                                           bottom_radius)) {
+    SchedulePaint();
+  }
 }
 
 void MediaNotificationView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
@@ -394,8 +396,10 @@ void MediaNotificationView::UpdateViewForExpandedState() {
 
   main_row_->Layout();
 
-  GetMediaNotificationBackground()->UpdateArtworkMaxWidthPct(
-      expanded ? kMediaImageMaxWidthExpandedPct : kMediaImageMaxWidthPct);
+  if (GetMediaNotificationBackground()->UpdateArtworkMaxWidthPct(
+          expanded ? kMediaImageMaxWidthExpandedPct : kMediaImageMaxWidthPct)) {
+    SchedulePaint();
+  }
 
   header_row_->SetExpanded(expanded);
 
@@ -439,7 +443,7 @@ void MediaNotificationView::UpdateForegroundColor() {
   const SkColor background =
       GetMediaNotificationBackground()->GetBackgroundColor();
   const SkColor foreground =
-      GetMediaNotificationBackground()->GetForegroundColor();
+      GetMediaNotificationBackground()->GetForegroundColor(*this);
 
   title_label_->SetEnabledColor(foreground);
   artist_label_->SetEnabledColor(foreground);