Commit 912bfe0a authored by rdevlin.cronin's avatar rdevlin.cronin Committed by Commit bot

Revert of Memoize LayoutTheemDefault::clampedMenuListArrowPaddingSize()....

Revert of Memoize LayoutTheemDefault::clampedMenuListArrowPaddingSize(). (patchset #1 id:1 of https://codereview.chromium.org/2600873002/ )

Reason for revert:
This appears to be causing the layout test failures in crbug.com/678684.  (Ran a local bisect to find the culprit.)

Original issue's description:
> Memoize LayoutTheemDefault::clampedMenuListArrowPaddingSize().
>
> This is the second try to fix a performance regression of menulist rendering.
> The first try [1] didn't have major improvement. This CL reverts it, and cache
> another value.
>
> [1] crrev.com/438496
>
> BUG=673754
>
> Committed: https://crrev.com/b1f1c5d7ee4bfe822c4c0a384a9ac2ddaa71554f
> Cr-Commit-Position: refs/heads/master@{#441569}

TBR=keishi@chromium.org,tkent@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=673754

Review-Url: https://codereview.chromium.org/2612113003
Cr-Commit-Position: refs/heads/master@{#441739}
parent bfeeddcd
......@@ -336,36 +336,31 @@ int LayoutThemeDefault::popupInternalPaddingBottom(
}
int LayoutThemeDefault::menuListArrowWidthInDIP() const {
if (m_menuListArrowWidthInDIP > 0)
return m_menuListArrowWidthInDIP;
int width = Platform::current()
->themeEngine()
->getSize(WebThemeEngine::PartScrollbarUpArrow)
.width;
return width > 0 ? width : 15;
const_cast<LayoutThemeDefault*>(this)->m_menuListArrowWidthInDIP =
width > 0 ? width : 15;
return m_menuListArrowWidthInDIP;
}
float LayoutThemeDefault::clampedMenuListArrowPaddingSize(
const HostWindow* host,
const ComputedStyle& style) const {
if (m_cachedMenuListArrowPaddingSize > 0 &&
style.effectiveZoom() == m_cachedMenuListArrowZoomLevel)
return m_cachedMenuListArrowPaddingSize;
m_cachedMenuListArrowZoomLevel = style.effectiveZoom();
int originalSize = menuListArrowWidthInDIP();
int scaledSize =
host ? host->windowToViewportScalar(originalSize) : originalSize;
// The result should not be samller than the scrollbar thickness in order to
// secure space for scrollbar in popup.
float deviceScale = 1.0f * scaledSize / originalSize;
float size;
if (m_cachedMenuListArrowZoomLevel < deviceScale) {
size = scaledSize;
} else {
// The value should be zoomed though scrollbars aren't scaled by zoom.
// crbug.com/432795.
size = originalSize * m_cachedMenuListArrowZoomLevel;
}
m_cachedMenuListArrowPaddingSize = size;
return size;
if (style.effectiveZoom() < deviceScale)
return scaledSize;
// The value should be zoomed though scrollbars aren't scaled by zoom.
// crbug.com/432795.
return originalSize * style.effectiveZoom();
}
// static
......
......@@ -159,9 +159,7 @@ class CORE_EXPORT LayoutThemeDefault : public LayoutTheme {
static unsigned m_inactiveSelectionForegroundColor;
ThemePainterDefault m_painter;
// Cached values for crbug.com/673754.
mutable float m_cachedMenuListArrowZoomLevel = 0;
mutable float m_cachedMenuListArrowPaddingSize = 0;
int m_menuListArrowWidthInDIP = 0;
};
} // namespace blink
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment