Move ServiceWorker public headers to public/{web,platform}/modules/serviceworker (3/3)

Move ServiceWorker public headers to public/{web,platform}/modules/serviceworker (3/3)

Three-sided patch:
[1] (blink) https://codereview.chromium.org/1262943002/
[2] (chromium) https://codereview.chromium.org/1254023006/
[3] (blink) THIS PATCH

BUG=453032

[kinuko, 2015-08-14 10:15:34]

kinuko@chromium.org changed reviewers:
+ falken@chromium.org, tkent@chromium.org

[kinuko, 2015-08-14 10:15:59]

Mechanical header change only. PTL

[tkent, 2015-08-17 00:37:20]

lgtm

[falken, 2015-08-17 08:40:49]

lgtm

https://codereview.chromium.org/1262603004/diff/40001/Source/modules/cachesto...
File Source/modules/cachestorage/CacheTest.cpp (right):

https://codereview.chromium.org/1262603004/diff/40001/Source/modules/cachesto...
Source/modules/cachestorage/CacheTest.cpp:23: #include
"public/platform/modules/serviceworker/WebServiceWorkerCache.h"
maybe modules before Web* (same for many places throughout this patch)

https://codereview.chromium.org/1262603004/diff/40001/public/platform/modules...
File public/platform/modules/serviceworker/WebServiceWorkerResponseType.h
(right):

https://codereview.chromium.org/1262603004/diff/40001/public/platform/modules...
public/platform/modules/serviceworker/WebServiceWorkerResponseType.h:17:
WebServiceWorkerResponseTypeLast = WebServiceWorkerResponseTypeOpaqueRedirect
this looks from a different patch?

[kinuko, 2015-08-19 09:22:00]

The CQ bit was checked by kinuko@chromium.org

[kinuko, 2015-08-19 09:22:00]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org,
falken@chromium.org
Link to the patchset: https://codereview.chromium.org/1262603004/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2015-08-19 09:22:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1262603004/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1262603004/60001

[commit-bot: I haz the power, 2015-08-19 09:57:38]

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

[commit-bot: I haz the power, 2015-08-19 09:57:39]

Try jobs failed on following builders:
  linux_chromium_gn_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[kinuko, 2015-08-19 14:47:22]

I've been having hard time making this build on all bots and I would like
tkent-san to take one more look.  Let me explain what's happening and what I
did:

Existing condition (before this patch):
* public/web/WebContextMenuData.h includes "../platform/WebURLResponse.h"
* public/platform/WebURLResponse.h includes "WebServiceWorkerResponseType.h" ...
[1]

This patch tries to:
* move "public/platform/WebServiceWorkerResponseType.h" to
public/platform/modules/serviceworker/

This means that "WebServiceWorkerResponseType.h" include path in
WebURLResponse.h ([1]) needs to be updated, however it doesn't really work in a
straightforward way. (This is related to why I did
https://codereview.chromium.org/1294803005/)

What I tried:
1. Change "WebServiceWorkerResponseType.h" include in WebURLResponse.h to
"public/platform/modules/serviceworker/..." --> this sould work but breaks
builds.  (PS4)
2. Change "WebServiceWorkerResponseType.h" include in WebURLResponse.h to a
relative path, i.e. "modules/serviceworker/WebServiceWorkerResponseType.h" -->
doesn't work because it's not forbidden by include_rules (as it's ambigious with
Source/modules)
3. Do 1. and also change "../platform/WebURLResponse.h" include in
public/web/WebContextMenuData.h to "public/platform/WebURLResponse.h" --> breaks
build.  (PS5)
4. Change "WebServiceWorkerResponseType.h" include in WebURLResponse.h to
"../platform/modules/serviceworker/WebServiceWorkerResponseType.h" --> doesn't
work because it's not allowed by include_rules

I'm stuck, so I did following in PS6 and it seems to build, finally:
4'. Do 4 and also add '../platform' to include_rules.

Kent-san: would this feel ok to you? Could you take one more look at
public/platform/DEPS and public/platform/WebURLResponse.h changes?

[tkent, 2015-08-20 00:25:01]

On 2015/08/19 14:47:22, kinuko wrote:
> 1. Change "WebServiceWorkerResponseType.h" include in WebURLResponse.h to
> "public/platform/modules/serviceworker/..." --> this sould work but breaks
> builds.  (PS4)

This is a right direction.  Broken build was due to missing dependency to Blink
in Chromium gyp/BUILD.gn.

> 2. Change "WebServiceWorkerResponseType.h" include in WebURLResponse.h to a
> relative path, i.e. "modules/serviceworker/WebServiceWorkerResponseType.h" -->
> doesn't work because it's not forbidden by include_rules (as it's ambigious
with
> Source/modules)

Yeah, we shouldn't use paths relative to the origin file.

> 3. Do 1. and also change "../platform/WebURLResponse.h" include in
> public/web/WebContextMenuData.h to "public/platform/WebURLResponse.h" -->
breaks
> build.  (PS5)

This is also right.  We should add Blink dependency to gyp/BUILD.gn in Chromium.

> 4. Change "WebServiceWorkerResponseType.h" include in WebURLResponse.h to
> "../platform/modules/serviceworker/WebServiceWorkerResponseType.h" --> doesn't
> work because it's not allowed by include_rules

We don't want to add relative path includes.

> I'm stuck, so I did following in PS6 and it seems to build, finally:
> 4'. Do 4 and also add '../platform' to include_rules.

I don't think we need to hurry to land this CL.  So, please do a right thing. 
i.e. fix dependency rules in Chromium .gyp and BUILD.gn.

[kinuko, 2015-08-20 01:34:15]

On 2015/08/20 00:25:01, tkent wrote:
> On 2015/08/19 14:47:22, kinuko wrote:
> > 1. Change "WebServiceWorkerResponseType.h" include in WebURLResponse.h to
> > "public/platform/modules/serviceworker/..." --> this sould work but breaks
> > builds.  (PS4)
> 
> This is a right direction.  Broken build was due to missing dependency to
Blink
> in Chromium gyp/BUILD.gn.
> 
> > 2. Change "WebServiceWorkerResponseType.h" include in WebURLResponse.h to a
> > relative path, i.e. "modules/serviceworker/WebServiceWorkerResponseType.h"
-->
> > doesn't work because it's not forbidden by include_rules (as it's ambigious
> with
> > Source/modules)
> 
> Yeah, we shouldn't use paths relative to the origin file.
> 
> > 3. Do 1. and also change "../platform/WebURLResponse.h" include in
> > public/web/WebContextMenuData.h to "public/platform/WebURLResponse.h" -->
> breaks
> > build.  (PS5)
> 
> This is also right.  We should add Blink dependency to gyp/BUILD.gn in
Chromium.
> 
> > 4. Change "WebServiceWorkerResponseType.h" include in WebURLResponse.h to
> > "../platform/modules/serviceworker/WebServiceWorkerResponseType.h" -->
doesn't
> > work because it's not allowed by include_rules
> 
> We don't want to add relative path includes.
> 
> > I'm stuck, so I did following in PS6 and it seems to build, finally:
> > 4'. Do 4 and also add '../platform' to include_rules.
> 
> I don't think we need to hurry to land this CL.  So, please do a right thing. 
> i.e. fix dependency rules in Chromium .gyp and BUILD.gn.

Sounds right. Ok I will try fixing the dependency rules first and come back to
this CL later. Thanks!

[kinuko, 2015-10-01 13:46:12]

On 2015/08/20 01:34:15, kinuko wrote:
> On 2015/08/20 00:25:01, tkent wrote:
> > On 2015/08/19 14:47:22, kinuko wrote:
> > > 1. Change "WebServiceWorkerResponseType.h" include in WebURLResponse.h to
> > > "public/platform/modules/serviceworker/..." --> this sould work but breaks
> > > builds.  (PS4)
> > 
> > This is a right direction.  Broken build was due to missing dependency to
> Blink
> > in Chromium gyp/BUILD.gn.
> > 
> > > 2. Change "WebServiceWorkerResponseType.h" include in WebURLResponse.h to
a
> > > relative path, i.e. "modules/serviceworker/WebServiceWorkerResponseType.h"
> -->
> > > doesn't work because it's not forbidden by include_rules (as it's
ambigious
> > with
> > > Source/modules)
> > 
> > Yeah, we shouldn't use paths relative to the origin file.
> > 
> > > 3. Do 1. and also change "../platform/WebURLResponse.h" include in
> > > public/web/WebContextMenuData.h to "public/platform/WebURLResponse.h" -->
> > breaks
> > > build.  (PS5)
> > 
> > This is also right.  We should add Blink dependency to gyp/BUILD.gn in
> Chromium.
> > 
> > > 4. Change "WebServiceWorkerResponseType.h" include in WebURLResponse.h to
> > > "../platform/modules/serviceworker/WebServiceWorkerResponseType.h" -->
> doesn't
> > > work because it's not allowed by include_rules
> > 
> > We don't want to add relative path includes.
> > 
> > > I'm stuck, so I did following in PS6 and it seems to build, finally:
> > > 4'. Do 4 and also add '../platform' to include_rules.
> > 
> > I don't think we need to hurry to land this CL.  So, please do a right
thing. 
> > i.e. fix dependency rules in Chromium .gyp and BUILD.gn.
> 
> Sounds right. Ok I will try fixing the dependency rules first and come back to
> this CL later. Thanks!

The dependency change is landed, and I've re-uploaded this patch here:
https://codereview.chromium.org/1373693002/

Closing this now.