Bug 161602

Summary:

[Fetch API] Move WebCore/loader/cache to WebCore/loader/fetch

Product:

WebKit

Reporter:

youenn fablet <youennf>

Component:

WebCore Misc.

Assignee:

youenn fablet <youennf>

Status:

NEW ---

Severity:

Normal

CC:

achristensen, beidson, commit-queue, koivisto

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

162963

Bug Blocks:

151937

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	achristensen: review-

Description youenn fablet 2016-09-05 06:19:31 PDT

As part of implementation of fetch, it seems best to transition from "CachedXXX" to "FetchedXXX".
Let's start with the folder name.

Comment 1 youenn fablet 2016-09-05 06:25:00 PDT

Created attachment 287948 [details]
Patch

Comment 2 WebKit Commit Bot 2016-09-05 06:27:38 PDT

Attachment 287948 [details] did not pass style-queue:


ERROR: Source/WebCore/loader/fetch/CachedResourceHandle.h:66:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WebCore/loader/fetch/CachedResourceHandle.h:67:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WebCore/loader/fetch/MemoryCache.h:25:  #ifndef header guard has wrong style, please use: MemoryCache_h  [build/header_guard] [5]
ERROR: Source/WebCore/loader/fetch/MemoryCache.h:116:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/MemoryCache.h:196:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/MemoryCache.h:207:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/MemoryCache.h:212:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/CachedCSSStyleSheet.h:33:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebCore/loader/fetch/CachedFont.cpp:58:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/CachedSVGFont.cpp:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/loader/fetch/CachedTextTrack.cpp:30:  You should not add a blank line before implementation file's own header.  [build/include_order] [4]
ERROR: Source/WebCore/loader/fetch/CachedRawResource.cpp:155:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/loader/fetch/CachedResourceLoader.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/loader/fetch/CachedResourceLoader.cpp:146:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WebCore/loader/fetch/CachedResourceLoader.cpp:297:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/WebCore/loader/fetch/CachedResourceLoader.cpp:1028:  'protectDocumentLoader' is incorrectly named. It should be named 'protector' or 'protectedDocumentLoader'.  [readability/naming/protected] [4]
ERROR: Source/WebCore/loader/fetch/CachedResourceLoader.cpp:1029:  'protectDocument' is incorrectly named. It should be named 'protector' or 'protectedDocument'.  [readability/naming/protected] [4]
ERROR: Source/WebCore/loader/fetch/CachedImage.cpp:295:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/loader/fetch/CachedImage.cpp:340:  An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
ERROR: Source/WebCore/loader/fetch/CachedCSSStyleSheet.cpp:123:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/CachedCSSStyleSheet.cpp:125:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/MemoryCache.cpp:284:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/loader/fetch/MemoryCache.cpp:367:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/loader/fetch/MemoryCache.cpp:753:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/loader/fetch/MemoryCache.cpp:762:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/loader/fetch/MemoryCache.cpp:764:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/loader/fetch/MemoryCache.cpp:765:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/loader/fetch/MemoryCache.cpp:766:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/loader/fetch/CachedResourceLoader.h:95:  The parameter name "url" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/loader/fetch/CachedResourceLoader.h:98:  The parameter name "url" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/loader/fetch/CachePolicy.h:31:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebCore/loader/fetch/CachedResource.h:89:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/CachedResource.h:90:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/CachedResource.h:91:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/CachedResource.h:245:  The parameter name "h" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/loader/fetch/CachedResource.h:246:  The parameter name "h" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/loader/fetch/CachedResource.cpp:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/loader/fetch/CachedResource.cpp:56:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/loader/fetch/CachedResource.cpp:656:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/loader/fetch/CachedResource.cpp:810:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 40 in 43 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 3 youenn fablet 2016-09-05 07:42:39 PDT

Created attachment 287956 [details]
Patch

Comment 4 WebKit Commit Bot 2016-09-05 08:06:25 PDT

Attachment 287956 [details] did not pass style-queue:


ERROR: Source/WebCore/loader/fetch/CachedResourceHandle.h:66:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WebCore/loader/fetch/CachedResourceHandle.h:67:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WebCore/loader/fetch/MemoryCache.h:25:  #ifndef header guard has wrong style, please use: MemoryCache_h  [build/header_guard] [5]
ERROR: Source/WebCore/loader/fetch/MemoryCache.h:116:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/MemoryCache.h:196:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/MemoryCache.h:207:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/MemoryCache.h:212:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/CachedCSSStyleSheet.h:33:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebCore/loader/fetch/CachedFont.cpp:58:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/CachedSVGFont.cpp:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/loader/fetch/CachedTextTrack.cpp:30:  You should not add a blank line before implementation file's own header.  [build/include_order] [4]
ERROR: Source/WebCore/loader/fetch/CachedResourceLoader.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/loader/fetch/CachedResourceLoader.cpp:146:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WebCore/loader/fetch/CachedResourceLoader.cpp:297:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/WebCore/loader/fetch/CachedResourceLoader.cpp:1028:  'protectDocumentLoader' is incorrectly named. It should be named 'protector' or 'protectedDocumentLoader'.  [readability/naming/protected] [4]
ERROR: Source/WebCore/loader/fetch/CachedResourceLoader.cpp:1029:  'protectDocument' is incorrectly named. It should be named 'protector' or 'protectedDocument'.  [readability/naming/protected] [4]
ERROR: Source/WebCore/loader/fetch/CachedImage.cpp:295:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/loader/fetch/CachedImage.cpp:340:  An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
ERROR: Source/WebCore/loader/fetch/CachedCSSStyleSheet.cpp:123:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/CachedCSSStyleSheet.cpp:125:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/CachedRawResource.cpp:155:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/loader/fetch/MemoryCache.cpp:284:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/loader/fetch/MemoryCache.cpp:367:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/loader/fetch/MemoryCache.cpp:753:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/loader/fetch/MemoryCache.cpp:762:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/loader/fetch/MemoryCache.cpp:764:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/loader/fetch/MemoryCache.cpp:765:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/loader/fetch/MemoryCache.cpp:766:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/loader/fetch/CachedResourceLoader.h:95:  The parameter name "url" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/loader/fetch/CachedResourceLoader.h:98:  The parameter name "url" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/loader/fetch/CachePolicy.h:31:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebCore/loader/fetch/CachedResource.h:89:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/CachedResource.h:90:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/CachedResource.h:91:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/CachedResource.h:245:  The parameter name "h" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/loader/fetch/CachedResource.h:246:  The parameter name "h" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/loader/fetch/CachedResource.cpp:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/loader/fetch/CachedResource.cpp:56:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/loader/fetch/CachedResource.cpp:656:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/loader/fetch/CachedResource.cpp:810:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 40 in 45 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 5 youenn fablet 2016-09-05 08:32:56 PDT

Created attachment 287962 [details]
Patch

Comment 6 WebKit Commit Bot 2016-09-05 08:36:29 PDT

Attachment 287962 [details] did not pass style-queue:


ERROR: Source/WebCore/loader/fetch/CachedSVGFont.cpp:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/loader/fetch/MemoryCache.h:25:  #ifndef header guard has wrong style, please use: MemoryCache_h  [build/header_guard] [5]
ERROR: Source/WebCore/loader/fetch/MemoryCache.h:116:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/MemoryCache.h:196:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/MemoryCache.h:207:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/MemoryCache.h:212:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/CachedTextTrack.cpp:30:  You should not add a blank line before implementation file's own header.  [build/include_order] [4]
ERROR: Source/WebCore/loader/fetch/CachedResourceHandle.h:66:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WebCore/loader/fetch/CachedResourceHandle.h:67:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WebCore/loader/fetch/CachedResource.h:89:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/CachedResource.h:90:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/CachedResource.h:91:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/CachedResource.h:245:  The parameter name "h" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/loader/fetch/CachedResource.h:246:  The parameter name "h" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/loader/fetch/CachedFont.cpp:58:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/CachedCSSStyleSheet.cpp:123:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/CachedCSSStyleSheet.cpp:125:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WebCore/loader/fetch/CachedResourceLoader.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/loader/fetch/CachedResourceLoader.cpp:146:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WebCore/loader/fetch/CachedResourceLoader.cpp:297:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/WebCore/loader/fetch/CachedResourceLoader.cpp:1028:  'protectDocumentLoader' is incorrectly named. It should be named 'protector' or 'protectedDocumentLoader'.  [readability/naming/protected] [4]
ERROR: Source/WebCore/loader/fetch/CachedResourceLoader.cpp:1029:  'protectDocument' is incorrectly named. It should be named 'protector' or 'protectedDocument'.  [readability/naming/protected] [4]
ERROR: Source/WebCore/loader/fetch/MemoryCache.cpp:284:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/loader/fetch/MemoryCache.cpp:367:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/loader/fetch/MemoryCache.cpp:753:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/loader/fetch/MemoryCache.cpp:762:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/loader/fetch/MemoryCache.cpp:764:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/loader/fetch/MemoryCache.cpp:765:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/loader/fetch/MemoryCache.cpp:766:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/PlatformWin.cmake:215:  Alphabetical sorting problem. "loader/fetch" should be before "loader/icon".  [list/order] [5]
ERROR: Source/WebCore/loader/fetch/CachedCSSStyleSheet.h:33:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebCore/loader/fetch/CachedImage.cpp:295:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/loader/fetch/CachedImage.cpp:340:  An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
ERROR: Source/WebCore/loader/fetch/CachedRawResource.cpp:155:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/loader/fetch/CachedResourceLoader.h:95:  The parameter name "url" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/loader/fetch/CachedResourceLoader.h:98:  The parameter name "url" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/loader/fetch/CachePolicy.h:31:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebCore/loader/fetch/CachedResource.cpp:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/loader/fetch/CachedResource.cpp:56:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/loader/fetch/CachedResource.cpp:656:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/loader/fetch/CachedResource.cpp:810:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 41 in 51 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 7 Alex Christensen 2016-09-05 12:36:52 PDT

Comment on attachment 287962 [details]
Patch

Why?  What will this improve?
And, if we're going to move and rename, we'll have to change all the file names as we change the names of the classes anyways, so there is definitely no reason for this move as-is.

Comment 8 youenn fablet 2016-09-05 13:28:34 PDT

(In reply to comment #7)
> Comment on attachment 287962 [details]
> Patch
> 
> Why?  What will this improve?

Goal is to better relate class and directory names with spec.
CachedResourceRequest will become FetchedResourceRequest for instance.

> And, if we're going to move and rename, we'll have to change all the file
> names as we change the names of the classes anyways, so there is definitely
> no reason for this move as-is.

Right, there will be another file renaming from CachedXX.h/.cpp to FetchedXX.h/.cpp.
I can do that progressively from loader/cache/CachedXX to loader/fetch/FetchedXX.

But it seems easier to rename the directory first, update the header paths in CMake, and then rename each file/each class plus corresponding cleaning.

What is the issue with this approach?

Comment 9 Alex Christensen 2016-09-06 14:50:19 PDT

> But it seems easier to rename the directory first, update the header paths
> in CMake, and then rename each file/each class plus corresponding cleaning.
I think if we do this, then we should do both of these steps at the same time.

Comment 10 youenn fablet 2016-09-06 23:02:28 PDT

(In reply to comment #9)
> > But it seems easier to rename the directory first, update the header paths
> > in CMake, and then rename each file/each class plus corresponding cleaning.
> I think if we do this, then we should do both of these steps at the same
> time.

Renaming files and directories at the same time is fine.
Renaming the classes is more work and I would prefer tackle these as separate bugs.

Also I would prefer doing the renaming of the class when changing the filename, hence the first option.

Comment 11 Alex Christensen 2016-09-07 09:56:07 PDT

If we are going to rename the classes, then we will have to have another big patch that changes all the filenames again with class name changes.  This patch is a huge change just to change directories, which isn't a significant improvement.  Changing directories at the same time as changing class names is fine.