[Sync] Creating sync_sessions component, moving revisit logic.

[Sync] Creating sync_sessions component, moving revisit logic.

BUG=536841
I've decided to abandon the previous typed urls revisit CL which caused us to decide to create the sync_sessions component. That CL was far too messy, doing far too many things. All the same changes should be split among three CLs, this being the first.

I've removed all substantive changes from these files, but a few tiny lint/import changes still remain. They're not too difficult to see.

stanisc@ - sync owner and primary reviewer for revisit CLs.

sky@ - Added you because we're declaring a dependency on components/sessions (and will shortly declare deps on history/core/browser and bookmarks/browser in subsequent CLs, components which you also own).

blundell@ - Added you because we're creating a new component.

Committed: https://crrev.com/6b9887ef36cf91aa6fe8539565609fd8dc851c12
Cr-Commit-Position: refs/heads/master@{#353395}

[skym, 2015-10-08 23:13:56]

Patchset #1 (id:1) has been deleted

[skym, 2015-10-08 23:20:26]

skym@chromium.org changed reviewers:
+ blundell@chromium.org, sky@chromium.org, stanisc@chromium.org

[blundell, 2015-10-09 08:55:06]

component creation LGTM

fwiw, I don't think you need to block on sky@ because you're just shuffling
around the pre-existing dependencies

https://codereview.chromium.org/1387253004/diff/20001/components/sync_session...
File components/sync_sessions.gypi (right):

https://codereview.chromium.org/1387253004/diff/20001/components/sync_session...
components/sync_sessions.gypi:44: 'target_name': 'sync_sessions_test_support',
?

[skym, 2015-10-09 15:16:35]

https://codereview.chromium.org/1387253004/diff/20001/components/sync_session...
File components/sync_sessions.gypi (right):

https://codereview.chromium.org/1387253004/diff/20001/components/sync_session...
components/sync_sessions.gypi:44: 'target_name': 'sync_sessions_test_support',
On 2015/10/09 08:55:06, blundell wrote:
> ?

I figured it would be easier to add the test support target now than later when
we actually needed it, even though we currently don't have any test support
files.

[skym, 2015-10-09 17:11:11]

https://codereview.chromium.org/1387253004/diff/20001/components/sync_session...
File components/sync_sessions.gypi (right):

https://codereview.chromium.org/1387253004/diff/20001/components/sync_session...
components/sync_sessions.gypi:44: 'target_name': 'sync_sessions_test_support',
On 2015/10/09 15:16:35, skym wrote:
> On 2015/10/09 08:55:06, blundell wrote:
> > ?
> 
> I figured it would be easier to add the test support target now than later
when
> we actually needed it, even though we currently don't have any test support
> files.

It seems the win8_chromium_ng build is failing from something related to this
test_support target... And seemingly only win8.

FAILED: E:/b/depot_tools/python276_bin/python.exe gyp-win-tool link-wrapper
environment.x86 False link.exe /nologo /OUT:components_unittests.exe
/PDB:components_unittests.exe.pdb @components_unittests.exe.rsp
LINK : fatal error LNK1181: cannot open input file
'obj/components/sync_sessions/test_support.lib'

I suppose I could remove the test_support if it's going to be a hassle to get to
build right now. My current assumption is that win8 skips making the .lib file
since there are no sources, and then the linking cannot find the file since it
doesn't exist.

Removing it all now.

[skym, 2015-10-09 17:32:49]

Patchset #3 (id:60001) has been deleted

[skym, 2015-10-09 17:33:03]

Patchset #3 (id:80001) has been deleted

[skym, 2015-10-09 17:37:53]

https://codereview.chromium.org/1387253004/diff/20001/components/sync_session...
File components/sync_sessions.gypi (right):

https://codereview.chromium.org/1387253004/diff/20001/components/sync_session...
components/sync_sessions.gypi:44: 'target_name': 'sync_sessions_test_support',
On 2015/10/09 17:11:11, skym wrote:
> On 2015/10/09 15:16:35, skym wrote:
> > On 2015/10/09 08:55:06, blundell wrote:
> > > ?
> > 
> > I figured it would be easier to add the test support target now than later
> when
> > we actually needed it, even though we currently don't have any test support
> > files.
> 
> It seems the win8_chromium_ng build is failing from something related to this
> test_support target... And seemingly only win8.
> 
> FAILED: E:/b/depot_tools/python276_bin/python.exe gyp-win-tool link-wrapper
> environment.x86 False link.exe /nologo /OUT:components_unittests.exe
> /PDB:components_unittests.exe.pdb @components_unittests.exe.rsp
> LINK : fatal error LNK1181: cannot open input file
> 'obj/components/sync_sessions/test_support.lib'
> 
> I suppose I could remove the test_support if it's going to be a hassle to get
to
> build right now. My current assumption is that win8 skips making the .lib file
> since there are no sources, and then the linking cannot find the file since it
> doesn't exist.
> 
> Removing it all now.

Err, bizarre, changing static_library to source_set for the GN build target of
test_support seemed to fix the win8 build.
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...
. I guess win8_chromium_ng is actually a GN build, my earlier understanding was
mistaken. Re-running default set of try bots now.

[blundell, 2015-10-09 18:00:57]

On 2015/10/09 17:37:53, skym wrote:
>
https://codereview.chromium.org/1387253004/diff/20001/components/sync_session...
> File components/sync_sessions.gypi (right):
> 
>
https://codereview.chromium.org/1387253004/diff/20001/components/sync_session...
> components/sync_sessions.gypi:44: 'target_name': 'sync_sessions_test_support',
> On 2015/10/09 17:11:11, skym wrote:
> > On 2015/10/09 15:16:35, skym wrote:
> > > On 2015/10/09 08:55:06, blundell wrote:
> > > > ?
> > > 
> > > I figured it would be easier to add the test support target now than later
> > when
> > > we actually needed it, even though we currently don't have any test
support
> > > files.
> > 
> > It seems the win8_chromium_ng build is failing from something related to
this
> > test_support target... And seemingly only win8.
> > 
> > FAILED: E:/b/depot_tools/python276_bin/python.exe gyp-win-tool link-wrapper
> > environment.x86 False link.exe /nologo /OUT:components_unittests.exe
> > /PDB:components_unittests.exe.pdb @components_unittests.exe.rsp
> > LINK : fatal error LNK1181: cannot open input file
> > 'obj/components/sync_sessions/test_support.lib'
> > 
> > I suppose I could remove the test_support if it's going to be a hassle to
get
> to
> > build right now. My current assumption is that win8 skips making the .lib
file
> > since there are no sources, and then the linking cannot find the file since
it
> > doesn't exist.
> > 
> > Removing it all now.
> 
> Err, bizarre, changing static_library to source_set for the GN build target of
> test_support seemed to fix the win8 build.
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...
> . I guess win8_chromium_ng is actually a GN build, my earlier understanding
was
> mistaken. Re-running default set of try bots now.

I don't think it's a good idea to have empty targets for exactly this kind of
reason (even if it happens to work across our set of configurations at the
current time).

[skym, 2015-10-09 18:04:23]

On 2015/10/09 18:00:57, blundell wrote:
> On 2015/10/09 17:37:53, skym wrote:
> >
>
https://codereview.chromium.org/1387253004/diff/20001/components/sync_session...
> > File components/sync_sessions.gypi (right):
> > 
> >
>
https://codereview.chromium.org/1387253004/diff/20001/components/sync_session...
> > components/sync_sessions.gypi:44: 'target_name':
'sync_sessions_test_support',
> > On 2015/10/09 17:11:11, skym wrote:
> > > On 2015/10/09 15:16:35, skym wrote:
> > > > On 2015/10/09 08:55:06, blundell wrote:
> > > > > ?
> > > > 
> > > > I figured it would be easier to add the test support target now than
later
> > > when
> > > > we actually needed it, even though we currently don't have any test
> support
> > > > files.
> > > 
> > > It seems the win8_chromium_ng build is failing from something related to
> this
> > > test_support target... And seemingly only win8.
> > > 
> > > FAILED: E:/b/depot_tools/python276_bin/python.exe gyp-win-tool
link-wrapper
> > > environment.x86 False link.exe /nologo /OUT:components_unittests.exe
> > > /PDB:components_unittests.exe.pdb @components_unittests.exe.rsp
> > > LINK : fatal error LNK1181: cannot open input file
> > > 'obj/components/sync_sessions/test_support.lib'
> > > 
> > > I suppose I could remove the test_support if it's going to be a hassle to
> get
> > to
> > > build right now. My current assumption is that win8 skips making the .lib
> file
> > > since there are no sources, and then the linking cannot find the file
since
> it
> > > doesn't exist.
> > > 
> > > Removing it all now.
> > 
> > Err, bizarre, changing static_library to source_set for the GN build target
of
> > test_support seemed to fix the win8 build.
> >
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...
> > . I guess win8_chromium_ng is actually a GN build, my earlier understanding
> was
> > mistaken. Re-running default set of try bots now.
> 
> I don't think it's a good idea to have empty targets for exactly this kind of
> reason (even if it happens to work across our set of configurations at the
> current time).

Hmm, okay, will rip out the test_support stuff.

[stanisc, 2015-10-09 19:12:08]

LGTM
Didn't see any issues.

[skym, 2015-10-09 19:12:50]

The CQ bit was checked by skym@chromium.org

[skym, 2015-10-09 19:12:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
blundell@chromium.org
Link to the patchset: https://codereview.chromium.org/1387253004/#ps100001
(title: "Removing test_support target.")

[commit-bot: I haz the power, 2015-10-09 19:17:08]

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

[commit-bot: I haz the power, 2015-10-09 19:37:25]

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

[commit-bot: I haz the power, 2015-10-09 19:37:26]

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

[skym, 2015-10-09 19:47:38]

Looks like I don't have the OWNER approvals to CQ this change.

** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    chrome/browser/BUILD.gn

When you get a chance, sky@, do you think you could look over this change?

[sky, 2015-10-09 21:47:38]

You only need me for the DEPS change, which LGTM

[skym, 2015-10-09 21:55:09]

The CQ bit was checked by skym@chromium.org

[commit-bot: I haz the power, 2015-10-09 21:56:32]

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

[commit-bot: I haz the power, 2015-10-09 22:10:55]

Committed patchset #3 (id:100001)

[commit-bot: I haz the power, 2015-10-09 22:11:57]

Patchset 3 (id:??) landed as
https://crrev.com/6b9887ef36cf91aa6fe8539565609fd8dc851c12
Cr-Commit-Position: refs/heads/master@{#353395}