Move all components/offline_pages/ files into component/offline_pages/core

Move all components/offline_pages/ files into component/offline_pages/core

We are adding some additional dependencies to content, with background loader.
We want to pre-emptively layer our component to make dependencies clear and to
allow easier porting of code to other OS

BUG=654635
Committed: https://crrev.com/ecd23804bb3f965dcfd344f2e0282fba18e1bb78
Cr-Commit-Position: refs/heads/master@{#436907}

[chili, 2016-11-17 01:00:29]

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

[commit-bot: I haz the power, 2016-11-17 01:03:02]

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-11-17 01:11:27]

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

[commit-bot: I haz the power, 2016-11-17 01:11:28]

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

[chili, 2016-11-17 19:21:07]

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

[commit-bot: I haz the power, 2016-11-17 19:22:22]

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

[chili, 2016-11-17 19:24:53]

chili@chromium.org changed reviewers:
+ carlosk@chromium.org, dewittj@chromium.org, dimich@chromium.org,
dougarnett@chromium.org, fgorski@chromium.org, jianli@chromium.org,
petewil@chromium.org, romax@chromium.org

[chili, 2016-11-17 19:24:56]

Adding everyone on the team as heads up - you may need to do some merges :)

Both tests and regular apk compiles on my local machine, but might not compile
on try bots if there are merge conflicts.

Will add the OWNERs/DEPs people once you guys give the OK.

Thanks!

[chili, 2016-11-17 19:28:40]

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

[commit-bot: I haz the power, 2016-11-17 19:29:39]

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

[Dmitry Titov, 2016-11-17 19:35:50]

lgtm if it passes bots :)

[commit-bot: I haz the power, 2016-11-17 19:56:48]

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

[commit-bot: I haz the power, 2016-11-17 19:56:50]

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

[chili, 2016-11-17 22:42:04]

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

[commit-bot: I haz the power, 2016-11-17 22:43:14]

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-11-17 22:52:11]

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

[commit-bot: I haz the power, 2016-11-17 22:52:15]

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

[jianli, 2016-11-18 00:07:08]

Thanks for updating this. Given that the branch point is so close, can you hold
off landing this until some days after that? I am afraid that some people might
it very difficult to merge things back to M56.

[Pete Williamson, 2016-11-18 00:23:47]

mostly looks good, a few questions.

https://codereview.chromium.org/2489443002/diff/30001/components/BUILD.gn
File components/BUILD.gn (left):

https://codereview.chromium.org/2489443002/diff/30001/components/BUILD.gn#old...
components/BUILD.gn:102: "//components/offline_pages:unit_tests",
Where did unit_tests go?  is it now core:unit_tests?

https://codereview.chromium.org/2489443002/diff/30001/components/offline_page...
File components/offline_pages/core/DEPS (right):

https://codereview.chromium.org/2489443002/diff/30001/components/offline_page...
components/offline_pages/core/DEPS:5: "+sql",
Does this need updating now that a number of files have moved here? (I'm
thinking they might have dependencies of their own that need to be reflected
here).

[chili, 2016-11-18 00:38:40]

https://codereview.chromium.org/2489443002/diff/30001/components/BUILD.gn
File components/BUILD.gn (left):

https://codereview.chromium.org/2489443002/diff/30001/components/BUILD.gn#old...
components/BUILD.gn:102: "//components/offline_pages:unit_tests",
On 2016/11/18 00:23:47, Pete Williamson wrote:
> Where did unit_tests go?  is it now core:unit_tests?  

yup, it got merged into core:unit_tests which originally contained unit tests
for tasks, but now contains everything

[chili, 2016-11-19 00:24:00]

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

[commit-bot: I haz the power, 2016-11-19 00:24:47]

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-11-19 00:48:50]

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

[commit-bot: I haz the power, 2016-11-19 00:48:51]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[chili, 2016-11-26 02:14:33]

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

[commit-bot: I haz the power, 2016-11-26 02:14:39]

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-11-26 02:39:45]

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

[commit-bot: I haz the power, 2016-11-26 02:39:46]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[chili, 2016-11-26 03:59:43]

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

[commit-bot: I haz the power, 2016-11-26 03:59:54]

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-11-26 05:37:46]

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

[commit-bot: I haz the power, 2016-11-26 05:37:48]

Dry run: This issue passed the CQ dry run.

[chili, 2016-12-06 00:40:26]

chili@chromium.org changed reviewers:
+ dbeam@chromium.org, tedchoc@chromium.org, treib@chromium.org

[chili, 2016-12-06 00:40:27]

+tedchoc for chrome/android includes updates
+dbeam for webui includes updates
+treib for ntp_snippets

There's a lot of OWNERS to add. sorry for +owners spam in advance

[Ted C, 2016-12-06 01:11:17]

On 2016/12/06 00:40:27, chili wrote:
> +tedchoc for chrome/android includes updates
> +dbeam for webui includes updates
> +treib for ntp_snippets
> 
> There's a lot of OWNERS to add. sorry for +owners spam in advance

chrome/android - lgtm

[chili, 2016-12-06 01:39:09]

chili@chromium.org changed reviewers:
+ cpu@chromium.org, sdefresne@chromium.org

[chili, 2016-12-06 01:39:10]

+sdefresne for components/error_page, components/ntp_snippets,
components/BUILD.gn

+cpu for any left-over files:
chrome/browser/browsing_data/*
chrome/browser/net/
chrome/browser/previews
chrome/test/base/testing_profile

[chili, 2016-12-06 01:44:17]

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

[commit-bot: I haz the power, 2016-12-06 01:44:56]

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-06 01:50:39]

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

[commit-bot: I haz the power, 2016-12-06 01:50:41]

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

[Pete Williamson, 2016-12-06 02:02:24]

I feel a bit conflicted about my suggestions for overriding the git cl format
results.  On one hand, it is pretty ugly, on the other hand, if we aren't
careful, the next edit will change it back.

Overall looks good, just a few things.

https://codereview.chromium.org/2489443002/diff/60001/components/offline_page...
File components/offline_pages/core/DEPS (right):

https://codereview.chromium.org/2489443002/diff/60001/components/offline_page...
components/offline_pages/core/DEPS:5: "+sql",
This file doesn't seem to have changed.  Should it have been?  If not, you can
remove it from the changelist.

https://codereview.chromium.org/2489443002/diff/60001/components/offline_page...
File components/offline_pages/core/background/BUILD.gn (right):

https://codereview.chromium.org/2489443002/diff/60001/components/offline_page...
components/offline_pages/core/background/BUILD.gn:53:
"//components/offline_pages/core:switches",
Does the background component still need switches?  We may have removed the
switch, and forgotten to remove this dependency back when DougArnett removed
some switch checks.

https://codereview.chromium.org/2489443002/diff/60001/components/offline_page...
File components/offline_pages/core/background/DEPS (right):

https://codereview.chromium.org/2489443002/diff/60001/components/offline_page...
components/offline_pages/core/background/DEPS:2: "+sql",
I don't see any changes here either.

https://codereview.chromium.org/2489443002/diff/60001/components/offline_page...
File components/offline_pages/core/background/save_page_request.cc (right):

https://codereview.chromium.org/2489443002/diff/60001/components/offline_page...
components/offline_pages/core/background/save_page_request.cc:55: return
request_id_ == other.request_id_ && url_ == other.url_ &&
I know you probably got this from git cl format, but I think it was more
readable the way it was before with exactly one condition per line.

https://codereview.chromium.org/2489443002/diff/60001/components/offline_page...
File components/offline_pages/core/offline_page_item.cc (right):

https://codereview.chromium.org/2489443002/diff/60001/components/offline_page...
components/offline_pages/core/offline_page_item.cc:62: return url == other.url
&& offline_id == other.offline_id &&
I think this was also easier to read before the git cl format.

https://codereview.chromium.org/2489443002/diff/60001/components/offline_page...
File components/offline_pages/core/request_header/BUILD.gn (right):

https://codereview.chromium.org/2489443002/diff/60001/components/offline_page...
components/offline_pages/core/request_header/BUILD.gn:27: ":request_header",
I don't see any changes here.

[Marc Treib, 2016-12-06 09:34:46]

components/ntp_snippets LGTM

[dougarnett, 2016-12-06 17:37:24]

lgtm

[chili, 2016-12-06 19:08:54]

https://codereview.chromium.org/2489443002/diff/60001/components/offline_page...
File components/offline_pages/core/DEPS (right):

https://codereview.chromium.org/2489443002/diff/60001/components/offline_page...
components/offline_pages/core/DEPS:5: "+sql",
On 2016/12/06 02:02:24, Pete Williamson wrote:
> This file doesn't seem to have changed.  Should it have been?  If not, you can
> remove it from the changelist.

This hasn't changed because the diff is between the old version (at
components/offline_pages/DEPS) to current (components/offline_pages/core/DEPS) I
think

https://codereview.chromium.org/2489443002/diff/60001/components/offline_page...
File components/offline_pages/core/background/BUILD.gn (right):

https://codereview.chromium.org/2489443002/diff/60001/components/offline_page...
components/offline_pages/core/background/BUILD.gn:53:
"//components/offline_pages/core:switches",
On 2016/12/06 02:02:24, Pete Williamson wrote:
> Does the background component still need switches?  We may have removed the
> switch, and forgotten to remove this dependency back when DougArnett removed
> some switch checks.

Not sure, but I'd rather keep everything the same for this CL and do dependency
pruning later.  Every single rebase-update causes a few non-compiles, and I'd be
more confident knowing it's because some include didn't get updated properly as
opposed to someone adding a file that uses the deps i just removed.

https://codereview.chromium.org/2489443002/diff/60001/components/offline_page...
File components/offline_pages/core/background/DEPS (right):

https://codereview.chromium.org/2489443002/diff/60001/components/offline_page...
components/offline_pages/core/background/DEPS:2: "+sql",
On 2016/12/06 02:02:24, Pete Williamson wrote:
> I don't see any changes here either.

same as above - the file moved, but the diff is showing the old file vs the
moved file

https://codereview.chromium.org/2489443002/diff/60001/components/offline_page...
File components/offline_pages/core/request_header/BUILD.gn (right):

https://codereview.chromium.org/2489443002/diff/60001/components/offline_page...
components/offline_pages/core/request_header/BUILD.gn:27: ":request_header",
On 2016/12/06 02:02:24, Pete Williamson wrote:
> I don't see any changes here.

ditto on expectedness

[Pete Williamson, 2016-12-06 19:33:03]

lgtm

[dougarnett, 2016-12-06 20:10:13]

https://codereview.chromium.org/2489443002/diff/60001/components/offline_page...
File components/offline_pages/core/background/BUILD.gn (right):

https://codereview.chromium.org/2489443002/diff/60001/components/offline_page...
components/offline_pages/core/background/BUILD.gn:53:
"//components/offline_pages/core:switches",
On 2016/12/06 19:08:53, chili wrote:
> On 2016/12/06 02:02:24, Pete Williamson wrote:
> > Does the background component still need switches?  We may have removed the
> > switch, and forgotten to remove this dependency back when DougArnett removed
> > some switch checks.
> 
> Not sure, but I'd rather keep everything the same for this CL and do
dependency
> pruning later.  Every single rebase-update causes a few non-compiles, and I'd
be
> more confident knowing it's because some include didn't get updated properly
as
> opposed to someone adding a file that uses the deps i just removed.

Believe this is needed for
offline_pages::IsOfflinePagesSvelteConcurrentLoadingEnabled() used in
Coordinator.

[cpu_(ooo_6.6-7.5), 2016-12-06 20:50:20]

rubber-stamp lgtm on the overall move.

[chili, 2016-12-07 03:50:03]

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

[commit-bot: I haz the power, 2016-12-07 03:50:29]

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-07 04:05:46]

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

[commit-bot: I haz the power, 2016-12-07 04:05:48]

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-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[chili, 2016-12-07 07:23:31]

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

[commit-bot: I haz the power, 2016-12-07 07:23:54]

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-07 07:36:44]

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

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

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_...)

[chili, 2016-12-07 07:53:21]

Patchset #9 (id:80001) has been deleted

[chili, 2016-12-07 08:32:50]

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

[commit-bot: I haz the power, 2016-12-07 08:33:02]

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-07 10:05:00]

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

[commit-bot: I haz the power, 2016-12-07 10:05:04]

Dry run: This issue passed the CQ dry run.

[chili, 2016-12-07 10:17:28]

The CQ bit was checked by chili@chromium.org

[chili, 2016-12-07 10:17:29]

The patchset sent to the CQ was uploaded after l-g-t-m from dimich@chromium.org,
tedchoc@chromium.org, cpu@chromium.org, petewil@chromium.org,
treib@chromium.org, dougarnett@chromium.org
Link to the patchset: https://codereview.chromium.org/2489443002/#ps100001
(title: "rebase")

[commit-bot: I haz the power, 2016-12-07 10:17:48]

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-07 10:27:16]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1481105848077410,
"parent_rev": "754e8e8c4594e7586d2423e798f3f152a117b7f3", "commit_rev":
"65e7bb3b4ced9490b2f7c42dd52b44e54ba892e5"}

[commit-bot: I haz the power, 2016-12-07 10:27:44]

Committed patchset #10 (id:100001)

[commit-bot: I haz the power, 2016-12-07 10:30:41]

Description was changed from

==========
Move all components/offline_pages/ files into component/offline_pages/core

We are adding some additional dependencies to content, with background loader.
We want to pre-emptively layer our component to make dependencies clear and to
allow easier porting of code to other OS

BUG=654635
==========

to

==========
Move all components/offline_pages/ files into component/offline_pages/core

We are adding some additional dependencies to content, with background loader.
We want to pre-emptively layer our component to make dependencies clear and to
allow easier porting of code to other OS

BUG=654635

Committed: https://crrev.com/ecd23804bb3f965dcfd344f2e0282fba18e1bb78
Cr-Commit-Position: refs/heads/master@{#436907}
==========

[commit-bot: I haz the power, 2016-12-07 10:30:44]

Patchset 10 (id:??) landed as
https://crrev.com/ecd23804bb3f965dcfd344f2e0282fba18e1bb78
Cr-Commit-Position: refs/heads/master@{#436907}