Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2661323002:
Implement CSSPaintValue and add a layout test. (Closed)
|
Created:
3 years, 10 months ago by renjieliu1 Modified:
3 years, 9 months ago CC:
chromium-reviews, blink-reviews-style_chromium.org, blink-reviews-css, haraken, dglazkov+blink, apavlov+blink_chromium.org, darktears, blink-reviews, rwlbuis Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionImplement CSSPaintValue and add a layout test.
BUG=672647
Review-Url: https://codereview.chromium.org/2661323002
Cr-Commit-Position: refs/heads/master@{#453546}
Committed: https://chromium.googlesource.com/chromium/src/+/d18dd5bf544f6e8a0ed730588956df9da41db235
Patch Set 1 #Patch Set 2 : Pass in extracted csspaint arguments and call paint function. #Patch Set 3 : updates #
Total comments: 2
Patch Set 4 : fix indent #
Total comments: 4
Patch Set 5 : parse arguments early #
Total comments: 8
Patch Set 6 : rebase #
Total comments: 6
Patch Set 7 : check generator ready #
Total comments: 4
Patch Set 8 : add flag #
Total comments: 6
Patch Set 9 : refactor #
Total comments: 2
Patch Set 10 : format #
Total comments: 2
Patch Set 11 : format #Patch Set 12 : rebase #
Total comments: 2
Patch Set 13 : rebase #Messages
Total messages: 53 (27 generated)
Description was changed from ========== Implement CSSPaintValue and add a layout test. BUG= ========== to ========== Implement CSSPaintValue and add a layout test. BUG=672647 ==========
renjieliu@chromium.org changed reviewers: + meade@chromium.org
lgtm This seems pretty straight forward. Looks like you still need a paint owner. https://codereview.chromium.org/2661323002/diff/40001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/csspaint/paint-arguments.html (right): https://codereview.chromium.org/2661323002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/csspaint/paint-arguments.html:47: importPaintWorkletAndTerminateTestAfterAsyncPaint(document.getElementById('code').textContent); Nit: The js in this file is 4-indented. It should be 2-indented.
The CQ bit was checked by renjieliu@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...
thanks for the review! https://codereview.chromium.org/2661323002/diff/40001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/csspaint/paint-arguments.html (right): https://codereview.chromium.org/2661323002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/csspaint/paint-arguments.html:47: importPaintWorkletAndTerminateTestAfterAsyncPaint(document.getElementById('code').textContent); On 2017/02/09 06:48:45, Eddy wrote: > Nit: The js in this file is 4-indented. It should be 2-indented. thank you! :)
renjieliu@chromium.org changed reviewers: + ikilpatrick@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Friendly ping: Hi Ian, can you review the cl? Thank you! :)
https://codereview.chromium.org/2661323002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/css/CSSPaintValue.cpp (right): https://codereview.chromium.org/2661323002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/css/CSSPaintValue.cpp:50: return m_generator->paint(layoutObject, size, zoom, m_argumentVariableData); should be able to parse the variable data inside the constructor above? https://codereview.chromium.org/2661323002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp (right): https://codereview.chromium.org/2661323002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp:111: variableData[i].get()->parseForSyntax(m_inputArgumentTypes[i]); this feels too late to parse the data? I think you should be able to do this once inside CSSPaintValue? (instead on every invocation?)
thank you for the review! https://codereview.chromium.org/2661323002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/css/CSSPaintValue.cpp (right): https://codereview.chromium.org/2661323002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/css/CSSPaintValue.cpp:50: return m_generator->paint(layoutObject, size, zoom, m_argumentVariableData); On 2017/02/16 03:19:36, ikilpatrick wrote: > should be able to parse the variable data inside the constructor above? do you think we can pass in the input argument types here? https://codereview.chromium.org/2661323002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp (right): https://codereview.chromium.org/2661323002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp:111: variableData[i].get()->parseForSyntax(m_inputArgumentTypes[i]); On 2017/02/16 03:19:36, ikilpatrick wrote: > this feels too late to parse the data? I think you should be able to do this > once inside CSSPaintValue? (instead on every invocation?) good suggestion! however parse the CSSVariableData needs input argument types, do you think we can get it in CSSPaintValue?
Refactored the code to parse the input arguments earlier to avoid parsing every time paint function is invoked. PTAL, thanks a lot! :)
https://codereview.chromium.org/2661323002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/css/CSSPaintValue.cpp (right): https://codereview.chromium.org/2661323002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/css/CSSPaintValue.cpp:52: if (!m_parsedInputArguments) { If you make the type of m_parsedInputArguments just CSSStyleValueVector, you can check on its length. But because of the issue I mentioned below, you might need another way of determining whether you've parsed yet. https://codereview.chromium.org/2661323002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/css/CSSPaintValue.cpp:59: if (m_argumentVariableData.size() != inputArgumentTypes.size()) { In these cases, is there a case where you bail early here, but can end up with m_parsedInputArguments partially filled? You might need to clear m_parsedInputArguments when you bail. https://codereview.chromium.org/2661323002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/css/CSSPaintValue.cpp:66: return nullptr; Likewise https://codereview.chromium.org/2661323002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/css/CSSPaintValue.h (right): https://codereview.chromium.org/2661323002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/css/CSSPaintValue.h:82: Member<CSSStyleValueVector> m_parsedInputArguments; Sorry if I mislead you when we had our chat earlier.. but isn't CSSStyleValueVector already a HeapVector<Member<CSSStyleValue>>? You shouldn't need a pointer to a vector...
thank you for the review! https://codereview.chromium.org/2661323002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/css/CSSPaintValue.cpp (right): https://codereview.chromium.org/2661323002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/css/CSSPaintValue.cpp:52: if (!m_parsedInputArguments) { On 2017/02/17 07:09:37, Eddy wrote: > If you make the type of m_parsedInputArguments just CSSStyleValueVector, you can > check on its length. But because of the issue I mentioned below, you might need > another way of determining whether you've parsed yet. I'm not sure if I should check the length since a function with no arguments should be allowed. I'm making m_parsedInputArguments null again if parse failed. https://codereview.chromium.org/2661323002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/css/CSSPaintValue.cpp:59: if (m_argumentVariableData.size() != inputArgumentTypes.size()) { On 2017/02/17 07:09:37, Eddy wrote: > In these cases, is there a case where you bail early here, but can end up with > m_parsedInputArguments partially filled? You might need to clear > m_parsedInputArguments when you bail. thank you for the catch! https://codereview.chromium.org/2661323002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/css/CSSPaintValue.cpp:66: return nullptr; On 2017/02/17 07:09:37, Eddy wrote: > Likewise thank you! https://codereview.chromium.org/2661323002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/css/CSSPaintValue.h (right): https://codereview.chromium.org/2661323002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/css/CSSPaintValue.h:82: Member<CSSStyleValueVector> m_parsedInputArguments; On 2017/02/17 07:09:37, Eddy wrote: > Sorry if I mislead you when we had our chat earlier.. but isn't > CSSStyleValueVector already a HeapVector<Member<CSSStyleValue>>? You shouldn't > need a pointer to a vector... If I don't put it into member, the compiler tells me that class 'CSSPaintValue' has untraced fields that require tracing.
https://codereview.chromium.org/2661323002/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/css/CSSPaintValue.cpp (right): https://codereview.chromium.org/2661323002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/css/CSSPaintValue.cpp:52: if (!m_parsedInputArguments) { This is unfortunately racy. You'll need to check if the generator is "ready" yet before getting the inputArgumentsTypes from it. It may make sense to make this into CSSPaintValue:paintImageGeneratorReady if this code block doesn't depend on layoutObject, size, or zoom. (This can happen if you use a css paint() function before the paint worklet has loaded its script for example). https://codereview.chromium.org/2661323002/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp (right): https://codereview.chromium.org/2661323002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp:103: v8::Local<v8::Value> argv[] = { you'll probably need to surround this with "if (RuntimeEnabledFeature::isInput...)" to switch between 3 and 4 arg versions.
haraken@chromium.org changed reviewers: + haraken@chromium.org
https://codereview.chromium.org/2661323002/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/modules/csspaint/CSSPaintImageGeneratorImpl.cpp (right): https://codereview.chromium.org/2661323002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/modules/csspaint/CSSPaintImageGeneratorImpl.cpp:81: DEFINE_STATIC_LOCAL(Vector<CSSSyntaxDescriptor>, emptyVector, ()); Nit: Is it really worth defining an empty vector? I guess we could just return Vector<CSSSyntaxDescriptor>().
thank you for the review! https://codereview.chromium.org/2661323002/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/css/CSSPaintValue.cpp (right): https://codereview.chromium.org/2661323002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/css/CSSPaintValue.cpp:52: if (!m_parsedInputArguments) { On 2017/02/21 17:40:36, ikilpatrick wrote: > This is unfortunately racy. You'll need to check if the generator is "ready" yet > before getting the inputArgumentsTypes from it. > > It may make sense to make this into CSSPaintValue:paintImageGeneratorReady if > this code block doesn't depend on layoutObject, size, or zoom. > > (This can happen if you use a css paint() function before the paint worklet has > loaded its script for example). thank you for the suggestion! I will keep the code inside CSSPaintValue::image since it's easier to check whether m_generator is ready and parse the input arguments. https://codereview.chromium.org/2661323002/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp (right): https://codereview.chromium.org/2661323002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp:103: v8::Local<v8::Value> argv[] = { On 2017/02/21 17:40:36, ikilpatrick wrote: > you'll probably need to surround this with "if > (RuntimeEnabledFeature::isInput...)" to switch between 3 and 4 arg versions. Done. https://codereview.chromium.org/2661323002/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/modules/csspaint/CSSPaintImageGeneratorImpl.cpp (right): https://codereview.chromium.org/2661323002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/modules/csspaint/CSSPaintImageGeneratorImpl.cpp:81: DEFINE_STATIC_LOCAL(Vector<CSSSyntaxDescriptor>, emptyVector, ()); On 2017/02/21 23:51:55, haraken wrote: > > Nit: Is it really worth defining an empty vector? I guess we could just return > Vector<CSSSyntaxDescriptor>(). > the compiler will throw me an error says returning reference to local temporary object
just a few more comments! https://codereview.chromium.org/2661323002/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/css/CSSPaintValue.cpp (right): https://codereview.chromium.org/2661323002/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/css/CSSPaintValue.cpp:53: m_parsedInputArguments = new CSSStyleValueVector(); this should be moved down to line 66, and remove m_parsedInputArguments = nullptr lines. https://codereview.chromium.org/2661323002/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/css/CSSPaintValue.cpp:71: m_parsedInputArguments = nullptr; this seems wrong? we don't want to try and parse each time if we know its going to fail? a better solution might be to store a flag on this class and early exit if its set above?
https://codereview.chromium.org/2661323002/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/css/CSSPaintValue.cpp (right): https://codereview.chromium.org/2661323002/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/css/CSSPaintValue.cpp:53: m_parsedInputArguments = new CSSStyleValueVector(); On 2017/02/22 19:25:13, ikilpatrick wrote: > this should be moved down to line 66, and remove m_parsedInputArguments = > nullptr lines. you're right! thank you for the suggestion! https://codereview.chromium.org/2661323002/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/css/CSSPaintValue.cpp:71: m_parsedInputArguments = nullptr; On 2017/02/22 19:25:13, ikilpatrick wrote: > this seems wrong? we don't want to try and parse each time if we know its going > to fail? a better solution might be to store a flag on this class and early exit > if its set above? sounds good to me!
lgtm https://codereview.chromium.org/2661323002/diff/140001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/css/CSSPaintValue.cpp (right): https://codereview.chromium.org/2661323002/diff/140001/third_party/WebKit/Sou... third_party/WebKit/Source/core/css/CSSPaintValue.cpp:56: // Parse arguments. Could you extract this into a private function, and just have: if (!parseInputArguments()) return nullptr; bool CSSPaintValue::parseInputArguments() { if (somethingBad...) { return false; } return true; } https://codereview.chromium.org/2661323002/diff/140001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/css/CSSPaintValue.h (right): https://codereview.chromium.org/2661323002/diff/140001/third_party/WebKit/Sou... third_party/WebKit/Source/core/css/CSSPaintValue.h:79: bool m_inputArgumentsInValid = false; m_inputArgumentsInvalid (invalid is just one word). https://codereview.chromium.org/2661323002/diff/140001/third_party/WebKit/Sou... File third_party/WebKit/Source/modules/csspaint/CSSPaintImageGeneratorImpl.cpp (right): https://codereview.chromium.org/2661323002/diff/140001/third_party/WebKit/Sou... third_party/WebKit/Source/modules/csspaint/CSSPaintImageGeneratorImpl.cpp:86: return m_definition ? true : false; just: return m_definition;
thanks for the review! https://codereview.chromium.org/2661323002/diff/140001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/css/CSSPaintValue.cpp (right): https://codereview.chromium.org/2661323002/diff/140001/third_party/WebKit/Sou... third_party/WebKit/Source/core/css/CSSPaintValue.cpp:56: // Parse arguments. On 2017/02/27 17:25:02, ikilpatrick wrote: > Could you extract this into a private function, and just have: > > if (!parseInputArguments()) > return nullptr; > > bool CSSPaintValue::parseInputArguments() { > if (somethingBad...) { > return false; > } > > return true; > } thanks for the suggestion https://codereview.chromium.org/2661323002/diff/140001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/css/CSSPaintValue.h (right): https://codereview.chromium.org/2661323002/diff/140001/third_party/WebKit/Sou... third_party/WebKit/Source/core/css/CSSPaintValue.h:79: bool m_inputArgumentsInValid = false; On 2017/02/27 17:25:03, ikilpatrick wrote: > m_inputArgumentsInvalid > > (invalid is just one word). thanks for the catch! https://codereview.chromium.org/2661323002/diff/140001/third_party/WebKit/Sou... File third_party/WebKit/Source/modules/csspaint/CSSPaintImageGeneratorImpl.cpp (right): https://codereview.chromium.org/2661323002/diff/140001/third_party/WebKit/Sou... third_party/WebKit/Source/modules/csspaint/CSSPaintImageGeneratorImpl.cpp:86: return m_definition ? true : false; On 2017/02/27 17:25:03, ikilpatrick wrote: > just: > > return m_definition; > Done.
https://codereview.chromium.org/2661323002/diff/160001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/css/CSSPaintValue.cpp (right): https://codereview.chromium.org/2661323002/diff/160001/third_party/WebKit/Sou... third_party/WebKit/Source/core/css/CSSPaintValue.cpp:62: if (!m_parsedInputArguments) { oh could you de-nest this? e.g. if (m_parsedInputArguments || !RuntimeEnabledFeatures::css...()) return true; if (!m_generator->isImageGeneratorReady()) return false; // other code. return true;
https://codereview.chromium.org/2661323002/diff/160001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/css/CSSPaintValue.cpp (right): https://codereview.chromium.org/2661323002/diff/160001/third_party/WebKit/Sou... third_party/WebKit/Source/core/css/CSSPaintValue.cpp:62: if (!m_parsedInputArguments) { On 2017/02/27 23:24:29, ikilpatrick wrote: > oh could you de-nest this? e.g. > > if (m_parsedInputArguments || !RuntimeEnabledFeatures::css...()) > return true; > > if (!m_generator->isImageGeneratorReady()) > return false; > > // other code. > > return true; Done.
lgtm https://codereview.chromium.org/2661323002/diff/180001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/css/CSSPaintValue.cpp (right): https://codereview.chromium.org/2661323002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/core/css/CSSPaintValue.cpp:66: if (!m_generator->isImageGeneratorReady()) { no need for braces here.
https://codereview.chromium.org/2661323002/diff/180001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/css/CSSPaintValue.cpp (right): https://codereview.chromium.org/2661323002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/core/css/CSSPaintValue.cpp:66: if (!m_generator->isImageGeneratorReady()) { On 2017/02/28 00:10:45, ikilpatrick wrote: > no need for braces here. Done.
The CQ bit was checked by renjieliu@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: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
The CQ bit was checked by renjieliu@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...) 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_...)
The CQ bit was checked by renjieliu@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...
V8 binding usage LGTM https://codereview.chromium.org/2661323002/diff/220001/third_party/WebKit/Sou... File third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp (right): https://codereview.chromium.org/2661323002/diff/220001/third_party/WebKit/Sou... third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp:116: instance, 4, argv, isolate); Nit: You can use WTF_ARRAY_LENGTH(argv).
The CQ bit was unchecked by renjieliu@chromium.org
https://codereview.chromium.org/2661323002/diff/220001/third_party/WebKit/Sou... File third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp (right): https://codereview.chromium.org/2661323002/diff/220001/third_party/WebKit/Sou... third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp:116: instance, 4, argv, isolate); On 2017/02/28 04:21:30, haraken wrote: > > Nit: You can use WTF_ARRAY_LENGTH(argv). thanks for the suggestion!
The CQ bit was checked by renjieliu@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: linux_chromium_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 renjieliu@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from meade@chromium.org, ikilpatrick@chromium.org, haraken@chromium.org Link to the patchset: https://codereview.chromium.org/2661323002/#ps240001 (title: "rebase")
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": 240001, "attempt_start_ts": 1488268047719600,
"parent_rev": "eaef9914566e8539eb62f2d768ffeb856b157d96", "commit_rev":
"d18dd5bf544f6e8a0ed730588956df9da41db235"}
Message was sent while issue was closed.
Description was changed from ========== Implement CSSPaintValue and add a layout test. BUG=672647 ========== to ========== Implement CSSPaintValue and add a layout test. BUG=672647 Review-Url: https://codereview.chromium.org/2661323002 Cr-Commit-Position: refs/heads/master@{#453546} Committed: https://chromium.googlesource.com/chromium/src/+/d18dd5bf544f6e8a0ed730588956... ==========
Message was sent while issue was closed.
Committed patchset #13 (id:240001) as https://chromium.googlesource.com/chromium/src/+/d18dd5bf544f6e8a0ed730588956... |
