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

Issue 2594673002: Fix the performance issuse called by last CL. (Closed)

Created:
4 years ago by cathiechentx
Modified:
4 years ago
CC:
chromium-reviews, szager+layoutwatch_chromium.org, zoltan1, blink-reviews-layout_chromium.org, pdr+renderingwatchlist_chromium.org, eae+blinkwatch, leviw+renderwatch, jchaffraix+rendering, blink-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix the performance issuse called by last CL. Adding superclusters into m_potentiallyInconsistentSuperclusters too frequently caused this issuse. Try to reduce unnecessary adding in these two ways: 1. skip everHadLayout blocks in record. 2. skip UnknownAmountOfText superclusters BUG=675534 Committed: https://crrev.com/340893b524c359dbd40b7ad656e9a6c8240724e4 Cr-Commit-Position: refs/heads/master@{#440026}

Patch Set 1 #

Total comments: 4

Patch Set 2 : add parameter name #

Unified diffs Side-by-side diffs Delta from patch set Stats (+17 lines, -12 lines) Patch
M third_party/WebKit/Source/core/layout/TextAutosizer.h View 1 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/TextAutosizer.cpp View 6 chunks +16 lines, -11 lines 0 comments Download

Messages

Total messages: 14 (7 generated)
cathiechentx
Fix the performance issuse called by inconsistent CL.
4 years ago (2016-12-20 13:00:15 UTC) #3
cathiechentx
https://codereview.chromium.org/2594673002/diff/1/third_party/WebKit/Source/core/layout/TextAutosizer.cpp File third_party/WebKit/Source/core/layout/TextAutosizer.cpp (right): https://codereview.chromium.org/2594673002/diff/1/third_party/WebKit/Source/core/layout/TextAutosizer.cpp#newcode334 third_party/WebKit/Source/core/layout/TextAutosizer.cpp:334: if (!block->everHadLayout()) No need to deal with block if ...
4 years ago (2016-12-20 13:07:26 UTC) #4
skobes
lgtm with nit https://codereview.chromium.org/2594673002/diff/1/third_party/WebKit/Source/core/layout/TextAutosizer.h File third_party/WebKit/Source/core/layout/TextAutosizer.h (right): https://codereview.chromium.org/2594673002/diff/1/third_party/WebKit/Source/core/layout/TextAutosizer.h#newcode237 third_party/WebKit/Source/core/layout/TextAutosizer.h:237: Supercluster* createSuperclusterIfNeeded(LayoutBlock*, bool&); include the parameter ...
4 years ago (2016-12-20 23:44:23 UTC) #5
cathiechentx
https://codereview.chromium.org/2594673002/diff/1/third_party/WebKit/Source/core/layout/TextAutosizer.h File third_party/WebKit/Source/core/layout/TextAutosizer.h (right): https://codereview.chromium.org/2594673002/diff/1/third_party/WebKit/Source/core/layout/TextAutosizer.h#newcode237 third_party/WebKit/Source/core/layout/TextAutosizer.h:237: Supercluster* createSuperclusterIfNeeded(LayoutBlock*, bool&); On 2016/12/20 23:44:23, skobes wrote: > ...
4 years ago (2016-12-21 02:56:43 UTC) #6
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/2594673002/20001
4 years ago (2016-12-21 02:57:29 UTC) #9
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years ago (2016-12-21 05:16:32 UTC) #12
commit-bot: I haz the power
4 years ago (2016-12-21 05:18:53 UTC) #14
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/340893b524c359dbd40b7ad656e9a6c8240724e4
Cr-Commit-Position: refs/heads/master@{#440026}

Powered by Google App Engine
This is Rietveld 408576698