views: fix initial style of LabelButton in activatable bubble on Mac

This is a quick fix for the bug on Mac that an activatable bubble
initially appears in inactive style even when it's in an active browser
window.

We generally want the inactive/active visual style to be in sync for all
widgets in a widget tree. Previously, PaintAsActiveLock partially
achieves this goal by having bubble widget lock its parent widget.
This solution has two caveats:

a) The lock is only on the parent widget, but not on other ancestors.
   If we have a password bubble with an focused in-product help promo,
   the password bubble will be in active style, but the browser window
   will be in inactive style.
b) The lock does not lock sibling widgets.
   This is the culprit of crbug/1112244 and is addressed by this CL,
   but only for LabelButton (which is the only impacted control).

A holistic solution that locks all widgets in tree is WIP to address
both caveats.

Bug: 1112244
Change-Id: I230e23d6880f44ab9b569fc1d57fb223d6a01ea1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2508447
Commit-Queue: Keren Zhu <kerenzhu@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822984}

ui/views/controls/button/label_button.cc

@@ -558,9 +558,16 @@ gfx::Size LabelButton::GetUnclampedSizeWithoutLabel() const {
 
 Button::ButtonState LabelButton::GetVisualState() const {
   const auto* widget = GetWidget();
-  if (PlatformStyle::kInactiveWidgetControlsAppearDisabled && widget &&
-      widget->CanActivate() && !widget->ShouldPaintAsActive())
+  if (!widget || !widget->CanActivate() ||
+      !PlatformStyle::kInactiveWidgetControlsAppearDisabled)
+    return GetState();
+
+  // Paint as inactive if neither this widget nor its parent should paint as
+  // active.
+  if (!widget->ShouldPaintAsActive() &&
+      !(widget->parent() && widget->parent()->ShouldPaintAsActive()))
     return STATE_DISABLED;
+
   return GetState();
 }
 

ui/views/controls/button/label_button.h

@@ -161,8 +161,8 @@ class VIEWS_EXPORT LabelButton : public Button, public NativeThemeDelegate {
   virtual void UpdateBackgroundColor() {}
 
   // Returns the current visual appearance of the button. This takes into
-  // account both the button's underlying state and the state of the containing
-  // widget.
+  // account both the button's underlying state, the state of the containing
+  // widget, and the parent of the containing widget.
   ButtonState GetVisualState() const;
 
   // Fills |params| with information about the button.

ui/views/controls/button/label_button_unittest.cc

@@ -74,6 +74,7 @@ class TestLabelButton : public LabelButton {
                            int button_context = style::CONTEXT_BUTTON)
       : LabelButton(Button::PressedCallback(), text, button_context) {}
 
+  using LabelButton::GetVisualState;
   using LabelButton::image;
   using LabelButton::label;
   using LabelButton::OnThemeChanged;
@@ -794,4 +795,90 @@ TEST_F(InkDropLabelButtonTest, TargetEventHandler) {
   EXPECT_EQ(button_, target_view);
 }
 
+class LabelButtonVisualStateTest : public test::WidgetTest {
+ public:
+  LabelButtonVisualStateTest() = default;
+  LabelButtonVisualStateTest(const LabelButtonVisualStateTest&) = delete;
+  LabelButtonVisualStateTest& operator=(const LabelButtonVisualStateTest&) =
+      delete;
+
+  // testing::Test:
+  void SetUp() override {
+    WidgetTest::SetUp();
+    test_widget_ = CreateTopLevelPlatformWidget();
+    dummy_widget_ = CreateTopLevelPlatformWidget();
+
+    button_ = MakeButtonAsContent(test_widget_);
+
+    style_of_inactive_widget_ =
+        PlatformStyle::kInactiveWidgetControlsAppearDisabled
+            ? Button::STATE_DISABLED
+            : Button::STATE_NORMAL;
+  }
+
+  void TearDown() override {
+    test_widget_->CloseNow();
+    dummy_widget_->CloseNow();
+    WidgetTest::TearDown();
+  }
+
+ protected:
+  std::unique_ptr<Widget> CreateActivatableChildWidget(Widget* parent) {
+    auto child = std::make_unique<Widget>();
+    Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP);
+    params.parent = parent->GetNativeView();
+    params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+    params.activatable = Widget::InitParams::ACTIVATABLE_YES;
+    child->Init(std::move(params));
+    child->SetContentsView(std::make_unique<View>());
+    return child;
+  }
+
+  TestLabelButton* MakeButtonAsContent(Widget* widget) {
+    return widget->GetContentsView()->AddChildView(
+        std::make_unique<TestLabelButton>());
+  }
+
+  TestLabelButton* button_ = nullptr;
+  Widget* test_widget_ = nullptr;
+  Widget* dummy_widget_ = nullptr;
+  Button::ButtonState style_of_inactive_widget_;
+};
+
+TEST_F(LabelButtonVisualStateTest, IndependentWidget) {
+  test_widget_->ShowInactive();
+  EXPECT_EQ(button_->GetVisualState(), style_of_inactive_widget_);
+
+  test_widget_->Activate();
+  EXPECT_EQ(button_->GetVisualState(), Button::STATE_NORMAL);
+
+  auto paint_as_active_lock = test_widget_->LockPaintAsActive();
+  dummy_widget_->Show();
+  EXPECT_EQ(button_->GetVisualState(), Button::STATE_NORMAL);
+}
+
+TEST_F(LabelButtonVisualStateTest, ChildWidget) {
+  std::unique_ptr<Widget> child_widget =
+      CreateActivatableChildWidget(test_widget_);
+  TestLabelButton* child_button = MakeButtonAsContent(child_widget.get());
+
+  test_widget_->Show();
+  EXPECT_EQ(button_->GetVisualState(), Button::STATE_NORMAL);
+  EXPECT_EQ(child_button->GetVisualState(), Button::STATE_NORMAL);
+
+  dummy_widget_->Show();
+  EXPECT_EQ(button_->GetVisualState(), style_of_inactive_widget_);
+  EXPECT_EQ(child_button->GetVisualState(), style_of_inactive_widget_);
+
+  child_widget->Show();
+#if defined(OS_MAC)
+  // Child widget is in a key window and it will lock its parent.
+  // See crrev.com/c/2048144.
+  EXPECT_EQ(button_->GetVisualState(), Button::STATE_NORMAL);
+#else
+  EXPECT_EQ(button_->GetVisualState(), style_of_inactive_widget_);
+#endif
+  EXPECT_EQ(child_button->GetVisualState(), Button::STATE_NORMAL);
+}
+
 }  // namespace views

ui/views/widget/widget.h

@@ -889,6 +889,7 @@ class VIEWS_EXPORT Widget : public internal::NativeWidgetDelegate,
   // Returns the parent of this widget. Note that a top-level widget is not
   // necessarily a root widget and can have a parent.
   Widget* parent() { return parent_; }
+  const Widget* parent() const { return parent_; }
 
   // True if the widget is considered top level widget. Top level widget
   // is a widget of TYPE_WINDOW, TYPE_PANEL, TYPE_WINDOW_FRAMELESS, BUBBLE,