[Sync] Move sessions/ to engine/cycle/ and rename things to match.

[Sync] Move sessions/ to engine/cycle/ and rename things to match.

- Move everything in sessions/ to engine/cycle/.
- Move everything in sessions_impl/ to engine_impl/cycle/.
- Rename SyncSession* to SyncCycle* (within engine).
- Remove the syncer::sessions:: namespace entirely.
- Clean up deps.

BUG=637950
TBR=thestig,noyau

Committed: https://crrev.com/3067278a4670c40ad786742bbd47bacc36a5c1b3
Cr-Commit-Position: refs/heads/master@{#413278}

[maxbogue, 2016-08-19 00:37:29]

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

[commit-bot: I haz the power, 2016-08-19 00:38:19]

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-08-19 02:09:57]

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

[commit-bot: I haz the power, 2016-08-19 02:09:58]

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

[maxbogue, 2016-08-19 05:09:48]

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

[commit-bot: I haz the power, 2016-08-19 05:10:01]

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-08-19 05:17:22]

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

[commit-bot: I haz the power, 2016-08-19 05:17:23]

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

[maxbogue, 2016-08-19 05:28:44]

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

[commit-bot: I haz the power, 2016-08-19 05:29:00]

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

[maxbogue, 2016-08-19 06:03:58]

maxbogue@chromium.org changed reviewers:
+ skym@chromium.org

[maxbogue, 2016-08-19 06:04:00]

Sky, PTAL! I needed a break from DTC thread madness...

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
File components/sync/engine_impl/model_type_registry_unittest.cc (right):

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
components/sync/engine_impl/model_type_registry_unittest.cc:233: // Add cycles
non-blocking type.
This one's gotta be changed back.

[commit-bot: I haz the power, 2016-08-19 06:43:57]

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

[commit-bot: I haz the power, 2016-08-19 06:43:58]

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

[skym, 2016-08-19 17:17:43]

lgtm

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
File components/sync/engine_impl/commit_util.cc (right):

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
components/sync/engine_impl/commit_util.cc:19: #include
"components/sync/engine_impl/cycle/sync_cycle.h"
I don't think this import actually does anything, you should be able to remove.

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
components/sync/engine_impl/commit_util.cc:43: void
AddExtensionsActivityToMessage(
Was this like required for stars? And we can rip out all of this? Or is this
like a debugging feature that I just don't know how to use? (I'm mostly just
curious, way out of scope for this CL.)

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
File components/sync/engine_impl/cycle/directory_type_debug_info_emitter.h
(right):

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
components/sync/engine_impl/cycle/directory_type_debug_info_emitter.h:18: 
And not everything in sync/sessions/ even had the sessions namespace! Ahhh!

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
File components/sync/engine_impl/cycle/sync_cycle_context.h (right):

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
components/sync/engine_impl/cycle/sync_cycle_context.h:33: class
TestScopedSessionEventListener;
Shouldn't need this forward declaration to friend class this thing. Also, where
is it now defined? And why did it keep 'Session' ?

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
components/sync/engine_impl/cycle/sync_cycle_context.h:127: friend class
TestScopedSessionEventListener;
As far as I can tell, you deleted this class. So no need to friend it anymore.

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
File components/sync/engine_impl/syncer_unittest.cc (right):

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
components/sync/engine_impl/syncer_unittest.cc:213: last_cycles_commit_delay_ =
iter->second;
I think this is suppose to be session

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
components/sync/engine_impl/syncer_unittest.cc:253: void ResetSession() {
cycle_.reset(SyncCycle::Build(context_.get(), this)); }
This should be ResetCycle()

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
components/sync/engine_impl/syncer_unittest.cc:581: base::TimeDelta
last_cycles_commit_delay_;
Should be session

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
File components/sync/engine_impl/update_applicator.h (right):

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
components/sync/engine_impl/update_applicator.h:21: #include
"components/sync/engine_impl/cycle/status_controller.h"
I don't think this is used in this file, but seems to cause compile build errors
if I remove it. I think you should probably move this to the actually required
files instead.

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
components/sync/engine_impl/update_applicator.h:26: class ConflictResolver;
Where is this used? Remove?

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
components/sync/engine_impl/update_applicator.h:28: class StatusController;
This is also getting #included, definitely don't need to forward declare it.

[maxbogue, 2016-08-19 18:02:23]

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

[commit-bot: I haz the power, 2016-08-19 18:03:08]

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

[maxbogue, 2016-08-19 19:12:01]

Thanks for the quick review!

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
File components/sync/engine_impl/commit_util.cc (right):

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
components/sync/engine_impl/commit_util.cc:19: #include
"components/sync/engine_impl/cycle/sync_cycle.h"
On 2016/08/19 17:17:42, skym wrote:
> I don't think this import actually does anything, you should be able to
remove.

Removed.

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
components/sync/engine_impl/commit_util.cc:43: void
AddExtensionsActivityToMessage(
On 2016/08/19 17:17:42, skym wrote:
> Was this like required for stars? And we can rip out all of this? Or is this
> like a debugging feature that I just don't know how to use? (I'm mostly just
> curious, way out of scope for this CL.)

I don't think it was for stars; looks like it was added in 2013 in
https://codereview.chromium.org/25638003.

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
File components/sync/engine_impl/cycle/directory_type_debug_info_emitter.h
(right):

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
components/sync/engine_impl/cycle/directory_type_debug_info_emitter.h:18: 
On 2016/08/19 17:17:42, skym wrote:
> And not everything in sync/sessions/ even had the sessions namespace! Ahhh!

Yuuuuup...

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
File components/sync/engine_impl/cycle/sync_cycle_context.h (right):

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
components/sync/engine_impl/cycle/sync_cycle_context.h:33: class
TestScopedSessionEventListener;
On 2016/08/19 17:17:43, skym wrote:
> Shouldn't need this forward declaration to friend class this thing. Also,
where
> is it now defined? And why did it keep 'Session' ?

You're right, I deleted it because it wasn't used. Good catch!

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
components/sync/engine_impl/cycle/sync_cycle_context.h:127: friend class
TestScopedSessionEventListener;
On 2016/08/19 17:17:42, skym wrote:
> As far as I can tell, you deleted this class. So no need to friend it anymore.

Right you are!

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
File components/sync/engine_impl/model_type_registry_unittest.cc (right):

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
components/sync/engine_impl/model_type_registry_unittest.cc:233: // Add cycles
non-blocking type.
On 2016/08/19 06:04:00, maxbogue wrote:
> This one's gotta be changed back.

Done.

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
File components/sync/engine_impl/syncer_unittest.cc (right):

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
components/sync/engine_impl/syncer_unittest.cc:213: last_cycles_commit_delay_ =
iter->second;
On 2016/08/19 17:17:43, skym wrote:
> I think this is suppose to be session

Done.

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
components/sync/engine_impl/syncer_unittest.cc:253: void ResetSession() {
cycle_.reset(SyncCycle::Build(context_.get(), this)); }
On 2016/08/19 17:17:43, skym wrote:
> This should be ResetCycle()

Done.

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
components/sync/engine_impl/syncer_unittest.cc:581: base::TimeDelta
last_cycles_commit_delay_;
On 2016/08/19 17:17:43, skym wrote:
> Should be session

Done.

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
File components/sync/engine_impl/update_applicator.h (right):

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
components/sync/engine_impl/update_applicator.h:21: #include
"components/sync/engine_impl/cycle/status_controller.h"
On 2016/08/19 17:17:43, skym wrote:
> I don't think this is used in this file, but seems to cause compile build
errors
> if I remove it. I think you should probably move this to the actually required
> files instead.

Removed and added to components/sync/engine_impl/directory_update_handler.cc
instead, thanks!

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
components/sync/engine_impl/update_applicator.h:26: class ConflictResolver;
On 2016/08/19 17:17:43, skym wrote:
> Where is this used? Remove?

Done.

https://codereview.chromium.org/2258873003/diff/40001/components/sync/engine_...
components/sync/engine_impl/update_applicator.h:28: class StatusController;
On 2016/08/19 17:17:43, skym wrote:
> This is also getting #included, definitely don't need to forward declare it.

Done.

[maxbogue, 2016-08-19 19:22:46]

The CQ bit was unchecked by maxbogue@chromium.org

[maxbogue, 2016-08-19 19:22:54]

The CQ bit was checked by maxbogue@chromium.org

[maxbogue, 2016-08-19 19:22:55]

The patchset sent to the CQ was uploaded after l-g-t-m from skym@chromium.org
Link to the patchset: https://codereview.chromium.org/2258873003/#ps60001
(title: "Address comments.")

[commit-bot: I haz the power, 2016-08-19 19:23:02]

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-08-19 19:30:52]

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

[commit-bot: I haz the power, 2016-08-19 19:30:54]

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

[maxbogue, 2016-08-19 19:42:06]

Description was changed from

==========
[Sync] Move sessions/ to engine/cycle/ and rename things to match.

- Move everything in sessions/ to engine/cycle/.
- Move everything in sessions_impl/ to engine_impl/cycle/.
- Rename SyncSession* to SyncCycle* (within engine).
- Remove the syncer::sessions:: namespace entirely.
- Clean up deps.

BUG=637950
==========

to

==========
[Sync] Move sessions/ to engine/cycle/ and rename things to match.

- Move everything in sessions/ to engine/cycle/.
- Move everything in sessions_impl/ to engine_impl/cycle/.
- Rename SyncSession* to SyncCycle* (within engine).
- Remove the syncer::sessions:: namespace entirely.
- Clean up deps.

BUG=637950
TBR=thestig,noyau
==========

[maxbogue, 2016-08-19 19:42:56]

maxbogue@chromium.org changed reviewers:
+ noyau@chromium.org, thestig@chromium.org

[maxbogue, 2016-08-19 19:42:57]

TBR'ing:

thestig@ for //chrome/browser
noyau@ for //ios

[maxbogue, 2016-08-19 19:43:55]

The CQ bit was checked by maxbogue@chromium.org

[commit-bot: I haz the power, 2016-08-19 19:49:10]

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

[noyau (Ping after 24h), 2016-08-19 20:30:19]

ios lgtm.

There is no deps changes for ios, maybe the existing DEPS are too broad? As I'm
not sure what the goal of this move is I'm not sure if the DEPS needs to be
tightened up a bit to restrict the API used outside of sync itself.

[maxbogue, 2016-08-19 21:12:52]

On 2016/08/19 20:30:19, noyau wrote:
> ios lgtm.
> 
> There is no deps changes for ios, maybe the existing DEPS are too broad? As
I'm
> not sure what the goal of this move is I'm not sure if the DEPS needs to be
> tightened up a bit to restrict the API used outside of sync itself.

The goal of this specific move was just to improve sync's internal code sanity,
not external DEPS. Sorry if the bug was a bit misleading!

[commit-bot: I haz the power, 2016-08-19 21:41:53]

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

[commit-bot: I haz the power, 2016-08-19 21:41:55]

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

[maxbogue, 2016-08-19 22:07:35]

The CQ bit was checked by maxbogue@chromium.org

[commit-bot: I haz the power, 2016-08-19 22:08:12]

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

[Lei Zhang, 2016-08-19 22:39:21]

lgtm

[commit-bot: I haz the power, 2016-08-19 23:16:35]

Description was changed from

==========
[Sync] Move sessions/ to engine/cycle/ and rename things to match.

- Move everything in sessions/ to engine/cycle/.
- Move everything in sessions_impl/ to engine_impl/cycle/.
- Rename SyncSession* to SyncCycle* (within engine).
- Remove the syncer::sessions:: namespace entirely.
- Clean up deps.

BUG=637950
TBR=thestig,noyau
==========

to

==========
[Sync] Move sessions/ to engine/cycle/ and rename things to match.

- Move everything in sessions/ to engine/cycle/.
- Move everything in sessions_impl/ to engine_impl/cycle/.
- Rename SyncSession* to SyncCycle* (within engine).
- Remove the syncer::sessions:: namespace entirely.
- Clean up deps.

BUG=637950
TBR=thestig,noyau
==========

[commit-bot: I haz the power, 2016-08-19 23:16:36]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-08-19 23:18:31]

Description was changed from

==========
[Sync] Move sessions/ to engine/cycle/ and rename things to match.

- Move everything in sessions/ to engine/cycle/.
- Move everything in sessions_impl/ to engine_impl/cycle/.
- Rename SyncSession* to SyncCycle* (within engine).
- Remove the syncer::sessions:: namespace entirely.
- Clean up deps.

BUG=637950
TBR=thestig,noyau
==========

to

==========
[Sync] Move sessions/ to engine/cycle/ and rename things to match.

- Move everything in sessions/ to engine/cycle/.
- Move everything in sessions_impl/ to engine_impl/cycle/.
- Rename SyncSession* to SyncCycle* (within engine).
- Remove the syncer::sessions:: namespace entirely.
- Clean up deps.

BUG=637950
TBR=thestig,noyau

Committed: https://crrev.com/3067278a4670c40ad786742bbd47bacc36a5c1b3
Cr-Commit-Position: refs/heads/master@{#413278}
==========

[commit-bot: I haz the power, 2016-08-19 23:18:33]

Patchset 4 (id:??) landed as
https://crrev.com/3067278a4670c40ad786742bbd47bacc36a5c1b3
Cr-Commit-Position: refs/heads/master@{#413278}