Commit ba9ef11b authored by skym's avatar skym Committed by Commit bot

[Sync] Enable USS DeviceInfo for bots.

As we turn the feature EnableSyncUSSDeviceInfo to handle DeviceInfo
with a USS approach instead of via Directory, we want to have the bots
testing the new code paths.

There were several issues that were fixed before this patch. Here we're
only making two changes to fix failing tests.

1. Python sync server implementation is relaxing its restrictions about
progress marker. It seemed arbitrarily and pedantic for it to treat
empty tokens and non-present tokens differently, so now starts both as
first time syncs. This more accurately reflects how the real server
handles this case.

2. EnableDisableSingleClientTest no longer tests USS types. As we add
more USS  types we'll unfortunately have to update this code to exclude
those types as well. There isn't a good USS equivalent to checking for
top level nodes to see if a type has actual sync data. There is no
directory and there are no top level nodes. When USS is completely
rolled out we may just want to remove this test class.

BUG=650725

Review-Url: https://codereview.chromium.org/2568543004
Cr-Commit-Position: refs/heads/master@{#439870}
parent 7b7cf317
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "chrome/browser/sync/test/integration/sync_test.h" #include "chrome/browser/sync/test/integration/sync_test.h"
#include "components/browser_sync/profile_sync_service.h" #include "components/browser_sync/profile_sync_service.h"
#include "components/sync/base/model_type.h" #include "components/sync/base/model_type.h"
#include "components/sync/driver/sync_driver_switches.h"
#include "components/sync/syncable/read_node.h" #include "components/sync/syncable/read_node.h"
#include "components/sync/syncable/read_transaction.h" #include "components/sync/syncable/read_transaction.h"
...@@ -15,6 +16,9 @@ ...@@ -15,6 +16,9 @@
namespace { namespace {
using base::FeatureList;
using syncer::ModelTypeSet;
class EnableDisableSingleClientTest : public SyncTest { class EnableDisableSingleClientTest : public SyncTest {
public: public:
EnableDisableSingleClientTest() : SyncTest(SINGLE_CLIENT) {} EnableDisableSingleClientTest() : SyncTest(SINGLE_CLIENT) {}
...@@ -39,21 +43,36 @@ bool IsUnready(const syncer::DataTypeStatusTable& data_type_status_table, ...@@ -39,21 +43,36 @@ bool IsUnready(const syncer::DataTypeStatusTable& data_type_status_table,
return data_type_status_table.GetUnreadyErrorTypes().Has(type); return data_type_status_table.GetUnreadyErrorTypes().Has(type);
} }
// The current approach this test class takes is to examine the Directory and
// check for root nodes to see if a type is currently enabled. While this works
// for things in the directory, it does not work for USS types. USS does not
// have any general data access mechanism, at least yet. Until that exists,
// simply omit types that may be USS from these cases.
ModelTypeSet UnifiedSyncServiceTypes() {
ModelTypeSet set;
if (FeatureList::IsEnabled(switches::kSyncUSSDeviceInfo)) {
set.Put(syncer::DEVICE_INFO);
}
return set;
}
IN_PROC_BROWSER_TEST_F(EnableDisableSingleClientTest, EnableOneAtATime) { IN_PROC_BROWSER_TEST_F(EnableDisableSingleClientTest, EnableOneAtATime) {
ASSERT_TRUE(SetupClients()); ASSERT_TRUE(SetupClients());
// Setup sync with no enabled types. // Setup sync with no enabled types.
ASSERT_TRUE(GetClient(0)->SetupSync(syncer::ModelTypeSet())); ASSERT_TRUE(GetClient(0)->SetupSync(ModelTypeSet()));
syncer::UserShare* user_share = GetSyncService(0)->GetUserShare(); syncer::UserShare* user_share = GetSyncService(0)->GetUserShare();
const syncer::DataTypeStatusTable& data_type_status_table = const syncer::DataTypeStatusTable& data_type_status_table =
GetSyncService(0)->data_type_status_table(); GetSyncService(0)->data_type_status_table();
const syncer::ModelTypeSet registered_types = const ModelTypeSet registered_types =
GetSyncService(0)->GetRegisteredDataTypes(); GetSyncService(0)->GetRegisteredDataTypes();
const syncer::ModelTypeSet registered_user_types = const ModelTypeSet registered_directory_types =
Difference(registered_types, UnifiedSyncServiceTypes());
const ModelTypeSet registered_directory_user_types =
Intersection(registered_types, syncer::UserSelectableTypes()); Intersection(registered_types, syncer::UserSelectableTypes());
for (syncer::ModelTypeSet::Iterator it = registered_user_types.First(); for (ModelTypeSet::Iterator it = registered_directory_user_types.First();
it.Good(); it.Inc()) { it.Good(); it.Inc()) {
ASSERT_TRUE(GetClient(0)->EnableSyncForDatatype(it.Get())); ASSERT_TRUE(GetClient(0)->EnableSyncForDatatype(it.Get()));
...@@ -90,8 +109,10 @@ IN_PROC_BROWSER_TEST_F(EnableDisableSingleClientTest, DisableOneAtATime) { ...@@ -90,8 +109,10 @@ IN_PROC_BROWSER_TEST_F(EnableDisableSingleClientTest, DisableOneAtATime) {
// Setup sync with no disabled types. // Setup sync with no disabled types.
ASSERT_TRUE(GetClient(0)->SetupSync()); ASSERT_TRUE(GetClient(0)->SetupSync());
const syncer::ModelTypeSet registered_types = const ModelTypeSet registered_types =
GetSyncService(0)->GetRegisteredDataTypes(); GetSyncService(0)->GetRegisteredDataTypes();
const ModelTypeSet registered_directory_types =
Difference(registered_types, UnifiedSyncServiceTypes());
syncer::UserShare* user_share = GetSyncService(0)->GetUserShare(); syncer::UserShare* user_share = GetSyncService(0)->GetUserShare();
...@@ -99,7 +120,7 @@ IN_PROC_BROWSER_TEST_F(EnableDisableSingleClientTest, DisableOneAtATime) { ...@@ -99,7 +120,7 @@ IN_PROC_BROWSER_TEST_F(EnableDisableSingleClientTest, DisableOneAtATime) {
GetSyncService(0)->data_type_status_table(); GetSyncService(0)->data_type_status_table();
// Make sure all top-level nodes exist first. // Make sure all top-level nodes exist first.
for (syncer::ModelTypeSet::Iterator it = registered_types.First(); for (ModelTypeSet::Iterator it = registered_directory_types.First();
it.Good(); it.Inc()) { it.Good(); it.Inc()) {
if (!syncer::ProxyTypes().Has(it.Get())) { if (!syncer::ProxyTypes().Has(it.Get())) {
ASSERT_TRUE(DoesTopLevelNodeExist(user_share, it.Get()) || ASSERT_TRUE(DoesTopLevelNodeExist(user_share, it.Get()) ||
...@@ -107,7 +128,7 @@ IN_PROC_BROWSER_TEST_F(EnableDisableSingleClientTest, DisableOneAtATime) { ...@@ -107,7 +128,7 @@ IN_PROC_BROWSER_TEST_F(EnableDisableSingleClientTest, DisableOneAtATime) {
} }
} }
for (syncer::ModelTypeSet::Iterator it = registered_types.First(); for (ModelTypeSet::Iterator it = registered_directory_types.First();
it.Good(); it.Inc()) { it.Good(); it.Inc()) {
// SUPERVISED_USERS and SUPERVISED_USER_SHARED_SETTINGS are always synced. // SUPERVISED_USERS and SUPERVISED_USER_SHARED_SETTINGS are always synced.
if (it.Get() == syncer::SUPERVISED_USERS || if (it.Get() == syncer::SUPERVISED_USERS ||
......
...@@ -429,10 +429,8 @@ class UpdateSieve(object): ...@@ -429,10 +429,8 @@ class UpdateSieve(object):
elif marker.token: elif marker.token:
(timestamp, version) = pickle.loads(marker.token) (timestamp, version) = pickle.loads(marker.token)
self._migration_versions_to_check[data_type] = version self._migration_versions_to_check[data_type] = version
elif marker.HasField('token'):
timestamp = 0
else: else:
raise ValueError('No timestamp information in progress marker.') timestamp = 0
data_type = ProtocolDataTypeIdToSyncType(marker.data_type_id) data_type = ProtocolDataTypeIdToSyncType(marker.data_type_id)
self._state[data_type] = timestamp self._state[data_type] = timestamp
elif request.HasField('from_timestamp'): elif request.HasField('from_timestamp'):
......
...@@ -671,6 +671,26 @@ ...@@ -671,6 +671,26 @@
] ]
} }
], ],
"EnableSyncUSSDeviceInfo": [
{
"platforms": [
"android",
"chromeos",
"ios",
"linux",
"mac",
"win"
],
"experiments": [
{
"name": "Enabled",
"enable_features": [
"EnableSyncUSSDeviceInfo"
]
}
]
}
],
"ExpectCTReporting": [ "ExpectCTReporting": [
{ {
"platforms": [ "platforms": [
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment