Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2855873003:
Split up the if checks in ComputedStyle to make them easy to generate (Closed)
DescriptionSplit up the if checks in ComputedStyle to make them easy to generate
This patch splits up the iff checks in the diffing function
ComputedStyle::UpdatePropertySpecificDifferences with the intention of
making it easy to generate with the macro fieldwise_diff created here:
https://codereview.chromium.org/2858863002
As can be seen in the try bot runs this has no perf impact
BUG=710938
Review-Url: https://codereview.chromium.org/2855873003
Cr-Commit-Position: refs/heads/master@{#469268}
Committed: https://chromium.googlesource.com/chromium/src/+/a26e3d4f16764fda0ed56f200af4d7fd17109ffe
Patch Set 1 #
Total comments: 2
Patch Set 2 : shend@ suggestions #Messages
Total messages: 23 (16 generated)
The CQ bit was checked by nainar@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Run perf tests of splitting up if statement checks BUG= ========== to ========== Split up the if checks in ComputedStyle to make them easy to generate This patch splits up the iff checks in the diffing function ComputedStyle::UpdatePropertySpecificDifferences with the intention of making it easy to generate with the macro fieldwise_diff created here: https://codereview.chromium.org/2858863002 BUG=710938 ==========
nainar@chromium.org changed reviewers: + shend@chromium.org
shend@ PTAL? Thanks! Perf results are part of the try bot run.
Description was changed from ========== Split up the if checks in ComputedStyle to make them easy to generate This patch splits up the iff checks in the diffing function ComputedStyle::UpdatePropertySpecificDifferences with the intention of making it easy to generate with the macro fieldwise_diff created here: https://codereview.chromium.org/2858863002 BUG=710938 ========== to ========== Split up the if checks in ComputedStyle to make them easy to generate This patch splits up the iff checks in the diffing function ComputedStyle::UpdatePropertySpecificDifferences with the intention of making it easy to generate with the macro fieldwise_diff created here: https://codereview.chromium.org/2858863002 As can be seen in the try bot runs this has no perf impact BUG=710938 ==========
https://codereview.chromium.org/2855873003/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right): https://codereview.chromium.org/2855873003/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/style/ComputedStyle.cpp:1080: if (rare_inherited_data_.Get() != other.rare_inherited_data_.Get() && Not exactly the same behaviour as the original code. I think this should be another else, with an if inside of it.
lgtm after comments
The CQ bit was checked by nainar@chromium.org to run a CQ dry run
nainar@chromium.org changed reviewers: + meade@chromium.org
shend@ made the changes you asked for. meade@ for OWNERS https://codereview.chromium.org/2855873003/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right): https://codereview.chromium.org/2855873003/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/style/ComputedStyle.cpp:1080: if (rare_inherited_data_.Get() != other.rare_inherited_data_.Get() && Yup. You're right. Wonder why this passes tests though. Low test coverage? Done regardless
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
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 nainar@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from shend@chromium.org Link to the patchset: https://codereview.chromium.org/2855873003/#ps20001 (title: "shend@ suggestions")
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": 1493873868978290,
"parent_rev": "d46b48bf70df88eeced7ed8d4ea6463bc7920601", "commit_rev":
"a26e3d4f16764fda0ed56f200af4d7fd17109ffe"}
Message was sent while issue was closed.
Description was changed from ========== Split up the if checks in ComputedStyle to make them easy to generate This patch splits up the iff checks in the diffing function ComputedStyle::UpdatePropertySpecificDifferences with the intention of making it easy to generate with the macro fieldwise_diff created here: https://codereview.chromium.org/2858863002 As can be seen in the try bot runs this has no perf impact BUG=710938 ========== to ========== Split up the if checks in ComputedStyle to make them easy to generate This patch splits up the iff checks in the diffing function ComputedStyle::UpdatePropertySpecificDifferences with the intention of making it easy to generate with the macro fieldwise_diff created here: https://codereview.chromium.org/2858863002 As can be seen in the try bot runs this has no perf impact BUG=710938 Review-Url: https://codereview.chromium.org/2855873003 Cr-Commit-Position: refs/heads/master@{#469268} Committed: https://chromium.googlesource.com/chromium/src/+/a26e3d4f16764fda0ed56f200af4... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/a26e3d4f16764fda0ed56f200af4... |
