[Sync] Fix flakiness from Sessions refactor

[Sync] Fix flakiness from Sessions refactor

Fixes a flaky issue introduced in the recent tabs submenu tests due to
inspecting possibly unset timestamp values. Also fixes a pre-existing issue
where sync ids for tabs were not being set, which the refactor brought
to light.

BUG=671902, 671902
TBR=msw

Committed: https://crrev.com/f4224baa9e43677897f657a97b810a89245f6631
Cr-Commit-Position: refs/heads/master@{#437604}

[Nicolas Zea, 2016-12-09 00:39:16]

Description was changed from

==========
[Sync] Fix flakiness from Sessions refactor

Fixes a flaky issue introduced in the recent tabs submenu tests due to
inspecting possibly unset timestamp values, along with adding some logging
to help identify a crash.

BUG=665196, 671902
==========

to

==========
[Sync] Fix flakiness from Sessions refactor

Fixes a flaky issue introduced in the recent tabs submenu tests due to
inspecting possibly unset timestamp values. Also fixes a pre-existing issue
where sync ids for tabs were not being set, which the refactor brought
to light.

BUG=665196, 671902
==========

[Nicolas Zea, 2016-12-09 00:39:48]

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

[Nicolas Zea, 2016-12-09 00:39:49]

PTAL! I suspect the fact that SetSyncId hasn't been called for a year has been
causing quite a bit of trouble :-/

[Nicolas Zea, 2016-12-09 01:49:05]

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

[commit-bot: I haz the power, 2016-12-09 01:49:40]

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-09 03:28:36]

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

[commit-bot: I haz the power, 2016-12-09 03:28:36]

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

[skym, 2016-12-09 15:40:01]

lgtm

This CL lists 665196 as a bug it addresses, again, I think that may be the wrong
number.

https://codereview.chromium.org/2562853002/diff/20001/components/sync_session...
File components/sync_sessions/sessions_sync_manager.cc (right):

https://codereview.chromium.org/2562853002/diff/20001/components/sync_session...
components/sync_sessions/sessions_sync_manager.cc:370:
tab_delegate->SetSyncId(tab_node_id);
I can't seem to find anywhere that reads this value (that's not clearly
placeholder logic). But I suppose consistency isn't bad.

https://codereview.chromium.org/2562853002/diff/20001/components/sync_session...
File components/sync_sessions/synced_session_tracker.cc (right):

https://codereview.chromium.org/2562853002/diff/20001/components/sync_session...
components/sync_sessions/synced_session_tracker.cc:265: LOG(ERROR) <<
"crbug.com/665196 Attempting to map tab " << tab_id
This doesn't seem like the right bug number.

[Nicolas Zea, 2016-12-09 17:32:16]

Description was changed from

==========
[Sync] Fix flakiness from Sessions refactor

Fixes a flaky issue introduced in the recent tabs submenu tests due to
inspecting possibly unset timestamp values. Also fixes a pre-existing issue
where sync ids for tabs were not being set, which the refactor brought
to light.

BUG=665196, 671902
==========

to

==========
[Sync] Fix flakiness from Sessions refactor

Fixes a flaky issue introduced in the recent tabs submenu tests due to
inspecting possibly unset timestamp values. Also fixes a pre-existing issue
where sync ids for tabs were not being set, which the refactor brought
to light.

BUG=671902, 671902
==========

[Nicolas Zea, 2016-12-09 17:33:36]

Thanks!

https://codereview.chromium.org/2562853002/diff/20001/components/sync_session...
File components/sync_sessions/sessions_sync_manager.cc (right):

https://codereview.chromium.org/2562853002/diff/20001/components/sync_session...
components/sync_sessions/sessions_sync_manager.cc:370:
tab_delegate->SetSyncId(tab_node_id);
On 2016/12/09 15:40:00, skym wrote:
> I can't seem to find anywhere that reads this value (that's not clearly
> placeholder logic). But I suppose consistency isn't bad.

Correct, we only use it for placeholders today (although perhaps in the future
that will expand)

https://codereview.chromium.org/2562853002/diff/20001/components/sync_session...
File components/sync_sessions/synced_session_tracker.cc (right):

https://codereview.chromium.org/2562853002/diff/20001/components/sync_session...
components/sync_sessions/synced_session_tracker.cc:265: LOG(ERROR) <<
"crbug.com/665196 Attempting to map tab " << tab_id
On 2016/12/09 15:40:00, skym wrote:
> This doesn't seem like the right bug number.

Oops! Fixed

[Nicolas Zea, 2016-12-09 18:06:19]

The CQ bit was checked by zea@chromium.org

[commit-bot: I haz the power, 2016-12-09 18:06:50]

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-09 18:45:07]

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

[commit-bot: I haz the power, 2016-12-09 18:45:07]

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

[Nicolas Zea, 2016-12-09 18:51:51]

Description was changed from

==========
[Sync] Fix flakiness from Sessions refactor

Fixes a flaky issue introduced in the recent tabs submenu tests due to
inspecting possibly unset timestamp values. Also fixes a pre-existing issue
where sync ids for tabs were not being set, which the refactor brought
to light.

BUG=671902, 671902
==========

to

==========
[Sync] Fix flakiness from Sessions refactor

Fixes a flaky issue introduced in the recent tabs submenu tests due to
inspecting possibly unset timestamp values. Also fixes a pre-existing issue
where sync ids for tabs were not being set, which the refactor brought
to light.

BUG=671902, 671902
TBR=msw
==========

[Nicolas Zea, 2016-12-09 18:51:52]

zea@chromium.org changed reviewers:
+ msw@chromium.org

[Nicolas Zea, 2016-12-09 18:52:13]

+TBR msw (just re-enabling a test that was disabled due to my patch)

[Nicolas Zea, 2016-12-09 18:52:20]

The CQ bit was checked by zea@chromium.org

[commit-bot: I haz the power, 2016-12-09 18:53:22]

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

[msw, 2016-12-09 19:11:52]

c/b/ui lgtm

[commit-bot: I haz the power, 2016-12-09 19:12:47]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1481309540904340,
"parent_rev": "d33ee282a752bdc6ab74daa2f52cf1505a20069c", "commit_rev":
"1ec0af92b43a6f4a431b1ee23f816750affd5179"}

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

Description was changed from

==========
[Sync] Fix flakiness from Sessions refactor

Fixes a flaky issue introduced in the recent tabs submenu tests due to
inspecting possibly unset timestamp values. Also fixes a pre-existing issue
where sync ids for tabs were not being set, which the refactor brought
to light.

BUG=671902, 671902
TBR=msw
==========

to

==========
[Sync] Fix flakiness from Sessions refactor

Fixes a flaky issue introduced in the recent tabs submenu tests due to
inspecting possibly unset timestamp values. Also fixes a pre-existing issue
where sync ids for tabs were not being set, which the refactor brought
to light.

BUG=671902, 671902
TBR=msw

Review-Url: https://codereview.chromium.org/2562853002
==========

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

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-12 14:36:45]

Description was changed from

==========
[Sync] Fix flakiness from Sessions refactor

Fixes a flaky issue introduced in the recent tabs submenu tests due to
inspecting possibly unset timestamp values. Also fixes a pre-existing issue
where sync ids for tabs were not being set, which the refactor brought
to light.

BUG=671902, 671902
TBR=msw

Review-Url: https://codereview.chromium.org/2562853002
==========

to

==========
[Sync] Fix flakiness from Sessions refactor

Fixes a flaky issue introduced in the recent tabs submenu tests due to
inspecting possibly unset timestamp values. Also fixes a pre-existing issue
where sync ids for tabs were not being set, which the refactor brought
to light.

BUG=671902, 671902
TBR=msw

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

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

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