Tab audio mute control (views UI), behind a switch (off by default).

chrome/browser/ui/views/tabs/tab.cc

Index: chrome/browser/ui/views/tabs/tab.cc
diff --git a/chrome/browser/ui/views/tabs/tab.cc b/chrome/browser/ui/views/tabs/tab.cc
index 441cb72677e4e493cced578cc9a8298f4c5ef68e..ee13dd30b081252b9520d4895c4c654559e94b81 100644
--- a/chrome/browser/ui/views/tabs/tab.cc
+++ b/chrome/browser/ui/views/tabs/tab.cc
@@ -21,6 +21,7 @@
 #include "chrome/browser/ui/views/touch_uma/touch_uma.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/grit/generated_resources.h"
+#include "content/public/browser/user_metrics.h"
 #include "grit/theme_resources.h"
 #include "third_party/skia/include/effects/SkGradientShader.h"
 #include "ui/accessibility/ax_view_state.h"
@@ -49,6 +50,8 @@
 #include "ui/views/widget/widget.h"
 #include "ui/views/window/non_client_view.h"
 
+using base::UserMetricsAction;
+
 namespace {
 
 // Padding around the "content" of a tab, occupied by the tab border graphics.
@@ -132,6 +135,7 @@ const double kImmersiveTabMinThrobOpacity = 0.66;
 const int kImmersiveLoadingStepCount = 32;
 
 const char kTabCloseButtonName[] = "TabCloseButton";
+const char kMediaIndicatorButtonName[] = "MediaIndicatorButton";
 
 void DrawIconAtLocation(gfx::Canvas* canvas,
                         const gfx::ImageSkia& image,
@@ -378,6 +382,162 @@ class Tab::TabCloseButton : public views::ImageButton,
 };
 
 ////////////////////////////////////////////////////////////////////////////////
+// MediaIndicatorButton
+//
+//  This is a Button subclass that serves as both the media indicator icon
+//  (audio, tab capture, etc.), and as a mute button.  When the indicator is
+//  transitioned to the audio playing or muting state, the button functionality
+//  is enabled and begins handling mouse events.  Otherwise, this view behaves
+//  like an image and all mouse events will be handled by the Tab (its parent
+//  View).
+class Tab::MediaIndicatorButton : public views::ImageButton,

[sky, 2014/09/23 22:58:18]

Move this into its own .h/.cc.

[miu, 2014/09/24 22:34:16]

Done.

+                                  public views::ViewTargeterDelegate {
+ public:
+  explicit MediaIndicatorButton(Tab* tab)

[sky, 2014/09/23 22:58:18]

You don't seem to use tab at all here.

[miu, 2014/09/24 22:34:16]

Done.  Yep, I can downcast from View::parent() when needed, and added DCHECK to
make sure it's a Tab.

+      : views::ImageButton(NULL),
+        media_state_(TAB_MEDIA_STATE_NONE),
+        showing_media_state_(TAB_MEDIA_STATE_NONE) {
+    SetEventTargeter(
+        scoped_ptr<views::ViewTargeter>(new views::ViewTargeter(this)));
+  }
+
+  virtual ~MediaIndicatorButton() {}
+
+  // Returns the current TabMediaState except, while the indicator image is
+  // fading out, returns the prior TabMediaState.
+  TabMediaState showing_media_state() const {
+    return showing_media_state_;
+  }
+
+  // Updates ImageButton images, starts fade animations, and
+  // activates/deactivates button functionality as appropriate.
+  void TransitionToMediaState(TabMediaState next_state) {
+    if (next_state == media_state_)
+      return;
+
+    if (next_state != TAB_MEDIA_STATE_NONE) {
+      const gfx::ImageSkia* const indicator_image =
+          chrome::GetTabMediaIndicatorImage(next_state).ToImageSkia();
+      SetImage(views::CustomButton::STATE_NORMAL, indicator_image);
+      SetImage(views::CustomButton::STATE_DISABLED, indicator_image);
+      const gfx::ImageSkia* const affordance_image =
+          chrome::GetTabMediaIndicatorAffordanceImage(next_state).ToImageSkia();
+      SetImage(views::CustomButton::STATE_HOVERED, affordance_image);
+      SetImage(views::CustomButton::STATE_PRESSED, affordance_image);
+    }
+
+    if ((media_state_ == TAB_MEDIA_STATE_AUDIO_PLAYING &&
+         next_state == TAB_MEDIA_STATE_AUDIO_MUTING) ||
+        (media_state_ == TAB_MEDIA_STATE_AUDIO_MUTING &&
+         next_state == TAB_MEDIA_STATE_AUDIO_PLAYING) ||
+        (media_state_ == TAB_MEDIA_STATE_AUDIO_MUTING &&
+         next_state == TAB_MEDIA_STATE_NONE)) {
+      // Instant user feedback: No fade animation.
+      showing_media_state_ = next_state;
+      media_indicator_animation_.reset();
+    } else {
+      if (next_state == TAB_MEDIA_STATE_NONE)
+        showing_media_state_ = media_state_;  // Fading-out indicator.
+      else
+        showing_media_state_ = next_state;  // Fading-in to next indicator.
+      media_indicator_animation_ =
+          chrome::CreateTabMediaIndicatorFadeAnimation(next_state);
+      media_indicator_animation_->set_delegate(this);
+      media_indicator_animation_->Start();
+    }
+
+    SetEnabled(chrome::IsTabAudioMutingFeatureEnabled() &&
+               (next_state == TAB_MEDIA_STATE_AUDIO_PLAYING ||
+                next_state == TAB_MEDIA_STATE_AUDIO_MUTING));
+
+    // An indicator state change should be made visible immediately, instead of
+    // the user being surprised when their mouse leaves the button.
+    if (state() == views::CustomButton::STATE_HOVERED) {
+      SetState(enabled() ? views::CustomButton::STATE_NORMAL :
+                           views::CustomButton::STATE_DISABLED);
+    }
+
+    media_state_ = next_state;
+  }
+
+ protected:
+  // views::View:
+  virtual const char* GetClassName() const OVERRIDE {
+    return kMediaIndicatorButtonName;
+  }
+
+  virtual View* GetTooltipHandlerForPoint(const gfx::Point& point) OVERRIDE {
+    return NULL;  // Tab (the parent View) provides the tooltip.
+  }
+
+  virtual bool OnMouseDragged(const ui::MouseEvent& event) OVERRIDE {
+    const ButtonState previous_state = state();
+    const bool ret = ImageButton::OnMouseDragged(event);
+    if (previous_state != views::CustomButton::STATE_NORMAL &&
+        state() == views::CustomButton::STATE_NORMAL)
+      content::RecordAction(UserMetricsAction("MediaIndicatorButton_Dragged"));
+    return ret;
+  }
+
+  virtual void OnPaint(gfx::Canvas* canvas) OVERRIDE {
+    double opaqueness = media_indicator_animation_ ?
+        media_indicator_animation_->GetCurrentValue() : 1.0;
+    if (media_state_ == TAB_MEDIA_STATE_NONE)
+      opaqueness = 1.0 - opaqueness;  // Fading out, not in.

[sky, 2014/09/23 22:58:18]

Can't you make the animation return the right value so you don't have to do
this?

[miu, 2014/09/24 22:34:16]

I think it goes against the intentions of gfx::Animation, which is always meant
to progress from 0.0 (semantically means start) to 1.0 (finish).  AFAICT, there
is no construct in gfx::Animation (or gfx::Tween) that allows progressing an
animation from 1.0 to 0.0.

Note that this isn't new code here (it is from the deleted
Tab::PaintMediaIndicator() method).  Also, whatever we change in tab_utils would
affect the Cocoa UI code, so if there's any action item here, it should be in a
different change.

[sky, 2014/09/25 19:25:04]

Well, it depends upon the animation. In theory you could make one that goes from
1 to 0, but you're right, we don't offer many that do this.

+    if (opaqueness < 1.0)
+      canvas->SaveLayerAlpha(opaqueness * SK_AlphaOPAQUE);
+    ImageButton::OnPaint(canvas);
+    if (opaqueness < 1.0)
+      canvas->Restore();
+  }
+
+  // views::ViewTargeterDelegate
+  virtual bool DoesIntersectRect(const View* target,
+                                 const gfx::Rect& rect) const OVERRIDE {
+    // If this button is not enabled, Tab (the parent View) handles all mouse
+    // events.
+    return enabled() &&
+        views::ViewTargeterDelegate::DoesIntersectRect(target, rect);
+  }
+
+  // views::Button:
+  virtual void NotifyClick(const ui::Event& event) OVERRIDE {

[sky, 2014/09/23 22:58:17]

Is it going to be annoying that the behavior of the button can change while the
mouse is over it?

[miu, 2014/09/24 22:34:16]

(I assume you're referring to the code/comment at line 453 above.)

Very likely not, and it should be rare that this ever happens at times other
than after a click on the button.  There's also a significant fade-in/out
duration, so the user experience shouldn't be jarring.

w.r.t. just after a click, there's a very good reason to do it: The default
behavior of ImageButton is to return to the "hover" image after a click. 
However, because MediaIndicatorButton doubles as an indicator, this makes it
unclear to the user whether the button they just pressed did something.  With my
logic, the "callback" from WebContents will cause the new muting state to be
shown, and IMO this is the natural behavior the user is expecting to see.

If you still have concerns, please try patching in this change and run with the
--enable-tab-audio-muting flag.  Or, at least be assured that this is a UI
experiment and we can iterate.  ;-)

[sky, 2014/09/25 19:25:04]

Just in general because the state may change at any point.

+    if (media_state_ == TAB_MEDIA_STATE_AUDIO_PLAYING)
+      content::RecordAction(UserMetricsAction("MuteTab"));
+    else if (media_state_ == TAB_MEDIA_STATE_AUDIO_MUTING)
+      content::RecordAction(UserMetricsAction("UnmuteTab"));
+    else
+      NOTREACHED();
+
+    if (Tab* const tab = static_cast<Tab*>(parent()))

[sky, 2014/09/23 22:58:18]

Can the parent really be NULL? I would expect this to be a DCHECK.

[miu, 2014/09/24 22:34:16]

Done.

+      tab->controller_->ToggleTabAudioMute(tab);
+  }
+
+  // gfx::AnimationDelegate
+  virtual void AnimationCanceled(const gfx::Animation* animation) OVERRIDE {

[sky, 2014/09/23 22:58:18]

Having ImageButton be the delegate for two animation classes is error prone. I
would rather you move the delegate for media_indicator_animation_ into it's own
class.

[miu, 2014/09/24 22:34:16]

Done.

+    showing_media_state_ = media_state_;
+    ImageButton::AnimationCanceled(animation);
+    SchedulePaint();
+  }
+
+  virtual void AnimationEnded(const gfx::Animation* animation) OVERRIDE {
+    showing_media_state_ = media_state_;
+    ImageButton::AnimationEnded(animation);
+    SchedulePaint();
+  }
+
+ private:
+  TabMediaState media_state_;
+
+  // Media indicator fade-in/out animation (i.e., only on show/hide, not a
+  // continuous animation).
+  scoped_ptr<gfx::Animation> media_indicator_animation_;
+  TabMediaState showing_media_state_;
+
+  DISALLOW_COPY_AND_ASSIGN(MediaIndicatorButton);
+};
+
+////////////////////////////////////////////////////////////////////////////////
 // ImageCacheEntry
 
 Tab::ImageCacheEntry::ImageCacheEntry()
@@ -409,8 +569,8 @@ Tab::Tab(TabController* controller)
       loading_animation_frame_(0),
       immersive_loading_step_(0),
       should_display_crashed_favicon_(false),
-      animating_media_state_(TAB_MEDIA_STATE_NONE),
       close_button_(NULL),
+      media_indicator_button_(NULL),
       title_(new views::Label()),
       tab_activated_with_last_tap_down_(false),
       hover_controller_(this),
@@ -509,11 +669,8 @@ void Tab::SetData(const TabRendererData& data) {
     ResetCrashedFavicon();
   }
 
-  if (data_.media_state != old.media_state) {
-    if (data_.media_state != TAB_MEDIA_STATE_NONE)
-      animating_media_state_ = data_.media_state;
-    StartMediaIndicatorAnimation();
-  }
+  if (data_.media_state != old.media_state)
+    LazyGetMediaIndicatorButton()->TransitionToMediaState(data_.media_state);
 
   if (old.mini != data_.mini) {
     StopAndDeleteAnimation(
@@ -642,14 +799,10 @@ void Tab::AnimationProgressed(const gfx::Animation* animation) {
 }
 
 void Tab::AnimationCanceled(const gfx::Animation* animation) {
-  if (media_indicator_animation_ == animation)
-    animating_media_state_ = data_.media_state;
   SchedulePaint();
 }
 
 void Tab::AnimationEnded(const gfx::Animation* animation) {
-  if (media_indicator_animation_ == animation)
-    animating_media_state_ = data_.media_state;
   SchedulePaint();
 }
 
@@ -657,6 +810,17 @@ void Tab::AnimationEnded(const gfx::Animation* animation) {
 // Tab, views::ButtonListener overrides:
 
 void Tab::ButtonPressed(views::Button* sender, const ui::Event& event) {
+  if (media_indicator_button_ && media_indicator_button_->visible()) {
+    if (media_indicator_button_->enabled())

[Mark P, 2014/09/23 18:56:15]

Are you planning to do sequence analysis?  If not, this type of structure looks
lot more like a histogram?  Consider recording a histogram with those four
values.

Also, I think we might have a different CloseTab user action that might be hit
on this flow.  Can you check?  i.e., you might need to add
CloseTab_NoMediaIndicator if you have a CloseTab that already is emitted by
something in this function / called by this function.

+      content::RecordAction(UserMetricsAction("CloseTab_MuteToggleAvailable"));
+    else if (data_.media_state == TAB_MEDIA_STATE_AUDIO_PLAYING)
+      content::RecordAction(UserMetricsAction("CloseTab_AudioIndicator"));
+    else
+      content::RecordAction(UserMetricsAction("CloseTab_CaptureIndicator"));

[Mark P, 2014/09/23 19:49:56]

Can you please give this a better name?

[miu, 2014/09/24 22:34:16]

Done.  Named it RecordingIndicator, per offline IM discussion.

+  } else {
+    content::RecordAction(UserMetricsAction("CloseTab_NoMediaIndicator"));
+  }
+
   const CloseTabSource source =
       (event.type() == ui::ET_MOUSE_RELEASED &&
        (event.flags() & ui::EF_FROM_TOUCH) == 0) ? CLOSE_TAB_FROM_MOUSE :
@@ -770,19 +934,21 @@ void Tab::Layout() {
   close_button_->SetVisible(showing_close_button_);
 
   showing_media_indicator_ = ShouldShowMediaIndicator();
-  media_indicator_bounds_.SetRect(lb.x(), lb.y(), 0, 0);
   if (showing_media_indicator_) {
-    const gfx::Image& media_indicator_image =
-        chrome::GetTabMediaIndicatorImage(animating_media_state_);
-    media_indicator_bounds_.set_width(media_indicator_image.Width());
-    media_indicator_bounds_.set_height(media_indicator_image.Height());
-    media_indicator_bounds_.set_y(
-        lb.y() + (lb.height() - media_indicator_bounds_.height() + 1) / 2);
+    views::ImageButton* const button = LazyGetMediaIndicatorButton();
+    const gfx::Size image_size(button->GetPreferredSize());
     const int right = showing_close_button_ ?
         close_button_->x() + close_button_->GetInsets().left() : lb.right();
-    media_indicator_bounds_.set_x(
-        std::max(lb.x(), right - media_indicator_bounds_.width()));
-    MaybeAdjustLeftForMiniTab(&media_indicator_bounds_);
+    gfx::Rect bounds(
+        std::max(lb.x(), right - image_size.width()),
+        lb.y() + (lb.height() - image_size.height() + 1) / 2,
+        image_size.width(),
+        image_size.height());
+    MaybeAdjustLeftForMiniTab(&bounds);
+    button->SetBoundsRect(bounds);

[sky, 2014/09/23 22:58:18]

Did you make sure this does the right thing when rtl?

[miu, 2014/09/24 22:34:15]

Good thing you had me check.  When I first implemented the indicator last year,
it was working for RTL locales.  However, it looks like it was broken with
r286754.  I've fixed the problem, and now everything works.

I modified the Tab layout unit tests to run in both LTR and RTL mode prevent
future regressions.

+    button->SetVisible(true);
+  } else if (media_indicator_button_) {
+    media_indicator_button_->SetVisible(false);
   }
 
   // Size the title to fill the remaining width and use all available height.
@@ -791,7 +957,7 @@ void Tab::Layout() {
     int title_left = favicon_bounds_.right() + kFaviconTitleSpacing;
     int title_width = lb.width() - title_left;
     if (showing_media_indicator_) {
-      title_width = media_indicator_bounds_.x() - kViewSpacing - title_left;
+      title_width = media_indicator_button_->x() - kViewSpacing - title_left;
     } else if (close_button_->visible()) {
       // Allow the title to overlay the close button's empty border padding.
       title_width = close_button_->x() + close_button_->GetInsets().left() -
@@ -904,6 +1070,11 @@ void Tab::OnMouseReleased(const ui::MouseEvent& event) {
     // selection. Reset it now to handle the case where multiple tabs were
     // selected.
     controller_->SelectTab(this);
+
+    if (media_indicator_button_ && media_indicator_button_->visible() &&
+        media_indicator_button_->bounds().Contains(event.location())) {
+      content::RecordAction(UserMetricsAction("TabMediaIndicator_Clicked"));
+    }
   }
 }
 
@@ -1020,9 +1191,6 @@ void Tab::PaintTab(gfx::Canvas* canvas) {
   if (show_icon)
     PaintIcon(canvas);
 
-  if (show_media_indicator)
-    PaintMediaIndicator(canvas);
-
   // If the close button color has changed, generate a new one.
   if (!close_button_color_ || title_color != close_button_color_) {
     close_button_color_ = title_color;
@@ -1341,28 +1509,6 @@ void Tab::PaintIcon(gfx::Canvas* canvas) {
   }
 }
 
-void Tab::PaintMediaIndicator(gfx::Canvas* canvas) {
-  if (media_indicator_bounds_.IsEmpty() || !media_indicator_animation_)
-    return;
-
-  gfx::Rect bounds = media_indicator_bounds_;
-  bounds.set_x(GetMirroredXForRect(bounds));
-
-  SkPaint paint;
-  paint.setAntiAlias(true);
-  double opaqueness = media_indicator_animation_->GetCurrentValue();
-  if (data_.media_state == TAB_MEDIA_STATE_NONE)
-    opaqueness = 1.0 - opaqueness;  // Fading out, not in.
-  paint.setAlpha(opaqueness * SK_AlphaOPAQUE);
-
-  const gfx::ImageSkia& media_indicator_image =
-      *(chrome::GetTabMediaIndicatorImage(animating_media_state_).
-            ToImageSkia());
-  DrawIconAtLocation(canvas, media_indicator_image, 0,
-                     bounds.x(), bounds.y(), media_indicator_image.width(),
-                     media_indicator_image.height(), true, paint);
-}
-
 void Tab::AdvanceLoadingAnimation(TabRendererData::NetworkState old_state,
                                   TabRendererData::NetworkState state) {
   static bool initialized = false;
@@ -1439,13 +1585,15 @@ int Tab::IconCapacity() const {
 bool Tab::ShouldShowIcon() const {
   return chrome::ShouldTabShowFavicon(
       IconCapacity(), data().mini, IsActive(), data().show_icon,
-      animating_media_state_);
+      media_indicator_button_ ? media_indicator_button_->showing_media_state() :
+                                data_.media_state);
 }
 
 bool Tab::ShouldShowMediaIndicator() const {
   return chrome::ShouldTabShowMediaIndicator(
       IconCapacity(), data().mini, IsActive(), data().show_icon,
-      animating_media_state_);
+      media_indicator_button_ ? media_indicator_button_->showing_media_state() :
+                                data_.media_state);
 }
 
 bool Tab::ShouldShowCloseBox() const {
@@ -1500,13 +1648,6 @@ bool Tab::IsPerformingCrashAnimation() const {
   return crash_icon_animation_.get() && data_.IsCrashed();
 }
 
-void Tab::StartMediaIndicatorAnimation() {
-  media_indicator_animation_ =
-      chrome::CreateTabMediaIndicatorFadeAnimation(data_.media_state);
-  media_indicator_animation_->set_delegate(this);
-  media_indicator_animation_->Start();
-}
-
 void Tab::ScheduleIconPaint() {
   gfx::Rect bounds = favicon_bounds_;
   if (bounds.IsEmpty())
@@ -1546,6 +1687,14 @@ void Tab::GetTabIdAndFrameId(views::Widget* widget,
   }
 }
 
+Tab::MediaIndicatorButton* Tab::LazyGetMediaIndicatorButton() {
+  if (!media_indicator_button_) {
+    media_indicator_button_ = new MediaIndicatorButton(this);
+    AddChildView(media_indicator_button_);  // Takes ownership.
+  }
+  return media_indicator_button_;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // Tab, private static: