Revert "Implemented fluent style for select multiple"

This reverts commit 6881d10. Reason for revert: Introduces a use of uninitialized value. NativeThemeBase::PaintInnerSpinButton passes |arrow| to PaintArrowButton without initializing its |right_to_left| member. See https://crbug.com/990672 Original change's description: > Implemented fluent style for select multiple > > -Styled the in-page select, options, and optgroups via css. > -Modified the scrollbar to have rounded corners > (had to plumb the zoom factor through the Scrollbar so that the native > theme aura code can get the rounding correct at paint time) > > Bug: 987292 > Change-Id: Id11d1a47fe3d5b2be01aa578aa066f80577a9bbd > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1715553 > Commit-Queue: Allison Pastewka <alpastew@microsoft.com> > Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> > Reviewed-by: Kent Tamura <tkent@chromium.org> > Reviewed-by: Kentaro Hara <haraken@chromium.org> > Reviewed-by: Mason Freed <masonfreed@chromium.org> > Cr-Commit-Position: refs/heads/master@{#683728} TBR=ellyjones@chromium.org,chrishtr@chromium.org,haraken@chromium.org,tkent@chromium.org,masonfreed@chromium.org,alpastew@microsoft.com # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 987292, 990672 Change-Id: I778b09437c79061b23de1d5e8335c8f0cf124e12 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1735548 Reviewed-by: Matt Giuca <mgiuca@chromium.org> Reviewed-by: Kent Tamura <tkent@chromium.org> Commit-Queue: Matt Giuca <mgiuca@chromium.org> Cr-Commit-Position: refs/heads/master@{#683912}
Denger-Network · Aug 5, 2019 · af9106b · af9106b
1 parent 9193384
commit af9106b
Show file tree

Hide file tree

Showing 33 changed files with 50 additions and 361 deletions.
diff --git a/chrome/browser/ui/libgtkui/native_theme_gtk.cc b/chrome/browser/ui/libgtkui/native_theme_gtk.cc
@@ -418,13 +418,11 @@ SkColor NativeThemeGtk::GetSystemColor(ColorId color_id,
   return color;
 }
 
-void NativeThemeGtk::PaintArrowButton(
-    cc::PaintCanvas* canvas,
-    const gfx::Rect& rect,
-    Part direction,
-    State state,
-    ColorScheme color_scheme,
-    const ScrollbarArrowExtraParams& arrow) const {
+void NativeThemeGtk::PaintArrowButton(cc::PaintCanvas* canvas,
+                                      const gfx::Rect& rect,
+                                      Part direction,
+                                      State state,
+                                      ColorScheme color_scheme) const {
   auto context = GetStyleContextFromCss(
       GtkVersionCheck(3, 20)
           ? "GtkScrollbar#scrollbar #contents GtkButton#button"

diff --git a/chrome/browser/ui/libgtkui/native_theme_gtk.h b/chrome/browser/ui/libgtkui/native_theme_gtk.h
@@ -33,8 +33,7 @@ class NativeThemeGtk : public ui::NativeThemeBase {
                         const gfx::Rect& rect,
                         Part direction,
                         State state,
-                        ColorScheme color_scheme,
-                        const ScrollbarArrowExtraParams& arrow) const override;
+                        ColorScheme color_scheme) const override;
   void PaintScrollbarTrack(cc::PaintCanvas* canvas,
                            Part part,
                            State state,

diff --git a/content/child/webthemeengine_impl_default.cc b/content/child/webthemeengine_impl_default.cc
@@ -216,15 +216,6 @@ static void GetNativeThemeExtraParams(
           NativeThemeScrollbarOverlayColorTheme(
               extra_params->scrollbar_thumb.scrollbar_theme);
       break;
-    case WebThemeEngine::kPartScrollbarDownArrow:
-    case WebThemeEngine::kPartScrollbarLeftArrow:
-    case WebThemeEngine::kPartScrollbarRightArrow:
-    case WebThemeEngine::kPartScrollbarUpArrow:
-      native_theme_extra_params->scrollbar_arrow.zoom =
-          extra_params->scrollbar_button.zoom;
-      native_theme_extra_params->scrollbar_arrow.right_to_left =
-          extra_params->scrollbar_button.right_to_left;
-      break;
     default:
       break;  // Parts that have no extra params get here.
   }

diff --git a/third_party/blink/public/platform/web_theme_engine.h b/third_party/blink/public/platform/web_theme_engine.h
@@ -148,11 +148,6 @@ class WebThemeEngine {
     WebScrollbarOverlayColorTheme scrollbar_theme;
   };
 
-  struct ScrollbarButtonExtraParams {
-    float zoom;
-    bool right_to_left;
-  };
-
   union ExtraParams {
     ScrollbarTrackExtraParams scrollbar_track;
     ButtonExtraParams button;
@@ -162,7 +157,6 @@ class WebThemeEngine {
     InnerSpinButtonExtraParams inner_spin;
     ProgressBarExtraParams progress_bar;
     ScrollbarThumbExtraParams scrollbar_thumb;
-    ScrollbarButtonExtraParams scrollbar_button;
   };
 
   virtual ~WebThemeEngine() {}

diff --git a/third_party/blink/renderer/core/html/resources/controls_refresh.css b/third_party/blink/renderer/core/html/resources/controls_refresh.css
@@ -10,15 +10,13 @@
 
 input,
 select,
-select:-internal-list-box,
 textarea {
   background-color: #ffffff;
   border-color: #cecece;
 }
 
 input:hover,
 select:hover,
-select:-internal-list-box:hover,
 textarea:hover {
   border-color: #9d9d9d;
 }
@@ -105,55 +103,3 @@ meter::-webkit-meter-even-less-good-value {
   background: #d83b01;
   border-radius: 1px 0px 0px 1px;
 }
-
-/* -internal-list-box is how we specify select[multiple] */
-/* select[multiple] is an exception to the prohibition on sizes here
-   because it is one of the few controls with borders that grow on zoom
-   (to solve a nasty scrollbar location problem) */
-select:-internal-list-box {
-  border-radius: 2px;
-}
-
-/* These options only apply to in-page 'multiple' select elements.
-   The options in the popup are handled in listPicker.css */
-select:-internal-list-box option, select:-internal-list-box optgroup {
-  padding: 0 3px;
-}
-
-select:-internal-list-box option {
-  border-radius: 2px;
-}
-
-select:-internal-list-box option:hover {
-  background-color: #f3f3f3;
-}
-
-/* option selected */
-select:-internal-list-box:focus option:checked,
-select:-internal-list-box:focus option:checked:hover,
-select:-internal-list-box option:checked,
-select:-internal-list-box option:checked:hover {
-  background-color: #cecece !important;
-  color: #101010 !important;
-}
-
-/* option disabled */
-select:-internal-list-box option:disabled,
-select:-internal-list-box option:disabled:hover,
-select:-internal-list-box:disabled option,
-select:-internal-list-box:disabled option:hover {
-  background-color: #ffffff !important;
-}
-
-/* option both disabled and selected */
-option:checked:disabled,
-option:checked:disabled:hover,
-select:-internal-list-box:focus option:checked:disabled,
-select:-internal-list-box:focus option:checked:disabled:hover,
-select:-internal-list-box:disabled option:checked,
-select:-internal-list-box:disabled option:checked:hover,
-select:-internal-list-box option:checked:disabled,
-select:-internal-list-box option:checked:disabled:hover {
-  background-color: #f0f0f0 !important;
-  color: #c5c5c5 !important;
-}
diff --git a/third_party/blink/renderer/core/layout/layout_scrollbar.cc b/third_party/blink/renderer/core/layout/layout_scrollbar.cc
@@ -50,9 +50,9 @@ LayoutScrollbar::LayoutScrollbar(ScrollableArea* scrollable_area,
     : Scrollbar(scrollable_area,
                 orientation,
                 kRegularScrollbar,
-                style_source,
                 nullptr,
-                LayoutScrollbarTheme::GetLayoutScrollbarTheme()) {
+                LayoutScrollbarTheme::GetLayoutScrollbarTheme()),
+      style_source_(style_source) {
   DCHECK(style_source);
 
   // FIXME: We need to do this because LayoutScrollbar::styleChanged is called
@@ -104,6 +104,7 @@ int LayoutScrollbar::HypotheticalScrollbarThickness(
 }
 
 void LayoutScrollbar::Trace(blink::Visitor* visitor) {
+  visitor->Trace(style_source_);
   Scrollbar::Trace(visitor);
 }
 
@@ -152,11 +153,11 @@ void LayoutScrollbar::SetPressedPart(ScrollbarPart part,
 scoped_refptr<ComputedStyle> LayoutScrollbar::GetScrollbarPseudoStyle(
     ScrollbarPart part_type,
     PseudoId pseudo_id) {
-  if (!StyleSource()->GetLayoutObject())
+  if (!style_source_->GetLayoutObject())
     return nullptr;
-  return StyleSource()->StyleForPseudoElement(
+  return style_source_->StyleForPseudoElement(
       PseudoStyleRequest(pseudo_id, this, part_type),
-      StyleSource()->GetLayoutObject()->Style());
+      style_source_->GetLayoutObject()->Style());
 }
 
 void LayoutScrollbar::UpdateScrollbarParts(bool destroy) {
@@ -279,7 +280,7 @@ void LayoutScrollbar::UpdateScrollbarPart(ScrollbarPart part_type,
   LayoutScrollbarPart* part_layout_object = parts_.at(part_type);
   if (!part_layout_object && need_layout_object && scrollable_area_) {
     part_layout_object = LayoutScrollbarPart::CreateAnonymous(
-        &StyleSource()->GetDocument(), scrollable_area_, this, part_type);
+        &style_source_->GetDocument(), scrollable_area_, this, part_type);
     parts_.Set(part_type, part_layout_object);
     SetNeedsPaintInvalidation(part_type);
   } else if (part_layout_object && !need_layout_object) {

diff --git a/third_party/blink/renderer/core/layout/layout_scrollbar.h b/third_party/blink/renderer/core/layout/layout_scrollbar.h
@@ -55,6 +55,11 @@ class LayoutScrollbar final : public Scrollbar {
                                             const LayoutBox& enclosing_box,
                                             const LayoutObject& style_source);
 
+  // The Element that supplies our style information. If the scrollbar is
+  // for a document, this is either the <body> or <html> element. Otherwise, it
+  // is the element that owns our PaintLayerScrollableArea.
+  Element* StyleSource() const { return style_source_.Get(); }
+
   IntRect ButtonRect(ScrollbarPart) const;
   IntRect TrackRect(int start_length, int end_length) const;
   IntRect TrackPieceRectWithMargins(ScrollbarPart, const IntRect&) const;
@@ -94,6 +99,9 @@ class LayoutScrollbar final : public Scrollbar {
   scoped_refptr<ComputedStyle> GetScrollbarPseudoStyle(ScrollbarPart, PseudoId);
   void UpdateScrollbarPart(ScrollbarPart, bool destroy = false);
 
+  // The element that supplies our style information.
+  Member<Element> style_source_;
+
   HashMap<unsigned, LayoutScrollbarPart*> parts_;
 };
 

diff --git a/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc b/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc
@@ -2444,17 +2444,13 @@ Scrollbar* PaintLayerScrollableArea::ScrollbarManager::CreateScrollbar(
       scrollbar_size = LayoutTheme::GetTheme().ScrollbarControlSizeForPart(
           style_source.StyleRef().EffectiveAppearance());
     }
-    Element* style_source_element = nullptr;
-    if (RuntimeEnabledFeatures::FormControlsRefreshEnabled()) {
-      style_source_element = DynamicTo<Element>(style_source.GetNode());
-    }
-    scrollbar = MakeGarbageCollected<Scrollbar>(
-        ScrollableArea(), orientation, scrollbar_size, style_source_element,
-        &ScrollableArea()
-             ->GetLayoutBox()
-             ->GetFrame()
-             ->GetPage()
-             ->GetChromeClient());
+    scrollbar = MakeGarbageCollected<Scrollbar>(ScrollableArea(), orientation,
+                                                scrollbar_size,
+                                                &ScrollableArea()
+                                                     ->GetLayoutBox()
+                                                     ->GetFrame()
+                                                     ->GetPage()
+                                                     ->GetChromeClient());
   }
   ScrollableArea()->GetLayoutBox()->GetDocument().View()->AddScrollbar(
       scrollbar);

diff --git a/third_party/blink/renderer/core/scroll/scrollable_area_test.cc b/third_party/blink/renderer/core/scroll/scrollable_area_test.cc
@@ -106,9 +106,8 @@ TEST_F(ScrollableAreaTest, ScrollbarGraphicsLayerInvalidation) {
   EXPECT_CALL(*scrollable_area, LayerForHorizontalScrollbar())
       .WillRepeatedly(Return(&graphics_layer));
 
-  auto* scrollbar =
-      MakeGarbageCollected<Scrollbar>(scrollable_area, kHorizontalScrollbar,
-                                      kRegularScrollbar, nullptr, nullptr);
+  auto* scrollbar = MakeGarbageCollected<Scrollbar>(
+      scrollable_area, kHorizontalScrollbar, kRegularScrollbar, nullptr);
   graphics_layer.ResetTrackedRasterInvalidations();
   scrollbar->SetNeedsPaintInvalidation(kNoPart);
   EXPECT_TRUE(graphics_layer.HasTrackedRasterInvalidations());

diff --git a/third_party/blink/renderer/core/scroll/scrollbar.cc b/third_party/blink/renderer/core/scroll/scrollbar.cc
@@ -31,22 +31,18 @@
 #include "third_party/blink/public/platform/web_gesture_event.h"
 #include "third_party/blink/public/platform/web_mouse_event.h"
 #include "third_party/blink/public/platform/web_scrollbar_overlay_color_theme.h"
-#include "third_party/blink/renderer/core/dom/element.h"
-#include "third_party/blink/renderer/core/layout/layout_object.h"
 #include "third_party/blink/renderer/core/page/chrome_client.h"
 #include "third_party/blink/renderer/core/scroll/scroll_animator_base.h"
 #include "third_party/blink/renderer/core/scroll/scrollable_area.h"
 #include "third_party/blink/renderer/core/scroll/scrollbar_theme.h"
 #include "third_party/blink/renderer/platform/geometry/float_rect.h"
 #include "third_party/blink/renderer/platform/graphics/paint/cull_rect.h"
-#include "third_party/blink/renderer/platform/text/text_direction.h"
 
 namespace blink {
 
 Scrollbar::Scrollbar(ScrollableArea* scrollable_area,
                      ScrollbarOrientation orientation,
                      ScrollbarControlSize control_size,
-                     Element* style_source,
                      ChromeClient* chrome_client,
                      ScrollbarTheme* theme)
     : scrollable_area_(scrollable_area),
@@ -71,8 +67,7 @@ Scrollbar::Scrollbar(ScrollableArea* scrollable_area,
       elastic_overscroll_(0),
       track_needs_repaint_(true),
       thumb_needs_repaint_(true),
-      injected_gesture_scroll_begin_(false),
-      style_source_(style_source) {
+      injected_gesture_scroll_begin_(false) {
   theme_.RegisterScrollbar(*this);
 
   // FIXME: This is ugly and would not be necessary if we fix cross-platform
@@ -93,7 +88,6 @@ Scrollbar::~Scrollbar() =default;
 void Scrollbar::Trace(blink::Visitor* visitor) {
   visitor->Trace(scrollable_area_);
   visitor->Trace(chrome_client_);
-  visitor->Trace(style_source_);
 }
 
 void Scrollbar::SetFrameRect(const IntRect& frame_rect) {
@@ -790,23 +784,6 @@ CompositorElementId Scrollbar::GetElementId() {
   return scrollable_area_->GetScrollbarElementId(orientation_);
 }
 
-float Scrollbar::EffectiveZoom() const {
-  if (RuntimeEnabledFeatures::FormControlsRefreshEnabled() && style_source_ &&
-      style_source_->GetLayoutObject()) {
-    return style_source_->GetLayoutObject()->Style()->EffectiveZoom();
-  }
-  return 1.0;
-}
-
-bool Scrollbar::ContainerIsRightToLeft() const {
-  if (RuntimeEnabledFeatures::FormControlsRefreshEnabled() && style_source_ &&
-      style_source_->GetLayoutObject()) {
-    TextDirection dir = style_source_->GetLayoutObject()->Style()->Direction();
-    return IsRtl(dir);
-  }
-  return false;
-}
-
 STATIC_ASSERT_ENUM(kWebScrollbarOverlayColorThemeDark,
                    kScrollbarOverlayColorThemeDark);
 STATIC_ASSERT_ENUM(kWebScrollbarOverlayColorThemeLight,

diff --git a/third_party/blink/renderer/core/scroll/scrollbar.h b/third_party/blink/renderer/core/scroll/scrollbar.h
@@ -37,7 +37,6 @@
 namespace blink {
 
 class CullRect;
-class Element;
 class GraphicsContext;
 class IntRect;
 class ChromeClient;
@@ -56,13 +55,12 @@ class CORE_EXPORT Scrollbar : public GarbageCollectedFinalized<Scrollbar>,
                                      ScrollbarControlSize size,
                                      ScrollbarTheme* theme) {
     return MakeGarbageCollected<Scrollbar>(scrollable_area, orientation, size,
-                                           nullptr, nullptr, theme);
+                                           nullptr, theme);
   }
 
   Scrollbar(ScrollableArea*,
             ScrollbarOrientation,
             ScrollbarControlSize,
-            Element* style_source,
             ChromeClient* = nullptr,
             ScrollbarTheme* = nullptr);
   ~Scrollbar() override;
@@ -191,14 +189,6 @@ class CORE_EXPORT Scrollbar : public GarbageCollectedFinalized<Scrollbar>,
 
   CompositorElementId GetElementId();
 
-  float EffectiveZoom() const;
-  bool ContainerIsRightToLeft() const;
-
-  // The Element that supplies our style information. If the scrollbar is
-  // for a document, this is either the <body> or <html> element. Otherwise, it
-  // is the element that owns our PaintLayerScrollableArea.
-  Element* StyleSource() const { return style_source_.Get(); }
-
   virtual void Trace(blink::Visitor*);
 
  protected:
@@ -252,7 +242,6 @@ class CORE_EXPORT Scrollbar : public GarbageCollectedFinalized<Scrollbar>,
   bool injected_gesture_scroll_begin_;
   IntRect visual_rect_;
   IntRect frame_rect_;
-  Member<Element> style_source_;
 };
 
 }  // namespace blink

diff --git a/third_party/blink/renderer/core/scroll/scrollbar_theme_aura.cc b/third_party/blink/renderer/core/scroll/scrollbar_theme_aura.cc
@@ -281,13 +281,8 @@ void ScrollbarThemeAura::PaintButton(GraphicsContext& gc,
   if (!params.should_paint)
     return;
   DrawingRecorder recorder(gc, scrollbar, display_item_type);
-
-  WebThemeEngine::ExtraParams extra_params;
-  extra_params.scrollbar_button.zoom = scrollbar.EffectiveZoom();
-  extra_params.scrollbar_button.right_to_left =
-      scrollbar.ContainerIsRightToLeft();
   Platform::Current()->ThemeEngine()->Paint(
-      gc.Canvas(), params.part, params.state, WebRect(rect), &extra_params,
+      gc.Canvas(), params.part, params.state, WebRect(rect), nullptr,
       WebColorScheme::
           kLight /* TODO(futhark): pass color scheme to scrollbar parts */);
 }

diff --git a/third_party/blink/web_tests/FlagExpectations/enable-blink-features=CompositeAfterPaint b/third_party/blink/web_tests/FlagExpectations/enable-blink-features=CompositeAfterPaint
@@ -550,6 +550,3 @@ crbug.com/985388 virtual/threaded/fast/scrolling/fractional-scroll-offset-iframe
 # Added to land BGPT-fixes
 crbug.com/979002 virtual/threaded/fast/scrolling/uncomposite-and-composite-scroll.html [ Failure ]
 crbug.com/981749 virtual/threaded/fast/scrolling/scroll-chaining-from-position-fixed.html [ Failure ]
-
-# These tests are failing due to subpixel differences but can't be rebaselined.
-Bug(none) virtual/controls-refresh/fast/forms/controls-new-ui/select/select-multiple-appearance-basic.html [ Failure ]
diff --git a/...fast/forms/controls-new-ui/select/select-multiple-appearance-basic-expected.png b/...fast/forms/controls-new-ui/select/select-multiple-appearance-basic-expected.png