[Clean Skeleton] Migrate code to clean/

[Clean Skeleton] Migrate code to clean/

BUG=

Review-Url: https://codereview.chromium.org/2592983003
Cr-Commit-Position: refs/heads/master@{#443291}
Committed: https://chromium.googlesource.com/chromium/src/+/525d6e76600f29e2f724f29584129fec46236641

[marq (ping after 24h), 2016-12-21 18:34:25]

marq@chromium.org changed reviewers:
+ edchin@chromium.org, lpromero@chromium.org, rohitrao@chromium.org,
sdefresne@chromium.org

[marq (ping after 24h), 2016-12-21 18:34:26]

Still need to fix up some headers, but this is most of what's needed to shift
the new-arch-only stuff into clean/.

A follow-up CL will remove the clean_skeleton affix from source set names, strip
out the banner comments, and clean up README.md files.

[marq (ping after 24h), 2016-12-22 10:11:26]

Fix includes.

[marq (ping after 24h), 2016-12-22 13:28:12]

The CQ bit was checked by marq@chromium.org

[commit-bot: I haz the power, 2016-12-22 13:28:36]

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

[commit-bot: I haz the power, 2016-12-22 13:28:37]

This CL has an open dependency (Issue 2590933002 Patch 1). Please resolve the
dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[marq (ping after 24h), 2016-12-22 14:46:11]

The CQ bit was checked by marq@chromium.org

[commit-bot: I haz the power, 2016-12-22 14:46:27]

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-22 14:46:28]

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

[commit-bot: I haz the power, 2016-12-22 14:46:29]

No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[marq (ping after 24h), 2016-12-22 14:46:49]

The CQ bit was unchecked by marq@chromium.org

[marq (ping after 24h), 2016-12-22 14:47:15]

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

[commit-bot: I haz the power, 2016-12-22 14:47:35]

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-22 15:43:52]

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

[commit-bot: I haz the power, 2016-12-22 15:43:53]

Dry run: This issue passed the CQ dry run.

[marq (ping after 24h), 2017-01-09 09:26:19]

Ping on this; I will update the CL to include other code that has since landed,
but the longer we leave this transition, the more painful it gets.

[sdefresne, 2017-01-09 09:45:04]

The initial message said the CL was not ready for review yet and then there was
no followup message saying that it was ready.

Regarding the CL, I'd like if we could rename the targets to remove
_clean_skeleton suffix before landing this (except for chrome_clean_skeleton as
the suffix is necessary there to disambiguate the app name).

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/BU...
File ios/clean/chrome/app/BUILD.gn (right):

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/BU...
ios/clean/chrome/app/BUILD.gn:57: source_set("main_clean_skeleton") {
nit: you can remove _clean_skeleton suffix of all those target (except
chrome_clean_skeleton). It will be cleaner. So this one should be "main", next
one "application_state", ...

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/BU...
ios/clean/chrome/app/BUILD.gn:80: "application_phase.h",
Can those file move to ios/chrome/app/application_state?

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/ap...
File ios/clean/chrome/app/application_phase.h (right):

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/ap...
ios/clean/chrome/app/application_phase.h:9: #ifndef
IOS_CHROME_APP_APPLICATION_PHASE_H_
Incorrect header guard. The tool tools/git/move_source_files.py would have
updated it.

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/ap...
File ios/clean/chrome/app/application_step.h (right):

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/ap...
ios/clean/chrome/app/application_step.h:9: #ifndef
IOS_CHROME_APP_APPLICATION_STEP_H_
Incorrect include guard.

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/st...
File ios/clean/chrome/app/steps/BUILD.gn (right):

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/st...
ios/clean/chrome/app/steps/BUILD.gn:31:
"//ios/clean/chrome/browser:browser_clean_skeleton",
If you remove _clean_skeleton suffix from those targets, it would become
"//ios/clean/chrome/browser".

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/st...
File ios/clean/chrome/app/steps/README.md (right):

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/st...
ios/clean/chrome/app/steps/README.md:4: **The files in this directory are only
used in the new architecture for iOS
Move to //ios/clean maybe?

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/commands/tab_commands.h (right):

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/commands/tab_commands.h:18: @end
nit: blank line before #endif

[sdefresne, 2017-01-09 09:45:24]

lg once the target have been renamed and the include guard fixed

[lpromero, 2017-01-09 11:54:46]

lgtm

[marq (ping after 24h), 2017-01-09 17:38:07]

Thanks for the review. I'll land this soon unless anyone objects.

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/BU...
File ios/clean/chrome/app/BUILD.gn (right):

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/BU...
ios/clean/chrome/app/BUILD.gn:57: source_set("main_clean_skeleton") {
On 2017/01/09 09:45:04, sdefresne wrote:
> nit: you can remove _clean_skeleton suffix of all those target (except
> chrome_clean_skeleton). It will be cleaner. So this one should be "main", next
> one "application_state", ...

Done.

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/BU...
ios/clean/chrome/app/BUILD.gn:80: "application_phase.h",
On 2017/01/09 09:45:04, sdefresne wrote:
> Can those file move to ios/chrome/app/application_state?

They are new-architecture files and should move to ios/clean/chrome/app/; if
they should be refactored into a subdirectory I'd rather do that in a new CL.

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/ap...
File ios/clean/chrome/app/application_phase.h (right):

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/ap...
ios/clean/chrome/app/application_phase.h:9: #ifndef
IOS_CHROME_APP_APPLICATION_PHASE_H_
On 2017/01/09 09:45:04, sdefresne wrote:
> Incorrect header guard. The tool tools/git/move_source_files.py would have
> updated it.

And I foolishly used tools/git/mass-rename.py. Is there a reason we have two
similar, poorly-documented tools that do almost the same thing?

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/ap...
File ios/clean/chrome/app/application_step.h (right):

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/ap...
ios/clean/chrome/app/application_step.h:9: #ifndef
IOS_CHROME_APP_APPLICATION_STEP_H_
On 2017/01/09 09:45:04, sdefresne wrote:
> Incorrect include guard.

Done.

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/st...
File ios/clean/chrome/app/steps/BUILD.gn (right):

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/st...
ios/clean/chrome/app/steps/BUILD.gn:31:
"//ios/clean/chrome/browser:browser_clean_skeleton",
On 2017/01/09 09:45:04, sdefresne wrote:
> If you remove _clean_skeleton suffix from those targets, it would become
> "//ios/clean/chrome/browser".

Acknowledged.

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/st...
File ios/clean/chrome/app/steps/README.md (right):

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/st...
ios/clean/chrome/app/steps/README.md:4: **The files in this directory are only
used in the new architecture for iOS
On 2017/01/09 09:45:04, sdefresne wrote:
> Move to //ios/clean maybe?

Or just delete it.

[sdefresne, 2017-01-09 17:45:00]

lgtm

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/BU...
File ios/clean/chrome/app/BUILD.gn (right):

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/BU...
ios/clean/chrome/app/BUILD.gn:80: "application_phase.h",
On 2017/01/09 17:38:07, marq wrote:
> On 2017/01/09 09:45:04, sdefresne wrote:
> > Can those file move to ios/chrome/app/application_state?
> 
> They are new-architecture files and should move to ios/clean/chrome/app/; if
> they should be refactored into a subdirectory I'd rather do that in a new CL.

I meant ios/clean/chrome/app/application_state. Agreed for followup CL.

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/ap...
File ios/clean/chrome/app/application_phase.h (right):

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/ap...
ios/clean/chrome/app/application_phase.h:9: #ifndef
IOS_CHROME_APP_APPLICATION_PHASE_H_
On 2017/01/09 17:38:07, marq wrote:
> On 2017/01/09 09:45:04, sdefresne wrote:
> > Incorrect header guard. The tool tools/git/move_source_files.py would have
> > updated it.
> 
> And I foolishly used tools/git/mass-rename.py. Is there a reason we have two
> similar, poorly-documented tools that do almost the same thing?

Maybe one of them is implemented by using the other? (and does a mffr.py to then
change include guards + run sort-headers.py).

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/st...
File ios/clean/chrome/app/steps/README.md (right):

https://codereview.chromium.org/2592983003/diff/20001/ios/clean/chrome/app/st...
ios/clean/chrome/app/steps/README.md:4: **The files in this directory are only
used in the new architecture for iOS
On 2017/01/09 17:38:07, marq wrote:
> On 2017/01/09 09:45:04, sdefresne wrote:
> > Move to //ios/clean maybe?
> 
> Or just delete it.

I was talking about this comment:

 -----
 **The files in this directory are only used in the new architecture for iOS
 Chrome**
 -----

I think it could be moved to ios/clean/README.md and removed from all other
files below, WDYT? (completely optional, probably only worth doing in a followup
is ever).

[marq (ping after 24h), 2017-01-09 17:49:15]

Add top-level README

[marq (ping after 24h), 2017-01-09 18:21:50]

The CQ bit was checked by marq@chromium.org

[marq (ping after 24h), 2017-01-09 18:21:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
lpromero@chromium.org, sdefresne@chromium.org
Link to the patchset: https://codereview.chromium.org/2592983003/#ps80001
(title: "Add top-level README")

[commit-bot: I haz the power, 2017-01-09 18:22: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-09 18:30:49]

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

[commit-bot: I haz the power, 2017-01-09 18:30:51]

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

[marq (ping after 24h), 2017-01-10 09:42:29]

fix another include.

[marq (ping after 24h), 2017-01-10 09:42:59]

The CQ bit was checked by marq@chromium.org

[marq (ping after 24h), 2017-01-10 09:43:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
lpromero@chromium.org, sdefresne@chromium.org
Link to the patchset: https://codereview.chromium.org/2592983003/#ps100001
(title: "fix another include.")

[commit-bot: I haz the power, 2017-01-10 09:43:22]

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-10 09:51:19]

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

[commit-bot: I haz the power, 2017-01-10 09:51:21]

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

[marq (ping after 24h), 2017-01-10 12:15:02]

Minimal DEPS.

[marq (ping after 24h), 2017-01-10 12:30:33]

The CQ bit was checked by marq@chromium.org

[marq (ping after 24h), 2017-01-10 12:30:34]

The patchset sent to the CQ was uploaded after l-g-t-m from
lpromero@chromium.org, sdefresne@chromium.org
Link to the patchset: https://codereview.chromium.org/2592983003/#ps120001
(title: "Minimal DEPS.")

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

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-10 12:38:29]

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

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

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

[marq (ping after 24h), 2017-01-10 13:04:54]

marq@chromium.org changed reviewers:
+ mark@chromium.org

[marq (ping after 24h), 2017-01-10 13:04:56]

+mark@ for ios/clean/DEPS (all dependencies there already exist in
ios/chrome/DEPS).

[Mark Mentovai, 2017-01-10 22:21:22]

LGTM

[marq (ping after 24h), 2017-01-11 07:27:27]

The CQ bit was checked by marq@chromium.org

[commit-bot: I haz the power, 2017-01-11 07:27:49]

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

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

[commit-bot: I haz the power, 2017-01-11 07:36:10]

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

[marq (ping after 24h), 2017-01-11 09:03:09]

marq@chromium.org changed reviewers:
+ juliatuttle@chromium.org, thakis@chromium.org

[marq (ping after 24h), 2017-01-11 09:03:10]

Reviewers added for ios/clean/DEPS, which duplicates some existing
ios/chrome/DEPS dependencies:

+juliatuttle for net
+thakis for ui

[Nico, 2017-01-11 15:48:33]

ios/clean/DEPS lgtm

[Julia Tuttle, 2017-01-12 15:12:17]

+net in ios/clean/DEPS lgtm

[marq (ping after 24h), 2017-01-12 15:19:25]

The CQ bit was checked by marq@chromium.org

[marq (ping after 24h), 2017-01-12 15:19:26]

The patchset sent to the CQ was uploaded after l-g-t-m from mark@chromium.org,
lpromero@chromium.org, sdefresne@chromium.org
Link to the patchset: https://codereview.chromium.org/2592983003/#ps140001
(title: "More DEPS pruning.")

[commit-bot: I haz the power, 2017-01-12 15:19:43]

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-12 16:21:37]

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

[commit-bot: I haz the power, 2017-01-12 16:21:40]

Failed to apply patch for ios/showcase/BUILD.gn:
While running git apply --index -p1;
  error: patch failed: ios/showcase/BUILD.gn:25
  error: ios/showcase/BUILD.gn: patch does not apply

Patch:       ios/showcase/BUILD.gn
Index: ios/showcase/BUILD.gn
diff --git a/ios/showcase/BUILD.gn b/ios/showcase/BUILD.gn
index
ff70d5fde9232f84591cf767451d13ae20837b13..c722e136bfe72995074ea1b78b3a15e90ae76755
100644
--- a/ios/showcase/BUILD.gn
+++ b/ios/showcase/BUILD.gn
@@ -25,7 +25,7 @@ ios_app_bundle("showcase") {
 
 group("features") {
   deps = [
-    "//ios/chrome/browser/ui/tools:tools_ui",
+    "//ios/clean/chrome/browser/ui/tools:tools_ui",
     "//ios/showcase/settings",
     "//ios/showcase/strip",
     "//ios/showcase/tab_grid",

[marq (ping after 24h), 2017-01-12 17:05:03]

Rebased

[marq (ping after 24h), 2017-01-12 17:05:04]

The CQ bit was checked by marq@chromium.org

[marq (ping after 24h), 2017-01-12 17:05:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
lpromero@chromium.org, mark@chromium.org, juliatuttle@chromium.org,
thakis@chromium.org, sdefresne@chromium.org
Link to the patchset: https://codereview.chromium.org/2592983003/#ps160001
(title: "Rebased")

[commit-bot: I haz the power, 2017-01-12 17:05:34]

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-12 18:15:05]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1484240704460920,
"parent_rev": "65d726a3d10b3a55ea563fb7089a34f129cd95f2", "commit_rev":
"525d6e76600f29e2f724f29584129fec46236641"}

[commit-bot: I haz the power, 2017-01-12 18:15:37]

Description was changed from

==========
[Clean Skeleton] Migrate code to clean/

BUG=
==========

to

==========
[Clean Skeleton] Migrate code to clean/

BUG=

Review-Url: https://codereview.chromium.org/2592983003
Cr-Commit-Position: refs/heads/master@{#443291}
Committed:
https://chromium.googlesource.com/chromium/src/+/525d6e76600f29e2f724f2958412...
==========

[commit-bot: I haz the power, 2017-01-12 18:15:39]

Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/525d6e76600f29e2f724f2958412...