|
|
DescriptionGenerate diffing functions for generated subgroup InheritedData in ComputedStyle
This patch generates the diffing functions for StyleInheritedData which is now
a generated subgroup of ComputedStyle.
Please note: No added callsite for
ComputedStyleBase::DiffNeedsFullLayoutAndPaintInvalidation has been
added since it is already being called. We have just appended to that
JSON entry.
Diff: https://gist.github.com/nainar/5650d8c7dc6b9cc5d784c5be2409b338/revisions
BUG=710938
Review-Url: https://codereview.chromium.org/2886233004
Cr-Commit-Position: refs/heads/master@{#473015}
Committed: https://chromium.googlesource.com/chromium/src/+/702510f88b10b7ab5df0a87ada3d09f6d1139873
Patch Set 1 #
Total comments: 7
Messages
Total messages: 25 (15 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.
nainar@chromium.org changed reviewers: + shend@chromium.org
shend@, PTAL? Thanks!
The generated diff seems to be for another patch, can you reupload? https://codereview.chromium.org/2886233004/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5 (right): https://codereview.chromium.org/2886233004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5:30: "padding-bottom", "line-height", "font", "-webkit-border-horizontal-spacing", "-webkit-border-vertical-spacing"], weird line wrapping from rietveld :/
Description was changed from ========== Generate diffing functions for generated subgroup InheritedData in ComputedStyle This patch generates the diffing functions for StyleInheritedData which is now a generated subgroup of ComputedStyle. Please note: No added callsite for ComputedStyleBase::DiffNeedsFullLayoutAndPaintInvalidation has been added since it is already being called. We have just appended to that JSON entry. Diff: https://gist.github.com/dc683ef590e3d4e88e1ed7f3a54ff16f/revisions BUG=710938 ========== to ========== Generate diffing functions for generated subgroup InheritedData in ComputedStyle This patch generates the diffing functions for StyleInheritedData which is now a generated subgroup of ComputedStyle. Please note: No added callsite for ComputedStyleBase::DiffNeedsFullLayoutAndPaintInvalidation has been added since it is already being called. We have just appended to that JSON entry. Diff: https://gist.github.com/nainar/5650d8c7dc6b9cc5d784c5be2409b338/revisions BUG=710938 ==========
Updated the diff in the CL description https://codereview.chromium.org/2886233004/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5 (right): https://codereview.chromium.org/2886233004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5:30: "padding-bottom", "line-height", "font", "-webkit-border-horizontal-spacing", "-webkit-border-vertical-spacing"], On 2017/05/18 at 22:17:51, shend wrote: > weird line wrapping from rietveld :/ I ran git cl format over this and it stayed the same.
lgtm https://codereview.chromium.org/2886233004/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5 (right): https://codereview.chromium.org/2886233004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5:30: "padding-bottom", "line-height", "font", "-webkit-border-horizontal-spacing", "-webkit-border-vertical-spacing"], On 2017/05/19 at 00:00:38, nainar wrote: > On 2017/05/18 at 22:17:51, shend wrote: > > weird line wrapping from rietveld :/ > > I ran git cl format over this and it stayed the same. oh it's not the code itself, it's way it's presented on rietveld I think.
The CQ bit was checked by nainar@chromium.org
The CQ bit was unchecked by nainar@chromium.org
nainar@chromium.org changed reviewers: + meade@chromium.org
meade, PTAL? Thanks! https://codereview.chromium.org/2886233004/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5 (right): https://codereview.chromium.org/2886233004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5:30: "padding-bottom", "line-height", "font", "-webkit-border-horizontal-spacing", "-webkit-border-vertical-spacing"], On 2017/05/19 at 00:09:18, shend wrote: > On 2017/05/19 at 00:00:38, nainar wrote: > > On 2017/05/18 at 22:17:51, shend wrote: > > > weird line wrapping from rietveld :/ > > > > I ran git cl format over this and it stayed the same. > > > oh it's not the code itself, it's way it's presented on rietveld I think. The side by side diff is a bit weird ok - I had it turned off. Sorry :(
The CQ bit was checked by nainar@chromium.org
The CQ bit was unchecked by nainar@chromium.org
nainar@chromium.org changed reviewers: + alancutter@chromium.org - meade@chromium.org
Subbing meade@ for alancutter@, PTAL? Thanks!
lgtm https://codereview.chromium.org/2886233004/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5 (right): https://codereview.chromium.org/2886233004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5:30: "padding-bottom", "line-height", "font", "-webkit-border-horizontal-spacing", "-webkit-border-vertical-spacing"], On 2017/05/19 at 00:15:02, nainar wrote: > On 2017/05/19 at 00:09:18, shend wrote: > > On 2017/05/19 at 00:00:38, nainar wrote: > > > On 2017/05/18 at 22:17:51, shend wrote: > > > > weird line wrapping from rietveld :/ > > > > > > I ran git cl format over this and it stayed the same. > > > > > > oh it's not the code itself, it's way it's presented on rietveld I think. > > The side by side diff is a bit weird ok - I had it turned off. Sorry :( I don't think git cl format does Python. https://codereview.chromium.org/2886233004/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/style/ComputedStyle.cpp (left): https://codereview.chromium.org/2886233004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/style/ComputedStyle.cpp:734: } Haha, this is so much easier to read in the generated code because no 80 char line limits! :D
https://codereview.chromium.org/2886233004/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/style/ComputedStyle.cpp (left): https://codereview.chromium.org/2886233004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/style/ComputedStyle.cpp:734: } On 2017/05/19 at 01:16:58, alancutter wrote: > Haha, this is so much easier to read in the generated code because no 80 char line limits! :D OMG I never realized - even more reason to generate all the code!!! :D
The CQ bit was checked by nainar@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": 1495156702179790, "parent_rev": "a64d3f2e11c37e5c017c4ae5cceb10b8144eaf0f", "commit_rev": "702510f88b10b7ab5df0a87ada3d09f6d1139873"}
Message was sent while issue was closed.
Description was changed from ========== Generate diffing functions for generated subgroup InheritedData in ComputedStyle This patch generates the diffing functions for StyleInheritedData which is now a generated subgroup of ComputedStyle. Please note: No added callsite for ComputedStyleBase::DiffNeedsFullLayoutAndPaintInvalidation has been added since it is already being called. We have just appended to that JSON entry. Diff: https://gist.github.com/nainar/5650d8c7dc6b9cc5d784c5be2409b338/revisions BUG=710938 ========== to ========== Generate diffing functions for generated subgroup InheritedData in ComputedStyle This patch generates the diffing functions for StyleInheritedData which is now a generated subgroup of ComputedStyle. Please note: No added callsite for ComputedStyleBase::DiffNeedsFullLayoutAndPaintInvalidation has been added since it is already being called. We have just appended to that JSON entry. Diff: https://gist.github.com/nainar/5650d8c7dc6b9cc5d784c5be2409b338/revisions BUG=710938 Review-Url: https://codereview.chromium.org/2886233004 Cr-Commit-Position: refs/heads/master@{#473015} Committed: https://chromium.googlesource.com/chromium/src/+/702510f88b10b7ab5df0a87ada3d... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/702510f88b10b7ab5df0a87ada3d... |