Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(58)

Issue 2668093003: Revert of Added api_methods flag to CSSProperties.json5. (Closed)

Created:
3 years, 10 months ago by tsergeant
Modified:
3 years, 10 months ago
Reviewers:
nainar, sashab, ktyliu, aazzam, shend
CC:
ktyliu, darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, chromium-reviews, dglazkov+blink, rwlbuis
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Added api_methods flag to CSSProperties.json5. (patchset #10 id:160001 of https://codereview.chromium.org/2654403003/ ) Reason for revert: This CL causes compile to fail on GPU Mac Builder: [1856/2060] CXX obj/third_party/WebKit/Source/core/css/css_3/CSSPropertyAPIMargin.o ../../third_party/WebKit/Source/core/css/properties/CSSPropertyAPIMargin.cpp:13:39: error: out-of-line definition of 'parseSingleValue' does not match any declaration in 'blink::CSSPropertyAPIMargin' const CSSValue* CSSPropertyAPIMargin::parseSingleValue( ^~~~~~~~~~~~~~~~ 1 error generated. See: https://build.chromium.org/p/chromium.gpu/builders/GPU%20Mac%20Builder/builds/82086 Original issue's description: > Added api_methods flag to CSSProperties.json5. > > A part of Project Ribbon, separating the parsing logic for CSS > properties from the parser into an API. > > This patch adds an api_methods=["method1", "method2"] flag to > CSSProperties.json5, which specifies which methods have been > implemented in the API implementation of that property. This is so > that when API .h files are generated, only methods implemented for > that property are defined. > > It also adds "parseSingleValue" to the list of api_methods for all > existing properties with an API that implements parseSingleValue. > > Before, when a method was added to the API, this method had to be > implemented in all API files - however, not every API should hold > each method. In this new design, to add a method to the API: > - Add the method name to the valid_values array in CSSProperties.json5. > - Add the method to CSSPropertyAPIFiles.h.tmpl > - Add the method name to the api_methods list in CSSProperties.json5 > for each property where it is implemented. > > BUG=668012 > > Review-Url: https://codereview.chromium.org/2654403003 > Cr-Commit-Position: refs/heads/master@{#447427} > Committed: https://chromium.googlesource.com/chromium/src/+/a252f0fa5f780f176eb3388df35ec67abaf8ca79 TBR=ktyliu@chromium.org,nainar@chromium.org,sashab@chromium.org,shend@chromium.org,aazzam@google.com # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=668012 Review-Url: https://codereview.chromium.org/2668093003 Cr-Commit-Position: refs/heads/master@{#447434} Committed: https://chromium.googlesource.com/chromium/src/+/0416febbb4c1c2007524f3facd9d0ebebee8d3d1

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+20 lines, -143 lines) Patch
M third_party/WebKit/Source/build/scripts/make_css_property_apis.py View 3 chunks +3 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/build/scripts/templates/CSSPropertyAPIFiles.h.tmpl View 1 chunk +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSProperties.json5 View 87 chunks +5 lines, -118 lines 0 comments Download
M third_party/WebKit/Source/core/css/properties/CSSPropertyAPI.h View 2 chunks +12 lines, -20 lines 0 comments Download

Messages

Total messages: 7 (4 generated)
tsergeant
Created Revert of Added api_methods flag to CSSProperties.json5.
3 years, 10 months ago (2017-02-01 03:07:05 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2668093003/1
3 years, 10 months ago (2017-02-01 03:07:17 UTC) #3
commit-bot: I haz the power
3 years, 10 months ago (2017-02-01 03:09:46 UTC) #7
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/0416febbb4c1c2007524f3facd9d...

Powered by Google App Engine
This is Rietveld 408576698