Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2576563003:
Cache scrollbar thickness in LayoutThemeDefault. (Closed)
|
Created:
4 years ago by tkent Modified:
4 years ago Reviewers:
keishi CC:
blink-reviews, blink-reviews-layout_chromium.org, chromium-reviews, eae+blinkwatch, jchaffraix+rendering, leviw+renderwatch, pdr+renderingwatchlist_chromium.org, szager+layoutwatch_chromium.org, zoltan1 Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionCache scrollbar thickness in LayoutThemeDefault.
This is a speculative fix for a performance regression on Android.
BUG=673754
Committed: https://crrev.com/7a5b75cdc26787eb9d5e2144055e93b59d7d2837
Cr-Commit-Position: refs/heads/master@{#438496}
Patch Set 1 : _ #
Total comments: 2
Messages
Total messages: 20 (14 generated)
The CQ bit was checked by tkent@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
tkent@chromium.org changed reviewers: + keishi@chromium.org
Keishi@, would you review this please?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by tkent@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #1 (id:1) has been deleted
LGTM https://codereview.chromium.org/2576563003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutThemeDefault.cpp (right): https://codereview.chromium.org/2576563003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutThemeDefault.cpp:339: if (m_scrollbarThicknessInDIP > 0) nit: Maybe initial value should be a negative value in case zero width overlay scrollbar becomes the default? https://plus.google.com/+FrancoisBeaufort/posts/XTjX1GGVpjZ?sfc=true
https://codereview.chromium.org/2576563003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutThemeDefault.cpp (right): https://codereview.chromium.org/2576563003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutThemeDefault.cpp:339: if (m_scrollbarThicknessInDIP > 0) On 2016/12/14 at 08:23:08, keishi wrote: > nit: Maybe initial value should be a negative value in case zero width overlay scrollbar becomes the default? > https://plus.google.com/+FrancoisBeaufort/posts/XTjX1GGVpjZ?sfc=true AFAIK, getSize(WebThemeEngine::PartScrollbarUpArrow) won't be changed by overlay scrollbar. blink::ScrollbarTheme is sensitive to overlay scrollbar, but WebThemeEngine is in a lower layer than ScrollbatTheme. I'll add more comments by a follow-up CL.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by tkent@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1481720513162470,
"parent_rev": "990025d005a9840a771cd25b18a11960c6256124", "commit_rev":
"ad31abc6443086f2d2aab8d49a05591f2a2db5a9"}
Message was sent while issue was closed.
Description was changed from ========== Cache scrollbar thickness in LayoutThemeDefault. This is a speculative fix for a performance regression on Android. BUG=673754 ========== to ========== Cache scrollbar thickness in LayoutThemeDefault. This is a speculative fix for a performance regression on Android. BUG=673754 Review-Url: https://codereview.chromium.org/2576563003 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Cache scrollbar thickness in LayoutThemeDefault. This is a speculative fix for a performance regression on Android. BUG=673754 Review-Url: https://codereview.chromium.org/2576563003 ========== to ========== Cache scrollbar thickness in LayoutThemeDefault. This is a speculative fix for a performance regression on Android. BUG=673754 Committed: https://crrev.com/7a5b75cdc26787eb9d5e2144055e93b59d7d2837 Cr-Commit-Position: refs/heads/master@{#438496} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/7a5b75cdc26787eb9d5e2144055e93b59d7d2837 Cr-Commit-Position: refs/heads/master@{#438496} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
