Skip to content

Commit

Permalink
Fix scrollbar buttons at hidpi when enable-use-zoom-for-dsf is on.
Browse files Browse the repository at this point in the history
Previously on Windows the scrollbar buttons were always sized at 1x
and would leave ugly black space between them at the edge of the
window.

BUG=612874
TEST=Scrollbar buttons on Windows should look right at 1.5x and 2x
device scales with the #enable-use-zoom-for-dsf flag on.

Review-Url: https://codereview.chromium.org/1911973002
Cr-Commit-Position: refs/heads/master@{#394893}
  • Loading branch information
bsep-chromium authored and Commit bot committed May 19, 2016
1 parent 3ad270d commit 6e7c308
Show file tree
Hide file tree
Showing 18 changed files with 815 additions and 195 deletions.
53 changes: 1 addition & 52 deletions content/child/webthemeengine_impl_default.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,6 @@ using blink::WebRect;
using blink::WebThemeEngine;

namespace content {
namespace {

#if defined(OS_WIN)
// The scrollbar metrics default to 17 dips which is the default value on
// Windows in most cases.
int32_t g_vertical_scroll_bar_width = 17;

// The height of a horizontal scroll bar in dips.
int32_t g_horizontal_scroll_bar_height = 17;

// The height of the arrow bitmap on a vertical scroll bar in dips.
int32_t g_vertical_arrow_bitmap_height = 17;

// The width of the arrow bitmap on a horizontal scroll bar in dips.
int32_t g_horizontal_arrow_bitmap_width = 17;
#endif

} // namespace

static ui::NativeTheme::Part NativeThemePart(
WebThemeEngine::Part part) {
Expand Down Expand Up @@ -185,27 +167,8 @@ static void GetNativeThemeExtraParams(

blink::WebSize WebThemeEngineImpl::getSize(WebThemeEngine::Part part) {
ui::NativeTheme::ExtraParams extra;
ui::NativeTheme::Part native_theme_part = NativeThemePart(part);
#if defined(OS_WIN)
switch (native_theme_part) {
case ui::NativeTheme::kScrollbarDownArrow:
case ui::NativeTheme::kScrollbarLeftArrow:
case ui::NativeTheme::kScrollbarRightArrow:
case ui::NativeTheme::kScrollbarUpArrow:
case ui::NativeTheme::kScrollbarHorizontalThumb:
case ui::NativeTheme::kScrollbarVerticalThumb:
case ui::NativeTheme::kScrollbarHorizontalTrack:
case ui::NativeTheme::kScrollbarVerticalTrack: {
return gfx::Size(g_vertical_scroll_bar_width,
g_vertical_scroll_bar_width);
}

default:
break;
}
#endif
return ui::NativeTheme::GetInstanceForWeb()->GetPartSize(
native_theme_part, ui::NativeTheme::kNormal, extra);
NativeThemePart(part), ui::NativeTheme::kNormal, extra);
}

void WebThemeEngineImpl::paint(
Expand Down Expand Up @@ -233,18 +196,4 @@ void WebThemeEngineImpl::paintStateTransition(blink::WebCanvas* canvas,
NativeThemeState(endState), progress, gfx::Rect(rect));
}

#if defined(OS_WIN)
// static
void WebThemeEngineImpl::cacheScrollBarMetrics(
int32_t vertical_scroll_bar_width,
int32_t horizontal_scroll_bar_height,
int32_t vertical_arrow_bitmap_height,
int32_t horizontal_arrow_bitmap_width) {
g_vertical_scroll_bar_width = vertical_scroll_bar_width;
g_horizontal_scroll_bar_height = horizontal_scroll_bar_height;
g_vertical_arrow_bitmap_height = vertical_arrow_bitmap_height;
g_horizontal_arrow_bitmap_width = horizontal_arrow_bitmap_width;
}
#endif

} // namespace content
8 changes: 0 additions & 8 deletions content/child/webthemeengine_impl_default.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,6 @@ class WebThemeEngineImpl : public blink::WebThemeEngine {
blink::WebThemeEngine::State endState,
double progress,
const blink::WebRect& rect);

#if defined(OS_WIN)
// Caches the scrollbar metrics.
static void cacheScrollBarMetrics(int32_t vertical_scroll_bar_width,
int32_t horizontal_scroll_bar_height,
int32_t vertical_arrow_bitmap_height,
int32_t horizontal_arrow_bitmap_width);
#endif
};

} // namespace content
Expand Down
1 change: 0 additions & 1 deletion content/renderer/render_view_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2586,7 +2586,6 @@ void RenderViewImpl::OnSetRendererPrefs(
renderer_preferences_ = renderer_prefs;

UpdateFontRenderingFromRendererPrefs();
UpdateThemePrefs();

#if defined(USE_DEFAULT_RENDER_THEME)
if (renderer_prefs.use_custom_colors) {
Expand Down
7 changes: 0 additions & 7 deletions content/renderer/render_view_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -738,13 +738,6 @@ class CONTENT_EXPORT RenderViewImpl
navigation_gesture_ = gesture;
}

// Platform specific theme preferences if any are updated here.
#if defined(OS_WIN)
void UpdateThemePrefs();
#else
void UpdateThemePrefs() {}
#endif

void UpdateWebViewWithDeviceScaleFactor();

// ---------------------------------------------------------------------------
Expand Down
8 changes: 0 additions & 8 deletions content/renderer/render_view_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,4 @@ void RenderViewImpl::UpdateFontRenderingFromRendererPrefs() {
!= gfx::FontRenderParams::SUBPIXEL_RENDERING_NONE);
}

void RenderViewImpl::UpdateThemePrefs() {
WebThemeEngineImpl::cacheScrollBarMetrics(
renderer_preferences_.vertical_scroll_bar_width_in_dips,
renderer_preferences_.horizontal_scroll_bar_height_in_dips,
renderer_preferences_.arrow_bitmap_height_vertical_scroll_bar_in_dips,
renderer_preferences_.arrow_bitmap_width_horizontal_scroll_bar_in_dips);
}

} // namespace content
501 changes: 501 additions & 0 deletions third_party/WebKit/LayoutTests/TestExpectations

Large diffs are not rendered by default.

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 4 additions & 0 deletions third_party/WebKit/Source/platform/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,10 @@ test("blink_platform_unittests") {
sources += [ "scroll/ScrollAnimatorTest.cpp" ]
}

if (use_default_render_theme) {
sources += [ "scroll/ScrollbarThemeAuraTest.cpp" ]
}

sources += [ "testing/RunAllTests.cpp" ]

configs += [
Expand Down
8 changes: 8 additions & 0 deletions third_party/WebKit/Source/platform/blink_platform.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -1261,6 +1261,7 @@
'graphics/test/MockImageDecoder.h',
],
'platform_test_support_files': [
'scroll/ScrollbarTestSuite.h',
'testing/CompositorTest.cpp',
'testing/CompositorTest.h',
'testing/FakeDisplayItemClient.h',
Expand Down Expand Up @@ -1324,6 +1325,13 @@
],
}
],
['use_default_render_theme==1',
{
'platform_test_files': [
'scroll/ScrollbarThemeAuraTest.cpp',
],
}
],
],
},
}
82 changes: 5 additions & 77 deletions third_party/WebKit/Source/platform/scroll/ScrollableAreaTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include "platform/graphics/Color.h"
#include "platform/graphics/GraphicsLayer.h"
#include "platform/scroll/ScrollbarTestSuite.h"
#include "platform/scroll/ScrollbarTheme.h"
#include "platform/scroll/ScrollbarThemeMock.h"
#include "platform/testing/FakeGraphicsLayer.h"
Expand All @@ -22,78 +23,15 @@ namespace {
using testing::_;
using testing::Return;

class MockScrollableArea : public GarbageCollectedFinalized<MockScrollableArea>, public ScrollableArea {
USING_GARBAGE_COLLECTED_MIXIN(MockScrollableArea);
class ScrollbarThemeWithMockInvalidation : public ScrollbarThemeMock {
public:
static MockScrollableArea* create(const IntPoint& maximumScrollPosition)
{
return new MockScrollableArea(maximumScrollPosition);
}

MOCK_CONST_METHOD0(visualRectForScrollbarParts, LayoutRect());
MOCK_CONST_METHOD0(isActive, bool());
MOCK_CONST_METHOD1(scrollSize, int(ScrollbarOrientation));
MOCK_CONST_METHOD0(isScrollCornerVisible, bool());
MOCK_CONST_METHOD0(scrollCornerRect, IntRect());
MOCK_CONST_METHOD0(horizontalScrollbar, Scrollbar*());
MOCK_CONST_METHOD0(verticalScrollbar, Scrollbar*());
MOCK_METHOD0(scrollControlWasSetNeedsPaintInvalidation, void());
MOCK_CONST_METHOD0(enclosingScrollableArea, ScrollableArea*());
MOCK_CONST_METHOD1(visibleContentRect, IntRect(IncludeScrollbarsInRect));
MOCK_CONST_METHOD0(contentsSize, IntSize());
MOCK_CONST_METHOD0(scrollableAreaBoundingBox, IntRect());
MOCK_CONST_METHOD0(layerForHorizontalScrollbar, GraphicsLayer*());
MOCK_CONST_METHOD0(layerForVerticalScrollbar, GraphicsLayer*());

bool userInputScrollable(ScrollbarOrientation) const override { return true; }
bool scrollbarsCanBeActive () const override { return true; }
bool shouldPlaceVerticalScrollbarOnLeft() const override { return false; }
void setScrollOffset(const DoublePoint& offset, ScrollType) override { m_scrollPosition = flooredIntPoint(offset).shrunkTo(m_maximumScrollPosition); }
IntPoint scrollPosition() const override { return m_scrollPosition; }
IntPoint minimumScrollPosition() const override { return IntPoint(); }
IntPoint maximumScrollPosition() const override { return m_maximumScrollPosition; }
int visibleHeight() const override { return 768; }
int visibleWidth() const override { return 1024; }
bool scrollAnimatorEnabled() const override { return false; }
int pageStep(ScrollbarOrientation) const override { return 0; }

using ScrollableArea::horizontalScrollbarNeedsPaintInvalidation;
using ScrollableArea::verticalScrollbarNeedsPaintInvalidation;
using ScrollableArea::clearNeedsPaintInvalidationForScrollControls;

DEFINE_INLINE_VIRTUAL_TRACE()
{
ScrollableArea::trace(visitor);
}

private:
explicit MockScrollableArea(const IntPoint& maximumScrollPosition)
: m_maximumScrollPosition(maximumScrollPosition) { }

IntPoint m_scrollPosition;
IntPoint m_maximumScrollPosition;
MOCK_CONST_METHOD0(shouldRepaintAllPartsOnInvalidation, bool());
MOCK_CONST_METHOD3(invalidateOnThumbPositionChange, ScrollbarPart(const ScrollbarThemeClient&, float, float));
};

} // namespace

class ScrollableAreaTest : public testing::Test {
public:
ScrollableAreaTest() { }

void SetUp() override
{
TestingPlatformSupport::Config config;
config.compositorSupport = Platform::current()->compositorSupport();
m_fakePlatform = adoptPtr(new TestingPlatformSupportWithMockScheduler(config));
}

void TearDown() override
{
m_fakePlatform = nullptr;
}

private:
OwnPtr<TestingPlatformSupportWithMockScheduler> m_fakePlatform;
class ScrollableAreaTest : public ScrollbarTestSuite {
};

TEST_F(ScrollableAreaTest, ScrollAnimatorCurrentPositionShouldBeSync)
Expand All @@ -103,16 +41,6 @@ TEST_F(ScrollableAreaTest, ScrollAnimatorCurrentPositionShouldBeSync)
EXPECT_EQ(100.0, scrollableArea->scrollAnimator().currentPosition().y());
}

namespace {

class ScrollbarThemeWithMockInvalidation : public ScrollbarThemeMock {
public:
MOCK_CONST_METHOD0(shouldRepaintAllPartsOnInvalidation, bool());
MOCK_CONST_METHOD3(invalidateOnThumbPositionChange, ScrollbarPart(const ScrollbarThemeClient&, float, float));
};

} // namespace

TEST_F(ScrollableAreaTest, ScrollbarTrackAndThumbRepaint)
{
ScrollbarThemeWithMockInvalidation theme;
Expand Down
106 changes: 106 additions & 0 deletions third_party/WebKit/Source/platform/scroll/ScrollbarTestSuite.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
// Copyright 2016 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 ScrollbarTestSuite_h
#define ScrollbarTestSuite_h

#include "platform/heap/GarbageCollected.h"
#include "platform/scroll/ScrollableArea.h"
#include "platform/scroll/Scrollbar.h"
#include "platform/scroll/ScrollbarThemeMock.h"
#include "platform/testing/TestingPlatformSupport.h"
#include "testing/gmock/include/gmock/gmock.h"

namespace blink {

class MockScrollableArea : public GarbageCollectedFinalized<MockScrollableArea>, public ScrollableArea {
USING_GARBAGE_COLLECTED_MIXIN(MockScrollableArea);

public:
static MockScrollableArea* create()
{
return new MockScrollableArea();
}

static MockScrollableArea* create(const IntPoint& maximumScrollPosition)
{
MockScrollableArea* mock = create();
mock->setMaximumScrollPosition(maximumScrollPosition);
return mock;
}

MOCK_CONST_METHOD0(visualRectForScrollbarParts, LayoutRect());
MOCK_CONST_METHOD0(isActive, bool());
MOCK_CONST_METHOD1(scrollSize, int(ScrollbarOrientation));
MOCK_CONST_METHOD0(isScrollCornerVisible, bool());
MOCK_CONST_METHOD0(scrollCornerRect, IntRect());
MOCK_CONST_METHOD0(horizontalScrollbar, Scrollbar*());
MOCK_CONST_METHOD0(verticalScrollbar, Scrollbar*());
MOCK_CONST_METHOD0(enclosingScrollableArea, ScrollableArea*());
MOCK_CONST_METHOD1(visibleContentRect, IntRect(IncludeScrollbarsInRect));
MOCK_CONST_METHOD0(contentsSize, IntSize());
MOCK_CONST_METHOD0(scrollableAreaBoundingBox, IntRect());
MOCK_CONST_METHOD0(layerForHorizontalScrollbar, GraphicsLayer*());
MOCK_CONST_METHOD0(layerForVerticalScrollbar, GraphicsLayer*());

bool userInputScrollable(ScrollbarOrientation) const override { return true; }
bool scrollbarsCanBeActive() const override { return true; }
bool shouldPlaceVerticalScrollbarOnLeft() const override { return false; }
void setScrollOffset(const DoublePoint& offset, ScrollType) override { m_scrollPosition = flooredIntPoint(offset).shrunkTo(m_maximumScrollPosition); }
IntPoint scrollPosition() const override { return m_scrollPosition; }
IntPoint minimumScrollPosition() const override { return IntPoint(); }
IntPoint maximumScrollPosition() const override { return m_maximumScrollPosition; }
int visibleHeight() const override { return 768; }
int visibleWidth() const override { return 1024; }
bool scrollAnimatorEnabled() const override { return false; }
int pageStep(ScrollbarOrientation) const override { return 0; }
void scrollControlWasSetNeedsPaintInvalidation() {}

using ScrollableArea::horizontalScrollbarNeedsPaintInvalidation;
using ScrollableArea::verticalScrollbarNeedsPaintInvalidation;
using ScrollableArea::clearNeedsPaintInvalidationForScrollControls;

DEFINE_INLINE_VIRTUAL_TRACE()
{
ScrollableArea::trace(visitor);
}

private:
void setMaximumScrollPosition(const IntPoint& maximumScrollPosition)
{
m_maximumScrollPosition = maximumScrollPosition;
}

explicit MockScrollableArea()
: m_maximumScrollPosition(IntPoint(0, 100))
{
}

IntPoint m_scrollPosition;
IntPoint m_maximumScrollPosition;
};

class ScrollbarTestSuite : public testing::Test {
public:
ScrollbarTestSuite() {}

void SetUp() override
{
TestingPlatformSupport::Config config;
config.compositorSupport = Platform::current()->compositorSupport();
m_fakePlatform = adoptPtr(new TestingPlatformSupportWithMockScheduler(config));
}

void TearDown() override
{
m_fakePlatform = nullptr;
}

private:
OwnPtr<TestingPlatformSupportWithMockScheduler> m_fakePlatform;
};

} // namespace blink

#endif
Original file line number Diff line number Diff line change
Expand Up @@ -326,13 +326,13 @@ ScrollbarPart ScrollbarThemeAura::invalidateOnThumbPositionChange(const Scrollba
IntSize ScrollbarThemeAura::buttonSize(const ScrollbarThemeClient& scrollbar)
{
if (scrollbar.orientation() == VerticalScrollbar) {
IntSize size = Platform::current()->themeEngine()->getSize(WebThemeEngine::PartScrollbarUpArrow);
return IntSize(size.width(), scrollbar.height() < 2 * size.height() ? scrollbar.height() / 2 : size.height());
int squareSize = scrollbar.width();
return IntSize(squareSize, scrollbar.height() < 2 * squareSize ? scrollbar.height() / 2 : squareSize);
}

// HorizontalScrollbar
IntSize size = Platform::current()->themeEngine()->getSize(WebThemeEngine::PartScrollbarLeftArrow);
return IntSize(scrollbar.width() < 2 * size.width() ? scrollbar.width() / 2 : size.width(), size.height());
int squareSize = scrollbar.height();
return IntSize(scrollbar.width() < 2 * squareSize ? scrollbar.width() / 2 : squareSize, squareSize);
}

} // namespace blink
Loading

0 comments on commit 6e7c308

Please sign in to comment.