Bug 73198 – [Chromium] Move "final" batch of headers into public/platform

• Component: New Bugs
• Status: RESOLVED FIXED

[Adam Barth]

[Chromium] Move "final" batch of headers into public/platform

[Adam Barth]

Created attachment 116714 [details]
Patch

[Adam Barth]

The next step after this patch is to remove uses of the forwarding headers and delete them.

[WebKit Review Bot]

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]

Please wait for approval from [e-mail] before submitting because this patch contains changes to the Chromium public API.

[Adam Barth]

Created attachment 116717 [details]
Patch

[WebKit Review Bot]

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 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]

> so these interfaces should probably not be moved to public/platform/.

Okiedokes.

[Adam Barth]

Created attachment 116770 [details]
Patch

[Darin Fisher (:fishd, Google)]

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)]

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]

> > 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]

Created attachment 116781 [details]
Patch

[Adam Barth]

> 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]

Committed r101267: <http://trac.webkit.org/changeset/101267>