Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2600873002:
Memoize LayoutThemeDefault::clampedMenuListArrowPaddingSize(). (Closed)
|
Created:
3 years, 12 months ago by tkent Modified:
3 years, 11 months 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. |
DescriptionMemoize LayoutThemeDefault::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
Review-Url: https://codereview.chromium.org/2600873002
Cr-Commit-Position: refs/heads/master@{#442840}
Committed: https://chromium.googlesource.com/chromium/src/+/46ad8afa30fb005f534fc13e40acbe1e0f0f4787
Patch Set 1 #Patch Set 2 : Update LayoutThemeDefault::didChangeThemeEngine #
Messages
Total messages: 29 (20 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?
Description was changed from ========== Memoize LayoutTheemDefault::clampedMenuListArrowPaddingSize(). This is the second try to fix a performance regression of menulist rendering. The first try [1] didn't have a major improvement. This CL reverts it, and cache another value. [1] crrev.com/438496 BUG=673754 ========== to ========== 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 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM
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": 1, "attempt_start_ts": 1483579939960950, "parent_rev":
"fc641ed78583c56cb0f0b1f5f4fdd945acaf4731", "commit_rev":
"05dec32bd6042f0fbcd2c6e5f1baca2be619c3ef"}
Message was sent while issue was closed.
Description was changed from ========== 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 ========== to ========== 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 Review-Url: https://codereview.chromium.org/2600873002 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== 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 Review-Url: https://codereview.chromium.org/2600873002 ========== to ========== 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} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/b1f1c5d7ee4bfe822c4c0a384a9ac2ddaa71554f Cr-Commit-Position: refs/heads/master@{#441569}
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in https://codereview.chromium.org/2612113003/ by rdevlin.cronin@chromium.org. The reason for reverting is: This appears to be causing the layout test failures in crbug.com/678684. (Ran a local bisect to find the culprit.).
Message was sent while issue was closed.
On 2017/01/05 at 19:53:47, rdevlin.cronin wrote: > A revert of this CL (patchset #1 id:1) has been created in https://codereview.chromium.org/2612113003/ by rdevlin.cronin@chromium.org. > > The reason for reverting is: This appears to be causing the layout test failures in crbug.com/678684. (Ran a local bisect to find the culprit.). I found crrev.com/438496 (not this CL) had a bug that LayoutThemeDefault kept a wrong cached value and it made layout tests order-dependent. https://codereview.chromium.org/2627773002 will address the bug.
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...
Description was changed from ========== 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} ========== to ========== 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 ==========
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
The patchset sent to the CQ was uploaded after l-g-t-m from keishi@chromium.org Link to the patchset: https://codereview.chromium.org/2600873002/#ps20001 (title: "Update LayoutThemeDefault::didChangeThemeEngine")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== 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 ========== to ========== Memoize LayoutThemeDefault::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 ==========
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484122463034040,
"parent_rev": "7686b278333dad35c85b95eb4a7826112d64caff", "commit_rev":
"46ad8afa30fb005f534fc13e40acbe1e0f0f4787"}
Message was sent while issue was closed.
Description was changed from ========== Memoize LayoutThemeDefault::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 ========== to ========== Memoize LayoutThemeDefault::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 Review-Url: https://codereview.chromium.org/2600873002 Cr-Commit-Position: refs/heads/master@{#442840} Committed: https://chromium.googlesource.com/chromium/src/+/46ad8afa30fb005f534fc13e40ac... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/46ad8afa30fb005f534fc13e40ac... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
