[vr] Refactor a vector icon button base class out of disc button

Previously the background and hit test planes would be sized incorrectly
if a disc button's corner radii were set.

Bug: None
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I1aeb4792aa06b51ac2fb8a839e672097690cabfa
Reviewed-on: https://chromium-review.googlesource.com/975581Reviewed-by: Christopher Grant <cjgrant@chromium.org>
Reviewed-by: Amirhossein Simjour <asimjour@chromium.org>
Commit-Queue: Ian Vollick <vollick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545132}

chrome/browser/vr/BUILD.gn

@@ -120,6 +120,8 @@ static_library("vr_common") {
     "elements/url_bar_texture.h",
     "elements/vector_icon.cc",
     "elements/vector_icon.h",
+    "elements/vector_icon_button.cc",
+    "elements/vector_icon_button.h",
     "elements/viewport_aware_root.cc",
     "elements/viewport_aware_root.h",
     "elements/webvr_url_toast.cc",
@@ -279,6 +281,7 @@ test("vr_common_unittests") {
     "elements/ui_element_iterator_unittest.cc",
     "elements/ui_element_unittest.cc",
     "elements/url_bar_texture_unittest.cc",
+    "elements/vector_icon_button_unittest.cc",
     "elements/vector_icon_unittest.cc",
     "elements/viewport_aware_root_unittest.cc",
     "fps_meter_unittest.cc",

chrome/browser/vr/elements/disc_button.cc

@@ -14,62 +14,15 @@
 
 namespace vr {
 
-namespace {
-
-constexpr float kDefaultIconScaleFactor = 0.5f;
-
-}  // namespace
-
 DiscButton::DiscButton(base::RepeatingCallback<void()> click_handler,
                        const gfx::VectorIcon& icon,
                        AudioDelegate* audio_delegate)
-    : Button(click_handler), icon_scale_factor_(kDefaultIconScaleFactor) {
-  auto vector_icon = std::make_unique<VectorIcon>(512);
-  vector_icon->SetType(kTypeButtonForeground);
-  vector_icon->SetIcon(icon);
-  vector_icon->set_bubble_events(true);
-  vector_icon->SetTransitionedProperties({TRANSFORM});
-  foreground_ = vector_icon.get();
-
-  // We now need to reparent the hit target so that it is a child of the
-  // newly-created foreground.
-  auto target = RemoveChild(hit_plane());
-  vector_icon->AddChild(std::move(target));
-  AddChild(std::move(vector_icon));
-
-  Sounds sounds;
-  sounds.hover_enter = kSoundButtonHover;
-  sounds.button_down = kSoundBackButtonClick;
-  SetSounds(sounds, audio_delegate);
-}
+    : VectorIconButton(click_handler, icon, audio_delegate) {}
 
 DiscButton::~DiscButton() = default;
 
-void DiscButton::OnStateUpdated() {
-  Button::OnStateUpdated();
-
-  if (hovered()) {
-    foreground_->SetTranslate(0.0, 0.0, hover_offset());
-  } else {
-    foreground_->SetTranslate(0.0, 0.0, 0.0);
-  }
-
-  foreground_->SetColor(colors().GetForegroundColor(!enabled()));
-}
-
-void DiscButton::OnSetDrawPhase() {
-  Button::OnSetDrawPhase();
-  foreground_->SetDrawPhase(draw_phase());
-}
-
-void DiscButton::OnSetName() {
-  Button::OnSetName();
-  foreground_->set_owner_name_for_test(name());
-}
-
 void DiscButton::OnSetCornerRadii(const CornerRadii& radii) {
-  Button::OnSetCornerRadii(radii);
-  foreground_->SetCornerRadii(radii);
+  NOTREACHED();
 }
 
 void DiscButton::NotifyClientSizeAnimated(const gfx::SizeF& size,
@@ -79,8 +32,8 @@ void DiscButton::NotifyClientSizeAnimated(const gfx::SizeF& size,
   if (target_property_id == BOUNDS) {
     background()->SetSize(size.width(), size.height());
     background()->set_corner_radius(size.width() * 0.5f);  // Creates a circle.
-    foreground()->SetSize(size.width() * icon_scale_factor_,
-                          size.height() * icon_scale_factor_);
+    foreground()->SetSize(size.width() * icon_scale_factor(),
+                          size.height() * icon_scale_factor());
     hit_plane()->SetSize(size.width(), size.height());
     hit_plane()->set_corner_radius(size.width() * 0.5f);  // Creates a circle.
   }

chrome/browser/vr/elements/disc_button.h

@@ -9,7 +9,7 @@
 
 #include "base/callback.h"
 #include "base/macros.h"
-#include "chrome/browser/vr/elements/button.h"
+#include "chrome/browser/vr/elements/vector_icon_button.h"
 #include "ui/gfx/vector_icon_types.h"
 
 namespace vr {
@@ -17,33 +17,21 @@ namespace vr {
 class VectorIcon;
 
 // A disc button has a circle as the background and a vector icon as the
-// foreground.  When hovered, background and foreground both move forward on Z
-// axis.  This matches the Daydream disk-style button.
-class DiscButton : public Button {
+// foreground. When hovered, background and foreground both move forward on Z
+// axis. This matches the Daydream disc-style button.
+class DiscButton : public VectorIconButton {
  public:
   DiscButton(base::RepeatingCallback<void()> click_handler,
              const gfx::VectorIcon& icon,
              AudioDelegate* audio_delegate);
   ~DiscButton() override;
 
-  VectorIcon* foreground() const { return foreground_; }
-
-  void set_icon_scale_factor(float factor) { icon_scale_factor_ = factor; }
-
  private:
-  void OnStateUpdated() override;
-  void OnSetDrawPhase() override;
-  void OnSetName() override;
   void OnSetCornerRadii(const CornerRadii& radii) override;
   void NotifyClientSizeAnimated(const gfx::SizeF& size,
                                 int target_property_id,
                                 cc::KeyframeModel* keyframe_model) override;
 
-  // This button will automatically scale down the given icon to fit the button.
-  // This value is used to determine the amount of scaling and can be set
-  // externally to create a smaller or larger icon.
-  float icon_scale_factor_;
-  VectorIcon* foreground_;
   DISALLOW_COPY_AND_ASSIGN(DiscButton);
 };
 

chrome/browser/vr/elements/vector_icon_button.cc

+// 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.
+
+#include "chrome/browser/vr/elements/vector_icon_button.h"
+
+#include "chrome/browser/vr/elements/rect.h"
+#include "chrome/browser/vr/elements/ui_element.h"
+#include "chrome/browser/vr/elements/ui_element_name.h"
+#include "chrome/browser/vr/elements/vector_icon.h"
+#include "chrome/browser/vr/ui_scene_constants.h"
+
+#include "ui/gfx/geometry/point_f.h"
+
+namespace vr {
+
+namespace {
+
+constexpr float kDefaultIconScaleFactor = 0.5f;
+
+}  // namespace
+
+VectorIconButton::VectorIconButton(
+    base::RepeatingCallback<void()> click_handler,
+    const gfx::VectorIcon& icon,
+    AudioDelegate* audio_delegate)
+    : Button(click_handler), icon_scale_factor_(kDefaultIconScaleFactor) {
+  auto vector_icon = std::make_unique<VectorIcon>(512);
+  vector_icon->SetType(kTypeButtonForeground);
+  vector_icon->SetIcon(icon);
+  vector_icon->set_bubble_events(true);
+  vector_icon->SetTransitionedProperties({TRANSFORM});
+  foreground_ = vector_icon.get();
+
+  // We now need to reparent the hit target so that it is a child of the
+  // newly-created foreground.
+  auto target = RemoveChild(hit_plane());
+  vector_icon->AddChild(std::move(target));
+  AddChild(std::move(vector_icon));
+
+  Sounds sounds;
+  sounds.hover_enter = kSoundButtonHover;
+  sounds.button_down = kSoundBackButtonClick;
+  SetSounds(sounds, audio_delegate);
+}
+
+VectorIconButton::~VectorIconButton() = default;
+
+void VectorIconButton::OnStateUpdated() {
+  Button::OnStateUpdated();
+
+  if (hovered()) {
+    foreground_->SetTranslate(0.0, 0.0, hover_offset());
+  } else {
+    foreground_->SetTranslate(0.0, 0.0, 0.0);
+  }
+
+  foreground_->SetColor(colors().GetForegroundColor(!enabled()));
+}
+
+void VectorIconButton::OnSetDrawPhase() {
+  Button::OnSetDrawPhase();
+  foreground_->SetDrawPhase(draw_phase());
+}
+
+void VectorIconButton::OnSetName() {
+  Button::OnSetName();
+  foreground_->set_owner_name_for_test(name());
+}
+
+void VectorIconButton::NotifyClientSizeAnimated(const gfx::SizeF& size,
+                                                int target_property_id,
+                                                cc::KeyframeModel* animation) {
+  Button::NotifyClientSizeAnimated(size, target_property_id, animation);
+  if (target_property_id == BOUNDS) {
+    background()->SetSize(size.width(), size.height());
+    foreground()->SetSize(size.width() * icon_scale_factor_,
+                          size.height() * icon_scale_factor_);
+    hit_plane()->SetSize(size.width(), size.height());
+  }
+}
+
+}  // namespace vr

chrome/browser/vr/elements/vector_icon_button.h

+// 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.
+
+#ifndef CHROME_BROWSER_VR_ELEMENTS_VECTOR_ICON_BUTTON_H_
+#define CHROME_BROWSER_VR_ELEMENTS_VECTOR_ICON_BUTTON_H_
+
+#include <memory>
+
+#include "base/callback.h"
+#include "base/macros.h"
+#include "chrome/browser/vr/elements/button.h"
+#include "ui/gfx/vector_icon_types.h"
+
+namespace vr {
+
+class VectorIcon;
+
+// A vector button has rect as a background and a vector icon as the
+// foreground. When hovered, background and foreground both move forward on Z
+// axis.
+class VectorIconButton : public Button {
+ public:
+  VectorIconButton(base::RepeatingCallback<void()> click_handler,
+                   const gfx::VectorIcon& icon,
+                   AudioDelegate* audio_delegate);
+  ~VectorIconButton() override;
+
+  VectorIcon* foreground() const { return foreground_; }
+
+  void set_icon_scale_factor(float factor) { icon_scale_factor_ = factor; }
+  float icon_scale_factor() const { return icon_scale_factor_; }
+
+ private:
+  void OnStateUpdated() override;
+  void OnSetDrawPhase() override;
+  void OnSetName() override;
+  void NotifyClientSizeAnimated(const gfx::SizeF& size,
+                                int target_property_id,
+                                cc::KeyframeModel* keyframe_model) override;
+
+  // This button will automatically scale down the given icon to fit the button.
+  // This value is used to determine the amount of scaling and can be set
+  // externally to create a smaller or larger icon.
+  float icon_scale_factor_;
+  VectorIcon* foreground_;
+  DISALLOW_COPY_AND_ASSIGN(VectorIconButton);
+};
+
+}  // namespace vr
+
+#endif  // CHROME_BROWSER_VR_ELEMENTS_VECTOR_ICON_BUTTON_H_

chrome/browser/vr/elements/vector_icon_button_unittest.cc

+// 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.
+
+#include "chrome/browser/vr/elements/vector_icon_button.h"
+
+#include "chrome/browser/vr/elements/rect.h"
+#include "chrome/browser/vr/elements/vector_icon.h"
+#include "components/vector_icons/vector_icons.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace vr {
+
+TEST(VectorIconButton, CornerRadiiOnResize) {
+  VectorIconButton button(base::RepeatingCallback<void()>(),
+                          vector_icons::kMicIcon, nullptr);
+  button.SetSize(1.0f, 1.0f);
+  button.set_corner_radius(0.25f);
+
+  EXPECT_FLOAT_EQ(0.25f, button.corner_radius());
+  // The foreground of vector icons is not not automatically affected by corner
+  // radius.
+  EXPECT_FLOAT_EQ(0.0f, button.foreground()->corner_radius());
+  EXPECT_FLOAT_EQ(0.25f, button.background()->corner_radius());
+  EXPECT_FLOAT_EQ(0.25f, button.hit_plane()->corner_radius());
+
+  button.SetSize(2.0f, 2.0f);
+
+  EXPECT_FLOAT_EQ(0.25f, button.corner_radius());
+  EXPECT_FLOAT_EQ(0.0f, button.foreground()->corner_radius());
+  EXPECT_FLOAT_EQ(0.25f, button.background()->corner_radius());
+  EXPECT_FLOAT_EQ(0.25f, button.hit_plane()->corner_radius());
+}
+
+}  // namespace vr

chrome/browser/vr/ui_scene_creator.cc

@@ -1258,7 +1258,7 @@ void UiSceneCreator::CreateWebVrTimeoutScreen() {
       Create<Text>(kWebVrTimeoutMessageButtonText, kPhaseForeground,
                    kTimeoutMessageTextFontHeightDMM);
 
-  // Disk-style button text is not uppercase. See crbug.com/787654.
+  // Disc-style button text is not uppercase. See https://crbug.com/787654.
   timeout_button_text->SetText(
       l10n_util::GetStringUTF16(IDS_VR_WEB_VR_EXIT_BUTTON_LABEL));
   timeout_button_text->SetColor(model_->color_scheme().web_vr_timeout_spinner);
@@ -1969,9 +1969,9 @@ void UiSceneCreator::CreateOverflowMenu() {
           {kOverflowMenuReloadButton, RIGHT, vector_icons::kReloadIcon},
       };
   for (auto& item : menu_buttons) {
-    auto button = Create<DiscButton>(std::get<0>(item), kPhaseForeground,
-                                     base::DoNothing(), std::get<2>(item),
-                                     audio_delegate_);
+    auto button = Create<VectorIconButton>(std::get<0>(item), kPhaseForeground,
+                                           base::DoNothing(), std::get<2>(item),
+                                           audio_delegate_);
     button->SetType(kTypeOverflowMenuButton);
     button->SetDrawPhase(kPhaseForeground);
     button->SetSize(kUrlBarButtonSizeDMM, kUrlBarButtonSizeDMM);
@@ -2317,7 +2317,7 @@ void UiSceneCreator::CreateOmnibox() {
           },
           base::Unretained(omnibox_text_field.get()))));
 
-  auto mic_button = Create<DiscButton>(
+  auto mic_button = Create<VectorIconButton>(
       kOmniboxVoiceSearchButton, kPhaseForeground,
       base::BindRepeating(
           [](UiBrowserInterface* b, Ui* ui) { b->SetVoiceSearchActive(true); },