|
|
DescriptionStore border-image-* on SurroundData in ComputedStyle
This patch moves the storage of border-image-* away from BorderStyle.
We instead store them on the generated SurroundData group on
ComputedStyle instead.
We generate a BorderValue to pass to Layout in
ComputedStyle from all the border information stored
there.
Diff: https://gist.github.com/a0eeb1d1383e4b502604b449867f9619/revisions
BUG=628043
Review-Url: https://codereview.chromium.org/2891003002
Cr-Commit-Position: refs/heads/master@{#473173}
Committed: https://chromium.googlesource.com/chromium/src/+/08b8ccad61fb9dd45f25aac7aab06dd8bbdb1856
Patch Set 1 #Patch Set 2 : Ready for review #
Total comments: 6
Patch Set 3 : Rebase #Patch Set 4 : Rebase #Patch Set 5 : Rebase #Patch Set 6 : Store border-image-* on SurroundData in ComputedStyle #
Total comments: 1
Messages
Total messages: 44 (37 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: 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_cronet on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...) cast_shell_android on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...) 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-...) chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...) 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_ozone_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_rel_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 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...
nainar@chromium.org changed reviewers: + shend@chromium.org
shend@, PTAL? Thanks! https://codereview.chromium.org/2891003002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/style/ComputedStyle.h (left): https://codereview.chromium.org/2891003002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:3005: SetBorderTopStyle(EBorderStyle::kNone); These are remenants from a previous CL - clearing style twice :/
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...) 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-...) 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_...)
lgtm https://codereview.chromium.org/2891003002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/style/ComputedStyle.h (left): https://codereview.chromium.org/2891003002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:3005: SetBorderTopStyle(EBorderStyle::kNone); On 2017/05/18 at 05:27:34, nainar wrote: > These are remenants from a previous CL - clearing style twice :/ :/ https://codereview.chromium.org/2891003002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/style/ComputedStyle.h (right): https://codereview.chromium.org/2891003002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:481: void SetBorderImage(const NinePieceImage& b) { SetBorderImageInternal(b); } In a future patch, could BorderImage actually be a 'external' field? We would be able to delete these three functions if so. https://codereview.chromium.org/2891003002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:3022: void ResetBorderImage() { SetBorderImageInternal(NinePieceImage()); } When we generate border-image as external, we get this function for free as well.
The CQ bit was checked by nainar@chromium.org to run a CQ dry run
nainar@chromium.org changed reviewers: + meade@chromium.org
meade@, PTAL? Thanks! https://codereview.chromium.org/2891003002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/style/ComputedStyle.h (right): https://codereview.chromium.org/2891003002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:481: void SetBorderImage(const NinePieceImage& b) { SetBorderImageInternal(b); } Got it. https://codereview.chromium.org/2891003002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:3022: void ResetBorderImage() { SetBorderImageInternal(NinePieceImage()); } Got it.
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_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
rune@opera.com changed reviewers: + rune@opera.com
rs lgtm
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: Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...) 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_cronet on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...) cast_shell_android on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...) linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...) cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...) 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-...) chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...) chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...) 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_ozone_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_rel_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 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: Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...) 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_cronet on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...) android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...) cast_shell_android on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...) linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...) 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_ozone_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_rel_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_...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
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: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
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 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.
https://codereview.chromium.org/2891003002/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/style/BUILD.gn (left): https://codereview.chromium.org/2891003002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/style/BUILD.gn:16: "BorderData.h", Deleted this entry since last 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, rune@opera.com Link to the patchset: https://codereview.chromium.org/2891003002/#ps100001 (title: "Store border-image-* on SurroundData in ComputedStyle")
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": 100001, "attempt_start_ts": 1495199320608380, "parent_rev": "b6fa2c513e2b0f9275b13fa48eda48c5351d6c1b", "commit_rev": "08b8ccad61fb9dd45f25aac7aab06dd8bbdb1856"}
Message was sent while issue was closed.
Description was changed from ========== Store border-image-* on SurroundData in ComputedStyle This patch moves the storage of border-image-* away from BorderStyle. We instead store them on the generated SurroundData group on ComputedStyle instead. We generate a BorderValue to pass to Layout in ComputedStyle from all the border information stored there. Diff: https://gist.github.com/a0eeb1d1383e4b502604b449867f9619/revisions BUG=628043 ========== to ========== Store border-image-* on SurroundData in ComputedStyle This patch moves the storage of border-image-* away from BorderStyle. We instead store them on the generated SurroundData group on ComputedStyle instead. We generate a BorderValue to pass to Layout in ComputedStyle from all the border information stored there. Diff: https://gist.github.com/a0eeb1d1383e4b502604b449867f9619/revisions BUG=628043 Review-Url: https://codereview.chromium.org/2891003002 Cr-Commit-Position: refs/heads/master@{#473173} Committed: https://chromium.googlesource.com/chromium/src/+/08b8ccad61fb9dd45f25aac7aab0... ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as https://chromium.googlesource.com/chromium/src/+/08b8ccad61fb9dd45f25aac7aab0... |