Move all remaining appcache-related code to content namespace

Move all remaining appcache-related code to content namespace

This introduces NO FUNCTIONAL CHANGES. It is strictly a file move and namespace change.

BUG=338338
R=michaeln@chromium.org
TBR=cevans, darin

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=279808

[michaeln, 2014-06-17 23:34:29]

we should probably refactor the 'interfaces' file instead of hoisting the entire
thing into /public. most of what is there does not belong in the content api.
the only things that are needed is stuff required for the GetAllInfo method
which is just three things...

enum AppCacheStatus
struct AppCacheInfo
typedef AppCacheInfoVector

https://codereview.chromium.org/344493002/diff/1/content/public/browser/appca...
File content/public/browser/appcache_service.h (right):

https://codereview.chromium.org/344493002/diff/1/content/public/browser/appca...
content/public/browser/appcache_service.h:35: // exclusive access to its
cache_directory on disk.
The class comment doesn't make as much sense in this context.

https://codereview.chromium.org/344493002/diff/1/content/public/common/appcac...
File content/public/common/appcache_interfaces.h (right):

https://codereview.chromium.org/344493002/diff/1/content/public/common/appcac...
content/public/common/appcache_interfaces.h:31: enum AppCacheStatus {
this is needed but not the other enums

https://codereview.chromium.org/344493002/diff/1/content/public/common/appcac...
content/public/common/appcache_interfaces.h:79: struct CONTENT_EXPORT
AppCacheInfo {
this struct is needed but not the other structs

https://codereview.chromium.org/344493002/diff/1/content/public/common/appcac...
content/public/common/appcache_interfaces.h:94: typedef
std::vector<AppCacheInfo> AppCacheInfoVector;
this is needed too

https://codereview.chromium.org/344493002/diff/1/content/public/common/appcac...
content/public/common/appcache_interfaces.h:130: struct CONTENT_EXPORT Namespace
{
this is an overly generic name for the larger content namespace

https://codereview.chromium.org/344493002/diff/1/content/public/common/appcac...
content/public/common/appcache_interfaces.h:207: CONTENT_EXPORT extern const
char kHttpHEADMethod[];
definitely these (and the other two) str constants do not belong in the public
api

https://codereview.chromium.org/344493002/diff/1/content/public/common/appcac...
content/public/common/appcache_interfaces.h:214: CONTENT_EXPORT bool
IsSchemeAndMethodSupported(
ditto these do not belong in public and also overly generic names for the larger
content namespace

[jam, 2014-06-18 00:50:09]

On 2014/06/17 23:34:29, michaeln wrote:
> we should probably refactor the 'interfaces' file instead of hoisting the
entire
> thing into /public. most of what is there does not belong in the content api.
> the only things that are needed is stuff required for the GetAllInfo method
> which is just three things...
> 
> enum AppCacheStatus
> struct AppCacheInfo
> typedef AppCacheInfoVector

+1, these should go into content/public/common/appcache_info.h. the rest need to
be hidden inside content

[pilgrim_google, 2014-06-20 14:40:27]

Please re-review.

[michaeln, 2014-06-20 22:21:12]

modulo a couple of things, lgtm

https://codereview.chromium.org/344493002/diff/70001/content/common/appcache_...
File content/common/appcache_interfaces.h (left):

https://codereview.chromium.org/344493002/diff/70001/content/common/appcache_...
content/common/appcache_interfaces.h:24: // process and in child processes.
please retain this comment in this .h file

https://codereview.chromium.org/344493002/diff/70001/content/public/browser/a...
File content/public/browser/appcache_service.h (right):

https://codereview.chromium.org/344493002/diff/70001/content/public/browser/a...
content/public/browser/appcache_service.h:35: // exclusive access to its
cache_directory on disk.
This class comments describes the the concrete implementation, but its out of
place on this very limited interface, it's also missing a comment about only
calling these methods on the IO thread. How about something like...

// Exposes a limited interface to the AppCacheService.
// Call these methods only on the IO thread.

https://codereview.chromium.org/344493002/diff/70001/content/public/browser/a...
content/public/browser/appcache_service.h:38: virtual ~AppCacheService() { }
the destructor needs to be in the protected section

https://codereview.chromium.org/344493002/diff/70001/content/public/common/ap...
File content/public/common/appcache_info.h (right):

https://codereview.chromium.org/344493002/diff/70001/content/public/common/ap...
content/public/common/appcache_info.h:17: // process and in child processes.
This comment is out of place here, maybe delete this comment here (while
retaining it in the _interfaces.h file).

[pilgrim_google, 2014-06-24 16:02:40]

+cevans because this touches appcache_messages.h (but no functional changes)

[michaeln, 2014-06-24 19:09:43]

ditto the couple things mentioned before

https://codereview.chromium.org/344493002/diff/110001/content/public/browser/...
File content/public/browser/appcache_service.h (right):

https://codereview.chromium.org/344493002/diff/110001/content/public/browser/...
content/public/browser/appcache_service.h:38: virtual ~AppCacheService() { }
nit: no space needed between {}

please make the destructor a protected member, the content api consumer
shouldn't delete the object

also would be nice to fixup the comment here and in the other new public .h
file, they describe abstractions that aren't represented in the public api and
impl details

[pilgrim_google, 2014-06-25 15:35:53]

The CQ bit was checked by pilgrim@chromium.org

[commit-bot: I haz the power, 2014-06-25 15:36:30]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pilgrim@chromium.org/344493002/130001

[commit-bot: I haz the power, 2014-06-25 18:25:00]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[commit-bot: I haz the power, 2014-06-25 18:29:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-25 18:29:54]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[pilgrim_google, 2014-06-25 20:58:08]

Committed patchset #9 manually as r279808 (presubmit successful).

[jam, 2014-06-25 21:51:14]

https://codereview.chromium.org/344493002/diff/150001/content/browser/appcach...
File content/browser/appcache/appcache.h (right):

https://codereview.chromium.org/344493002/diff/150001/content/browser/appcach...
content/browser/appcache/appcache.h:26: FORWARD_DECLARE_TEST(AppCacheTest,
InitializeWithManifest);
i dont think you need these lines anymore? (also in other files)

https://codereview.chromium.org/344493002/diff/150001/content/browser/appcach...
content/browser/appcache/appcache.h:33: namespace content {
nit: merge this with the section above (also in other files)

https://codereview.chromium.org/344493002/diff/150001/content/browser/quota/m...
File content/browser/quota/mock_quota_manager_unittest.cc (right):

https://codereview.chromium.org/344493002/diff/150001/content/browser/quota/m...
content/browser/quota/mock_quota_manager_unittest.cc:226: }  //
AppCacheNamespace content
should be "namespace content"

https://codereview.chromium.org/344493002/diff/150001/content/browser/transit...
File content/browser/transition_browsertest.cc (right):

https://codereview.chromium.org/344493002/diff/150001/content/browser/transit...
content/browser/transition_browsertest.cc:44: content::AppCacheService*
appcache_service,
nit: no "content::"

https://codereview.chromium.org/344493002/diff/150001/content/browser/webui/u...
File content/browser/webui/url_data_manager_backend.h (right):

https://codereview.chromium.org/344493002/diff/150001/content/browser/webui/u...
content/browser/webui/url_data_manager_backend.h:22: class AppCacheServiceImpl;
ditto

https://codereview.chromium.org/344493002/diff/150001/content/public/browser/...
File content/public/browser/resource_context.h (right):

https://codereview.chromium.org/344493002/diff/150001/content/public/browser/...
content/public/browser/resource_context.h:20: class AppCacheService;
nit: move down to the content section

https://codereview.chromium.org/344493002/diff/150001/content/public/browser/...
File content/public/browser/resource_dispatcher_host_delegate.h (right):

https://codereview.chromium.org/344493002/diff/150001/content/public/browser/...
content/public/browser/resource_dispatcher_host_delegate.h:19: class
AppCacheService;
ditto

https://codereview.chromium.org/344493002/diff/150001/content/public/browser/...
File content/public/browser/storage_partition.h (right):

https://codereview.chromium.org/344493002/diff/150001/content/public/browser/...
content/public/browser/storage_partition.h:15: namespace content {
ditto

https://codereview.chromium.org/344493002/diff/150001/content/public/common/a...
File content/public/common/appcache_info.h (right):

https://codereview.chromium.org/344493002/diff/150001/content/public/common/a...
content/public/common/appcache_info.h:22: APPCACHE_STATUS_UNCACHED,
nit: here and below use 2 space indentation

[pilgrim_google, 2014-06-27 19:26:23]

On 2014/06/25 21:51:14, jam wrote:
>
https://codereview.chromium.org/344493002/diff/150001/content/browser/appcach...
> File content/browser/appcache/appcache.h (right):
> 
>
https://codereview.chromium.org/344493002/diff/150001/content/browser/appcach...
> content/browser/appcache/appcache.h:26: FORWARD_DECLARE_TEST(AppCacheTest,
> InitializeWithManifest);
> i dont think you need these lines anymore? (also in other files)

They appear to still be required. (Removing them causes compilation errors.)

Working on the rest in a separate CL.

[pilgrim_google, 2014-06-27 19:33:32]

On 2014/06/27 19:26:23, pilgrim_google wrote:
> On 2014/06/25 21:51:14, jam wrote:
> >
> Working on the rest in a separate CL.

Other nits fixed in https://codereview.chromium.org/358993007/