Summary: | Change CSSParser::sourceSize returning Optional<CSSParser::SourceSize> | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Fujii Hironori <Hironori.Fujii> | ||||||||
Component: | CSS | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, darin, yoav | ||||||||
Priority: | P2 | ||||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Fujii Hironori
2016-07-11 23:26:49 PDT
Yoav, do you do this? yeah, I'm on it Created attachment 283399 [details]
Patch
Comment on attachment 283399 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=283399&action=review > Source/WebCore/css/CSSGrammar.y.in:580 > + auto result = parser->sourceSize(WTFMove(*$1), $2); > + if (result) I’d like this slightly better with the expression inside the if, but otherwise the patch looks perfect to me. Created attachment 283409 [details]
Patch
(In reply to comment #5) > Created attachment 283409 [details] > Patch (In reply to comment #4) > Comment on attachment 283399 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=283399&action=review > > > Source/WebCore/css/CSSGrammar.y.in:580 > > + auto result = parser->sourceSize(WTFMove(*$1), $2); > > + if (result) > > I’d like this slightly better with the expression inside the if, but > otherwise the patch looks perfect to me. Done. Thanks for reviewing! :) Comment on attachment 283409 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=283409&action=review > Source/WebCore/css/CSSParser.cpp:1560 > if (!value) { > value = parserValue.createCSSValue(); > if (!value) > - value = CSSPrimitiveValue::create(0, CSSPrimitiveValue::CSS_UNKNOWN); > + return Nullopt; > } > destroy(parserValue); Oops, missed the storage leak here. We have to structure this so destroy(parserValue) is called even if we are going to return Nullopt. Created attachment 283530 [details]
Patch
Comment on attachment 283530 [details] Patch Rejecting attachment 283530 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 283530, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: fs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ... Currently at 203161 = fa38974410e9b628f2311c8da4a778d48636d306 r203162 = f4555b72adc88d17af46507f6d6ff5e5c6f3b81e r203163 = 1b59d18bf9b48905986998af6522134b11aa6edb r203164 = 6226acd6e0632c939b28a948f749093ddaa3a0cd Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/origin/master. Full output: http://webkit-queues.webkit.org/results/1674971 Comment on attachment 283530 [details] Patch CQ failure at https://webkit-queues.webkit.org/results/1674971 seems spurious. "You have both LayoutTests/imported/w3c and LayoutTests/imported/w3c/ChangeLog" is not true, as this patch doesn't touch LayoutTests/imported. Trying to CQ it again. Comment on attachment 283530 [details] Patch Clearing flags on attachment: 283530 Committed r203269: <http://trac.webkit.org/changeset/203269> All reviewed patches have been landed. Closing bug. |