Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(184)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2657253003: Make ScrollPaintPropertyNode::toString() 26% more concise (Closed)

Created:
3 years, 10 months ago by pdr.
Modified:
3 years, 10 months ago
CC:
ajuma+watch_chromium.org, blink-reviews, blink-reviews-platform-graphics_chromium.org, Rik, chromium-reviews, danakj+watch_chromium.org, dshwang, drott+blinkwatch_chromium.org, krit, f(malita), jbroman, Justin Novosad, kinuko+watch, pdr+graphicswatchlist_chromium.org, rwlbuis, Stephen Chennney
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Make ScrollPaintPropertyNode::toString() 26% more concise Before: root 0x119cf8954118 parent=0x0 clip=0x0 bounds=0x0 userScrollableHorizontal=no userScrollableVertical=no mainThreadScrollingReasons= Scroll (FrameView) 0x119cf89540c0 parent=0x119cf8954118 clip=785x276 bounds=785x699 userScrollableHorizontal=yes userScrollableVertical=yes mainThreadScrollingReasons= After: root 0x36350c954118 parent=0x0 clip=0x0 bounds=0x0 userScrollable=none mainThreadReasons=none Scroll (FrameView) 0x36350c9540c0 parent=0x36350c954118 clip=785x276 bounds=785x699 userScrollable=both mainThreadReasons=none BUG=683774 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2657253003 Cr-Commit-Position: refs/heads/master@{#446932} Committed: https://chromium.googlesource.com/chromium/src/+/46cf3df5c1b750d2bade33909005304d5a4234a9

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+25 lines, -12 lines) Patch
M third_party/WebKit/Source/platform/graphics/paint/ScrollPaintPropertyNode.cpp View 1 chunk +25 lines, -12 lines 1 comment Download

Messages

Total messages: 18 (12 generated)
pdr.
3 years, 10 months ago (2017-01-27 23:24:19 UTC) #4
chrishtr
lgtm
3 years, 10 months ago (2017-01-27 23:27:06 UTC) #8
wkorman
lgtm
3 years, 10 months ago (2017-01-28 18:20:46 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2657253003/1
3 years, 10 months ago (2017-01-29 00:16:35 UTC) #13
commit-bot: I haz the power
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/46cf3df5c1b750d2bade33909005304d5a4234a9
3 years, 10 months ago (2017-01-29 03:03:27 UTC) #16
brucedawson
3 years, 10 months ago (2017-01-31 20:21:28 UTC) #18
Message was sent while issue was closed. 
https://codereview.chromium.org/2657253003/diff/1/third_party/WebKit/Source/p...
File
third_party/WebKit/Source/platform/graphics/paint/ScrollPaintPropertyNode.cpp
(right):

https://codereview.chromium.org/2657253003/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/paint/ScrollPaintPropertyNode.cpp:30:
else if (!m_userScrollableHorizontal && !m_userScrollableHorizontal)
The experimental /analyze builder pointed out that this code is probably
incorrect. It said:

third_party\webkit\source\platform\graphics\paint\scrollpaintpropertynode.cpp(30)
: warning C6287: Redundant code:  the left and right sub-expressions are
identical.

And indeed, it looks like the check should have been:

if (!m_userScrollableHorizontal && !m_userScrollableVertical)

I've created CL https://codereview.chromium.org/2664253003 to address this.

Powered by Google App Engine
This is Rietveld 408576698