Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2834583004:
Use generated copy and compare for VerticalAlign in ComputedStyle. (Closed)
DescriptionUse generated copy and compare for VerticalAlign in ComputedStyle.
The vertical_align_ field in ComputedStyle currently has generated
storage, but not copy and compare. This patch makes it use generated
copy and compare. We are able to do this now because we have added
the 'has_custom_compare_and_copy' parameter in a previous patch.
Diff of generated files:
https://gist.github.com/183fa3073f5de347a685b1834913ee2e/revisions
BUG=628043
Review-Url: https://codereview.chromium.org/2834583004
Cr-Commit-Position: refs/heads/master@{#467874}
Committed: https://chromium.googlesource.com/chromium/src/+/b68f094954f3a2173d54f748535c62528e1a6f35
Patch Set 1 #
Total comments: 2
Patch Set 2 : Address comments #Patch Set 3 : vertical align #
Depends on Patchset: Messages
Total messages: 32 (24 generated)
Description was changed from ========== Use generated copy and compare for VerticalAlign in ComputedStyle. The vertical_align_ field in ComputedStyle currently has generated storage, but not copy and compare. This patch makes it use generated copy and compare. BUG=628043 ========== to ========== Use generated copy and compare for VerticalAlign in ComputedStyle. The vertical_align_ field in ComputedStyle currently has generated storage, but not copy and compare. This patch makes it use generated copy and compare. Diff of generated files: https://gist.github.com/183fa3073f5de347a685b1834913ee2e/revisions BUG=628043 ==========
The CQ bit was checked by shend@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...
shend@chromium.org changed reviewers: + bugsnash@chromium.org
Hi Bugs, PTAL
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2017/04/21 at 03:53:31, shend wrote: > Hi Bugs, PTAL lgtm with comment request
ugh, didn't publish the comment request https://codereview.chromium.org/2834583004/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5 (right): https://codereview.chromium.org/2834583004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5:156: // which can be either a keyword or Length, this is generated in this file for now. Remove this Please add to this comment where in this file (function name?) this is generated
On 2017/04/24 at 00:33:42, Bugs Nash wrote: > ugh, didn't publish the comment request > > https://codereview.chromium.org/2834583004/diff/1/third_party/WebKit/Source/c... > File third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5 (right): > > https://codereview.chromium.org/2834583004/diff/1/third_party/WebKit/Source/c... > third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5:156: // which can be either a keyword or Length, this is generated in this file for now. Remove this > Please add to this comment where in this file (function name?) this is generated also please add link to changes in generated files :)
shend@chromium.org changed reviewers: + alancutter@chromium.org
Done. Hi Alan, PTAL. https://codereview.chromium.org/2834583004/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5 (right): https://codereview.chromium.org/2834583004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5:156: // which can be either a keyword or Length, this is generated in this file for now. Remove this On 2017/04/24 at 00:33:42, Bugs Nash wrote: > Please add to this comment where in this file (function name?) this is generated Oops, the comment is misleading. Changed.
Description was changed from ========== Use generated copy and compare for VerticalAlign in ComputedStyle. The vertical_align_ field in ComputedStyle currently has generated storage, but not copy and compare. This patch makes it use generated copy and compare. Diff of generated files: https://gist.github.com/183fa3073f5de347a685b1834913ee2e/revisions BUG=628043 ========== to ========== Use generated copy and compare for VerticalAlign in ComputedStyle. The vertical_align_ field in ComputedStyle currently has generated storage, but not copy and compare. This patch makes it use generated copy and compare. We are able to do this now because we have added the 'has_custom_compare_and_copy' parameter in a previous patch. Diff of generated files: https://gist.github.com/183fa3073f5de347a685b1834913ee2e/revisions BUG=628043 ==========
lgtm
The CQ bit was checked by shend@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: Try jobs failed on following builders: chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
The CQ bit was checked by shend@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: Try jobs failed on following builders: android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...) android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...) android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...) cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...) chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...) linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by shend@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.
The CQ bit was checked by shend@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from bugsnash@chromium.org, alancutter@chromium.org Link to the patchset: https://codereview.chromium.org/2834583004/#ps40001 (title: "vertical align")
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": 40001, "attempt_start_ts": 1493349939079470,
"parent_rev": "498b00e062fb73064e19187b6b174f0d6ff62968", "commit_rev":
"b68f094954f3a2173d54f748535c62528e1a6f35"}
Message was sent while issue was closed.
Description was changed from ========== Use generated copy and compare for VerticalAlign in ComputedStyle. The vertical_align_ field in ComputedStyle currently has generated storage, but not copy and compare. This patch makes it use generated copy and compare. We are able to do this now because we have added the 'has_custom_compare_and_copy' parameter in a previous patch. Diff of generated files: https://gist.github.com/183fa3073f5de347a685b1834913ee2e/revisions BUG=628043 ========== to ========== Use generated copy and compare for VerticalAlign in ComputedStyle. The vertical_align_ field in ComputedStyle currently has generated storage, but not copy and compare. This patch makes it use generated copy and compare. We are able to do this now because we have added the 'has_custom_compare_and_copy' parameter in a previous patch. Diff of generated files: https://gist.github.com/183fa3073f5de347a685b1834913ee2e/revisions BUG=628043 Review-Url: https://codereview.chromium.org/2834583004 Cr-Commit-Position: refs/heads/master@{#467874} Committed: https://chromium.googlesource.com/chromium/src/+/b68f094954f3a2173d54f748535c... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/b68f094954f3a2173d54f748535c... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
