WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
73198
[Chromium] Move "final" batch of headers into public/platform
https://bugs.webkit.org/show_bug.cgi?id=73198
Summary
[Chromium] Move "final" batch of headers into public/platform
Adam Barth
Reported
2011-11-28 01:59:33 PST
[Chromium] Move "final" batch of headers into public/platform
Attachments
Patch
(89.17 KB, patch)
2011-11-28 02:02 PST
,
Adam Barth
no flags
Details
Formatted Diff
Diff
Patch
(94.62 KB, patch)
2011-11-28 02:17 PST
,
Adam Barth
no flags
Details
Formatted Diff
Diff
Patch
(94.48 KB, patch)
2011-11-28 09:33 PST
,
Adam Barth
no flags
Details
Formatted Diff
Diff
Patch
(66.18 KB, patch)
2011-11-28 10:43 PST
,
Adam Barth
fishd
: review+
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Adam Barth
Comment 1
2011-11-28 02:02:33 PST
Created
attachment 116714
[details]
Patch
Adam Barth
Comment 2
2011-11-28 02:02:59 PST
The next step after this patch is to remove uses of the forwarding headers and delete them.
WebKit Review Bot
Comment 3
2011-11-28 02:05:49 PST
Attachment 116714
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/chromium/ChangeLog', u'Sourc..." exit_code: 1 Last 3072 characters of output: should be removed. [readability/parameter_name] [5] Source/WebKit/chromium/public/platform/WebURLLoadTiming.h:76: The parameter name "start" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit/chromium/public/platform/WebURLLoadTiming.h:79: The parameter name "end" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit/chromium/public/platform/WebURLLoadTiming.h:82: The parameter name "start" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit/chromium/public/platform/WebURLLoadTiming.h:85: The parameter name "end" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit/chromium/public/platform/WebURLLoadTiming.h:88: The parameter name "end" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit/chromium/public/platform/WebURLLoadTiming.h:91: The parameter name "start" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit/chromium/public/platform/WebURLLoadTiming.h:94: The parameter name "end" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit/chromium/public/platform/WebFontCache.h:54: One space before end of line comments [whitespace/comments] [5] Source/WebKit/chromium/public/platform/WebFontCache.h:57: One space before end of line comments [whitespace/comments] [5] Source/WebKit/chromium/public/platform/WebImageDecoder.h:58: The parameter name "data" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit/chromium/public/platform/WebImageDecoder.h:83: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit/chromium/public/platform/WebURLResponse.h:164: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WebKit/chromium/public/platform/WebHTTPBody.h:79: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WebKit/chromium/public/platform/WebHTTPBody.h:90: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WebKit/chromium/public/platform/WebThreadSafeData.h:45: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WebKit/chromium/public/platform/WebBlobData.h:71: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WebKit/chromium/public/platform/WebURLError.h:45: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WebKit/chromium/public/platform/WebURLError.h:49: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WebKit/chromium/public/platform/WebURLError.h:50: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Total errors found: 36 in 62 files If any of these errors are false positives, please file a bug against check-webkit-style.
WebKit Review Bot
Comment 4
2011-11-28 02:06:07 PST
Please wait for approval from
fishd@chromium.org
before submitting because this patch contains changes to the Chromium public API.
Adam Barth
Comment 5
2011-11-28 02:17:28 PST
Created
attachment 116717
[details]
Patch
WebKit Review Bot
Comment 6
2011-11-28 02:20:33 PST
Attachment 116717
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/chromium/ChangeLog', u'Sourc..." exit_code: 1 Source/WebKit/chromium/public/platform/WebTextDirection.h:38: One space before end of line comments [whitespace/comments] [5] Source/WebKit/chromium/public/platform/WebURLLoaderClient.h:44: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WebKit/chromium/public/platform/WebURLLoaderClient.h:48: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WebKit/chromium/public/platform/WebURLLoaderClient.h:56: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WebKit/chromium/public/platform/WebGlyphCache.h:46: One space before end of line comments [whitespace/comments] [5] Source/WebKit/chromium/public/platform/WebGlyphCache.h:49: One space before end of line comments [whitespace/comments] [5] Source/WebKit/chromium/public/platform/WebURLRequest.h:53: One space before end of line comments [whitespace/comments] [5] Source/WebKit/chromium/public/platform/WebURLRequest.h:114: The parameter name "allowCookies" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit/chromium/public/platform/WebURLRequest.h:119: The parameter name "allowStoredCredentials" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit/chromium/public/platform/WebURLRequest.h:169: The parameter name "id" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit/chromium/public/platform/WebURLRequest.h:172: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WebKit/chromium/public/platform/WebTextCaseSensitivity.h:38: One space before end of line comments [whitespace/comments] [5] Total errors found: 12 in 62 files If any of these errors are false positives, please file a bug against check-webkit-style.
Darin Fisher (:fishd, Google)
Comment 7
2011-11-28 08:09:30 PST
Comment on
attachment 116717
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=116717&action=review
> Source/WebKit/chromium/ChangeLog:17 > + 3) Geometry-reltaed headers.
nit: reltaed -> related
> Source/WebKit/chromium/ChangeLog:61 > + * public/platform/WebFontCache.h: Copied from Source/WebKit/chromium/public/WebFontCache.h.
even though the font and glyph caches are platform concepts, these interfaces only exist to provide the embedder with a way to access those WebCore systems. it seems like public/platform/ should only contain interfaces needed to support WebCore, so these interfaces should probably not be moved to public/platform/.
Adam Barth
Comment 8
2011-11-28 09:28:24 PST
> so these interfaces should probably not be moved to public/platform/.
Okiedokes.
Adam Barth
Comment 9
2011-11-28 09:33:59 PST
Created
attachment 116770
[details]
Patch
Darin Fisher (:fishd, Google)
Comment 10
2011-11-28 09:53:07 PST
Comment on
attachment 116770
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=116770&action=review
sorry, i should have reviewed this more carefully the first time around :-/
> Source/WebKit/chromium/ChangeLog:54 > + * public/platform/WebCookie.h: Copied from Source/WebKit/chromium/public/WebCookie.h.
WebCookie should be moved assuming WebCookieJar is moving. they go together.
> Source/WebKit/chromium/ChangeLog:58 > + * public/platform/WebFont.h: Copied from Source/WebKit/chromium/public/WebFont.h.
WebFont just looks like it exists to allow the embedder to access WebCore's font information and font drawing capabilities. probably isn't required by WebCore.
> Source/WebKit/chromium/ChangeLog:63 > + * public/platform/WebImageDecoder.h: Copied from Source/WebKit/chromium/public/WebImageDecoder.h.
ugh, sorry ... WebImageDecoder is also similarly just about exposing WebCore's image decoders to the embedder of webkit. so, this also shouldn't go into public/platform/, right?
> Source/WebKit/chromium/ChangeLog:65 > + * public/platform/WebRegularExpression.h: Copied from Source/WebKit/chromium/public/WebRegularExpression.h.
ditto for WebRegularExpression
> Source/WebKit/chromium/ChangeLog:76 > + * public/platform/WebVideoFrame.h: Copied from Source/WebKit/chromium/public/WebVideoFrame.h.
it makes sense to move WebVideoFrame assuming WebMediaPlayer is moving too. i suspect that is a good idea since the media player is a lot like the network stack.
Darin Fisher (:fishd, Google)
Comment 11
2011-11-28 09:53:58 PST
it may be the case that things like WebFont, WebRegularExpression and WebImageDecoder are neither "platform" nor "client" but rather they are optional "utilities" provided to the embedder.
Adam Barth
Comment 12
2011-11-28 10:01:47 PST
> > Source/WebKit/chromium/ChangeLog:54 > > + * public/platform/WebCookie.h: Copied from Source/WebKit/chromium/public/WebCookie.h. > > WebCookie should be moved assuming WebCookieJar is moving. they go together.
WebCookieJar has already moved.
> it may be the case that things like WebFont, WebRegularExpression and WebImageDecoder are neither "platform" nor "client" but rather they are optional "utilities" provided to the embedder.
Ok, that's a slightly different criteria than I used before. I'll go through these again with that in mind. Thanks.
Adam Barth
Comment 13
2011-11-28 10:43:13 PST
Created
attachment 116781
[details]
Patch
Adam Barth
Comment 14
2011-11-28 10:45:09 PST
> it makes sense to move WebVideoFrame assuming WebMediaPlayer is moving too. i suspect that is a good idea > since the media player is a lot like the network stack.
I decided to leave the media player issue for a future patch. :)
Adam Barth
Comment 15
2011-11-28 11:13:29 PST
Committed
r101267
: <
http://trac.webkit.org/changeset/101267
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug