Loading: move core/fetch to platform/loader/fetch

Loading: move core/fetch to platform/loader/fetch

Now that core/fetch does not have any unexpected dependency,
we can move it under platform.

BUG=655920
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2584423002
Cr-Commit-Position: refs/heads/master@{#445751}
Committed: https://chromium.googlesource.com/chromium/src/+/ecbfcdcef3fa7692d28b32aeb0c89e9a5e5c4ebc

[Takashi Toyoshima, 2016-12-19 12:22:53]

Description was changed from

==========
Loading: move core/fetch to platform/loader/fetch

Now that core/fetch does not have any unexpected dependency,
we can move it under platform.

BUG=655920
==========

to

==========
Loading: move core/fetch to platform/loader/fetch

Now that core/fetch does not have any unexpected dependency,
we can move it under platform.

BUG=655920
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[Takashi Toyoshima, 2016-12-19 12:25:45]

The CQ bit was checked by toyoshim@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-19 12:25:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Takashi Toyoshima, 2016-12-19 12:28:29]

Description was changed from

==========
Loading: move core/fetch to platform/loader/fetch

Now that core/fetch does not have any unexpected dependency,
we can move it under platform.

BUG=655920
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Loading: move core/fetch to platform/loader/fetch

Now that core/fetch does not have any unexpected dependency,
we can move it under platform.

BUG=655920
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[Takashi Toyoshima, 2016-12-19 12:28:30]

toyoshim@chromium.org changed reviewers:
+ haraken@chromium.org

[Takashi Toyoshima, 2016-12-19 12:30:35]

toyoshim@chromium.org changed reviewers:
+ brettw@chromium.org

[Takashi Toyoshima, 2016-12-19 12:36:12]

toyoshim@chromium.org changed reviewers:
+ yhirano@chromium.org

[Takashi Toyoshima, 2016-12-19 12:36:13]

brettw@, can you take a look?
This patch contains non-trivial BUILD.gn change to move core/fetch under
platform.

core/BUILD.gn - almost trivial, just remove core/fetch files
platform/BUILD.gn - almost trivial, just adding tests under fetch/
platform/loader/BUILD.gn - newly created. I'm afraid I forget or misunderstand
something here.

haraken@ for WebKit/
yhirano@ for core/fetch/ and platform/loader/

[commit-bot: I haz the power, 2016-12-19 12:58:13]

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

[commit-bot: I haz the power, 2016-12-19 12:58:16]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Takashi Toyoshima, 2016-12-19 13:11:14]

The CQ bit was checked by toyoshim@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-19 13:11:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-19 13:16:06]

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

[commit-bot: I haz the power, 2016-12-19 13:16:07]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[Takashi Toyoshima, 2016-12-20 06:35:48]

The CQ bit was checked by toyoshim@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-20 06:36:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-20 07:50:35]

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

[commit-bot: I haz the power, 2016-12-20 07:50:36]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Takashi Toyoshima, 2016-12-21 05:12:43]

brettw: ping. Now I notice that I need some modifications to run unit tests
inside platfork_blink_unittests environments, and am making separate CLs to
support it. But could you take an initial review for the build files? I need a
overview comment on BUILD.gn to confirm the direction is correct.

[Takashi Toyoshima, 2017-01-13 10:19:51]

toyoshim@chromium.org changed reviewers:
+ kouhei@chromium.org

[kinuko, 2017-01-16 05:25:07]

kinuko@chromium.org changed reviewers:
+ kinuko@chromium.org

[kinuko, 2017-01-16 05:25:09]

https://codereview.chromium.org/2584423002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/BUILD.gn (right):

https://codereview.chromium.org/2584423002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/BUILD.gn:1462:
"//third_party/WebKit/Source/platform/loader",
Would you be able to first just move all the files without making structural
changes in BUILD.gn (i.e. just add all files in this file) to make this CL
dedicated for simple file moving, and then create a separate CL only for
BUILD.gn changes?

[kinuko, 2017-01-16 05:27:00]

Also please make extra sure that all the stakeholders are happy with this bulk
file moving.

[brettw, 2017-01-16 22:50:29]

This looks fairly straightforward to me. Unless you're having specific problems,
I think a normal third_party/WebKit reviewer should be review it.

[Takashi Toyoshima, 2017-01-17 13:45:57]

toyoshim@chromium.org changed reviewers:
+ japhet@chromium.org, mkwst@chromium.org, tyoshino@chromium.org

[Takashi Toyoshima, 2017-01-17 13:51:48]

I added all OWNERS of core/fetch to reviewers list just in case.

I have submitted sub-patches to make loader unit tests work under platform, and
now I'm working on the last piece, ResourceFetcherTest. Also brettw@ says this
BUILD.gn change is straightforward thing. So, I think this main patch is ready
to start reviews by fetch/OWNERS.

https://codereview.chromium.org/2584423002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/BUILD.gn (right):

https://codereview.chromium.org/2584423002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/BUILD.gn:1462:
"//third_party/WebKit/Source/platform/loader",
Thanks. Since now brettw@ says this is straightforward change, let me keep this
as is.

brettw@
Thank you for replying. My concerns (you asked in chat) were
 - Is it reasonable to have a split BUILD.gn file under platform
 - Do I pick up necessary and sufficient import gnis in loader/BUILD.gn
correctly

[Yoav Weiss, 2017-01-17 14:36:34]

On 2017/01/17 13:51:48, toyoshim wrote:
> I added all OWNERS of core/fetch to reviewers list just in case.

I'd love to hear japhet@'s opinion on this change before landing (don't have a
strong opinion myself, but I know he did a lot of work on the loader/fetch
boundaries)

[brettw, 2017-01-17 21:19:38]

I like split build files (smaller things closer to code are better). It works if
the dependencies go one way (one component includes headers from the other, but
not vice-versa).

But if the dependencies aren't clear it can get tricky and it's probably worse
than keeping it all together. This happens if both components include headers
from the other. Then there's no clear ordering for compilation.

[yhirano, 2017-01-18 05:44:04]

Just in case, I'm fine with the overall direction.

[kouhei (in TOK), 2017-01-18 07:26:21]

LGTM 
I'm also on-board w/ this plan.

[tyoshino (SeeGerritForStatus), 2017-01-18 07:57:12]

me too fine with the refactoring.

[Mike West, 2017-01-18 10:24:33]

LGTM, FWIW.

[kinuko, 2017-01-18 13:28:49]

lgtm/2, fwiw.

[Nate Chapin, 2017-01-18 22:03:47]

On 2017/01/18 13:28:49, kinuko wrote:
> lgtm/2, fwiw.

I only skimmed the patch, but have another LGTM.

[Takashi Toyoshima, 2017-01-24 06:07:20]

Now the last sub-patch to make all tests work under platform was submitted.
I'd try submitting the patch set 5 via CQ.

[Takashi Toyoshima, 2017-01-24 06:22:48]

The CQ bit was checked by toyoshim@chromium.org

[Takashi Toyoshima, 2017-01-24 06:22:49]

The patchset sent to the CQ was uploaded after l-g-t-m from kinuko@chromium.org,
mkwst@chromium.org, kouhei@chromium.org, japhet@chromium.org
Link to the patchset: https://codereview.chromium.org/2584423002/#ps100001
(title: "FetchTestingPlatformSupport wasn't included")

[commit-bot: I haz the power, 2017-01-24 06:23:13]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-24 06:28:05]

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

[commit-bot: I haz the power, 2017-01-24 06:28:08]

Try jobs failed on following builders:
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[Takashi Toyoshima, 2017-01-24 07:02:49]

The CQ bit was checked by toyoshim@chromium.org

[Takashi Toyoshima, 2017-01-24 07:02:50]

The patchset sent to the CQ was uploaded after l-g-t-m from kinuko@chromium.org,
mkwst@chromium.org, kouhei@chromium.org, japhet@chromium.org
Link to the patchset: https://codereview.chromium.org/2584423002/#ps120001
(title: "rebase race")

[commit-bot: I haz the power, 2017-01-24 07:03:06]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-24 08:20:46]

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

[commit-bot: I haz the power, 2017-01-24 08:20:50]

Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[yhirano, 2017-01-24 08:25:57]

lgtm

[Takashi Toyoshima, 2017-01-24 08:45:38]

Hum... on linux_chromium_asan_rel_ng, all ResourceFetcherTest are failing with
the following check failure.
Is this bot running under a special isolated environment that can not allow
accessing to files for testing?

[971:971:0123/234353.815911:15152552263:FATAL:weburl_loader_mock_factory_impl.cc(48)]
Check failed: base::PathExists(response_info.file_path).
/b/s/w/irsYaUnC/third_party/WebKit/Source/web/tests/data/white-1x1.png does not
exist.

[Takashi Toyoshima, 2017-01-24 09:03:09]

The CQ bit was checked by toyoshim@chromium.org

[Takashi Toyoshima, 2017-01-24 09:03:10]

The patchset sent to the CQ was uploaded after l-g-t-m from kinuko@chromium.org,
mkwst@chromium.org, kouhei@chromium.org, japhet@chromium.org,
yhirano@chromium.org
Link to the patchset: https://codereview.chromium.org/2584423002/#ps140001
(title: "windows build fix")

[commit-bot: I haz the power, 2017-01-24 09:03:32]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Takashi Toyoshima, 2017-01-24 09:21:48]

Probably, I should include "web/tests/data/" in
group("blink_platform_unittests_data") to work it as is.
I will add this dependency in this CL, and create a followup CL that remove this
dependency (since it will need some non-mechanical code changes)

[Takashi Toyoshima, 2017-01-24 09:27:43]

The CQ bit was checked by toyoshim@chromium.org

[Takashi Toyoshima, 2017-01-24 09:27:44]

The patchset sent to the CQ was uploaded after l-g-t-m from kinuko@chromium.org,
mkwst@chromium.org, kouhei@chromium.org, japhet@chromium.org,
yhirano@chromium.org
Link to the patchset: https://codereview.chromium.org/2584423002/#ps160001
(title: "add test data dependency to BUILD.gn")

[commit-bot: I haz the power, 2017-01-24 09:27:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-24 10:10:38]

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

[commit-bot: I haz the power, 2017-01-24 10:10:42]

Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[Takashi Toyoshima, 2017-01-24 10:32:33]

The CQ bit was checked by toyoshim@chromium.org

[Takashi Toyoshima, 2017-01-24 10:32:34]

The patchset sent to the CQ was uploaded after l-g-t-m from kinuko@chromium.org,
mkwst@chromium.org, kouhei@chromium.org, japhet@chromium.org,
yhirano@chromium.org
Link to the patchset: https://codereview.chromium.org/2584423002/#ps180001
(title: "remove PLATFORM_EXPORT from test_support classes")

[commit-bot: I haz the power, 2017-01-24 10:33:04]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-24 11:29:13]

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

[commit-bot: I haz the power, 2017-01-24 11:29:16]

Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Takashi Toyoshima, 2017-01-24 11:30:09]

The CQ bit was checked by toyoshim@chromium.org

[Takashi Toyoshima, 2017-01-24 11:30:10]

The patchset sent to the CQ was uploaded after l-g-t-m from kinuko@chromium.org,
mkwst@chromium.org, kouhei@chromium.org, japhet@chromium.org,
yhirano@chromium.org
Link to the patchset: https://codereview.chromium.org/2584423002/#ps200001
(title: "another try")

[commit-bot: I haz the power, 2017-01-24 11:30:30]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-24 12:12:50]

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

[commit-bot: I haz the power, 2017-01-24 12:12:54]

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

[Takashi Toyoshima, 2017-01-24 13:22:18]

The CQ bit was checked by toyoshim@chromium.org

[commit-bot: I haz the power, 2017-01-24 13:22:33]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-24 14:02:05]

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

[commit-bot: I haz the power, 2017-01-24 14:02:09]

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

[Takashi Toyoshima, 2017-01-24 17:00:00]

The CQ bit was checked by toyoshim@chromium.org

[commit-bot: I haz the power, 2017-01-24 17:00:17]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-24 17:36:49]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1485277200119790,
"parent_rev": "6dbc00f485873cc326e3742f65fd7313d7af6aaa", "commit_rev":
"ecbfcdcef3fa7692d28b32aeb0c89e9a5e5c4ebc"}

[commit-bot: I haz the power, 2017-01-24 17:37:26]

Description was changed from

==========
Loading: move core/fetch to platform/loader/fetch

Now that core/fetch does not have any unexpected dependency,
we can move it under platform.

BUG=655920
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Loading: move core/fetch to platform/loader/fetch

Now that core/fetch does not have any unexpected dependency,
we can move it under platform.

BUG=655920
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2584423002
Cr-Commit-Position: refs/heads/master@{#445751}
Committed:
https://chromium.googlesource.com/chromium/src/+/ecbfcdcef3fa7692d28b32aeb0c8...
==========

[commit-bot: I haz the power, 2017-01-24 17:37:30]

Committed patchset #11 (id:200001) as
https://chromium.googlesource.com/chromium/src/+/ecbfcdcef3fa7692d28b32aeb0c8...