summaryrefslogtreecommitdiffstats
path: root/sync
diff options
context:
space:
mode:
authorrlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-12-12 23:53:19 +0000
committerrlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-12-12 23:53:19 +0000
commit47151c66e3160c66afdd837c7ff45df35db03658 (patch)
tree229a14bb5406f335fcd60496027bb2ec4f997d31 /sync
parent534bc2478349834e475c5c18d44c84ffc3f76c27 (diff)
downloadchromium_src-47151c66e3160c66afdd837c7ff45df35db03658.zip
chromium_src-47151c66e3160c66afdd837c7ff45df35db03658.tar.gz
chromium_src-47151c66e3160c66afdd837c7ff45df35db03658.tar.bz2
Remove initial_sync_ended bits
As of crbug.com/164288, we can no longer trust initial_sync_ended. If my diagnosis is correct, then there are some clients out there who have their initial_sync_ended bits set for some data types which have not actually been synced. Those clients now crash at every startup. This change attempts to solve the problem by removing the separate initial_sync_ended array of bools and checking the database directly to learn of a type's status. The new definition of initial_sync_ended is that a type has finished initial sync if its top level folder has been downloaded and applied. To prevent crashing if the server decides to not send us those root nodes during the control types configure step, we now check that all control types are initial_sync_ended following that configure and fail gracefully if their initial sync is not completed. The new definition of initial_sync_ended makes it much easier to hit crbug.com/164914. That bug would prevent us from saving our changes following a call to PurgeEntriesWithTypeIn() in some situations. This commit also fixes that bug. Since the initial sync ended status is no longer independent from the database state, it is no longer necessary to collect debug info as to its status. It has been removed from the about:sync UI and other debug info reporting mechanisms. Its role in unit tests has also been greatly reduced. BUG=164288,164914 Review URL: https://chromiumcodereview.appspot.com/11474036 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@172707 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'sync')
-rw-r--r--sync/engine/all_status.cc4
-rw-r--r--sync/engine/apply_control_data_updates.cc11
-rw-r--r--sync/engine/apply_control_data_updates_unittest.cc22
-rw-r--r--sync/engine/apply_updates_and_resolve_conflicts_command.cc14
-rw-r--r--sync/engine/syncer_unittest.cc12
-rw-r--r--sync/internal_api/js_sync_manager_observer_unittest.cc2
-rw-r--r--sync/internal_api/public/engine/sync_status.cc1
-rw-r--r--sync/internal_api/public/engine/sync_status.h2
-rw-r--r--sync/internal_api/public/sessions/sync_session_snapshot.cc18
-rw-r--r--sync/internal_api/public/sessions/sync_session_snapshot.h6
-rw-r--r--sync/internal_api/public/sessions/sync_session_snapshot_unittest.cc13
-rw-r--r--sync/internal_api/public/test/test_entry_factory.h5
-rw-r--r--sync/internal_api/sync_manager_impl.cc4
-rw-r--r--sync/internal_api/sync_manager_impl_unittest.cc127
-rw-r--r--sync/internal_api/test/test_entry_factory.cc22
-rw-r--r--sync/internal_api/test/test_user_share.cc22
-rw-r--r--sync/sessions/sync_session.cc10
-rw-r--r--sync/syncable/directory.cc46
-rw-r--r--sync/syncable/directory.h9
-rw-r--r--sync/syncable/directory_backing_store.cc77
-rw-r--r--sync/syncable/directory_backing_store.h2
-rw-r--r--sync/syncable/directory_backing_store_unittest.cc275
-rw-r--r--sync/syncable/syncable_unittest.cc29
-rw-r--r--sync/test/engine/test_syncable_utils.cc27
-rw-r--r--sync/test/engine/test_syncable_utils.h8
-rw-r--r--sync/test/test_directory_backing_store.h1
26 files changed, 505 insertions, 264 deletions
diff --git a/sync/engine/all_status.cc b/sync/engine/all_status.cc
index ff85d57..24ab03a 100644
--- a/sync/engine/all_status.cc
+++ b/sync/engine/all_status.cc
@@ -14,7 +14,6 @@
namespace syncer {
AllStatus::AllStatus() {
- status_.initial_sync_ended = true;
status_.notifications_enabled = false;
status_.cryptographer_ready = false;
status_.crypto_has_pending_keys = false;
@@ -32,7 +31,6 @@ SyncStatus AllStatus::CreateBlankStatus() const {
status.hierarchy_conflicts = 0;
status.server_conflicts = 0;
status.committed_count = 0;
- status.initial_sync_ended = false;
status.updates_available = 0;
return status;
}
@@ -52,8 +50,6 @@ SyncStatus AllStatus::CalcSyncing(const SyncEngineEvent &event) const {
status.syncing = false;
}
- status.initial_sync_ended |= snapshot.is_share_usable();
-
status.updates_available += snapshot.num_server_changes_remaining();
status.sync_protocol_error =
snapshot.model_neutral_state().sync_protocol_error;
diff --git a/sync/engine/apply_control_data_updates.cc b/sync/engine/apply_control_data_updates.cc
index 6277365..09778c3 100644
--- a/sync/engine/apply_control_data_updates.cc
+++ b/sync/engine/apply_control_data_updates.cc
@@ -79,17 +79,6 @@ void ApplyControlDataUpdates(sessions::SyncSession* session) {
&entry,
dir->GetCryptographer(&trans));
}
-
- // Set initial sync ended bits for all control types requested.
- for (ModelTypeSet::Iterator it =
- session->status_controller().updates_request_types().First();
- it.Good(); it.Inc()) {
- if (!IsControlType(it.Get()))
- continue;
-
- // This gets persisted to the directory's backing store.
- dir->set_initial_sync_ended_for_type(it.Get(), true);
- }
}
// Update the nigori handler with the server's nigori node.
diff --git a/sync/engine/apply_control_data_updates_unittest.cc b/sync/engine/apply_control_data_updates_unittest.cc
index 09c5e11..754bac3 100644
--- a/sync/engine/apply_control_data_updates_unittest.cc
+++ b/sync/engine/apply_control_data_updates_unittest.cc
@@ -67,11 +67,6 @@ TEST_F(ApplyControlDataUpdatesTest, NigoriUpdate) {
ModelTypeSet encrypted_types;
encrypted_types.PutAll(SyncEncryptionHandler::SensitiveTypes());
- // We start with initial_sync_ended == false. This is wrong, since we would
- // have no nigori node if that were the case. However, it makes it easier to
- // verify that ApplyControlDataUpdates sets initial_sync_ended correctly.
- EXPECT_FALSE(directory()->initial_sync_ended_types().Has(NIGORI));
-
{
syncable::ReadTransaction trans(FROM_HERE, directory());
cryptographer = directory()->GetCryptographer(&trans);
@@ -100,7 +95,6 @@ TEST_F(ApplyControlDataUpdatesTest, NigoriUpdate) {
syncable::ReadTransaction trans(FROM_HERE, directory());
EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
.Equals(ModelTypeSet::All()));
- EXPECT_TRUE(directory()->initial_sync_ended_types().Has(NIGORI));
}
}
@@ -383,7 +377,6 @@ TEST_F(ApplyControlDataUpdatesTest,
syncable::ReadTransaction trans(FROM_HERE, directory());
EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
.Equals(ModelTypeSet::All()));
- EXPECT_TRUE(directory()->initial_sync_ended_types().Has(NIGORI));
}
}
@@ -462,7 +455,6 @@ TEST_F(ApplyControlDataUpdatesTest,
syncable::ReadTransaction trans(FROM_HERE, directory());
EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
.Equals(ModelTypeSet::All()));
- EXPECT_TRUE(directory()->initial_sync_ended_types().Has(NIGORI));
}
}
@@ -534,7 +526,6 @@ TEST_F(ApplyControlDataUpdatesTest,
syncable::ReadTransaction trans(FROM_HERE, directory());
EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
.Equals(ModelTypeSet::All()));
- EXPECT_TRUE(directory()->initial_sync_ended_types().Has(NIGORI));
}
}
@@ -617,7 +608,6 @@ TEST_F(ApplyControlDataUpdatesTest,
syncable::ReadTransaction trans(FROM_HERE, directory());
EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
.Equals(ModelTypeSet::All()));
- EXPECT_TRUE(directory()->initial_sync_ended_types().Has(NIGORI));
}
}
@@ -701,7 +691,6 @@ TEST_F(ApplyControlDataUpdatesTest,
syncable::ReadTransaction trans(FROM_HERE, directory());
EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
.Equals(ModelTypeSet::All()));
- EXPECT_TRUE(directory()->initial_sync_ended_types().Has(NIGORI));
}
}
@@ -781,7 +770,6 @@ TEST_F(ApplyControlDataUpdatesTest,
syncable::ReadTransaction trans(FROM_HERE, directory());
EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans)
.Equals(ModelTypeSet::All()));
- EXPECT_TRUE(directory()->initial_sync_ended_types().Has(NIGORI));
}
}
@@ -866,14 +854,11 @@ TEST_F(ApplyControlDataUpdatesTest,
nigori().passphrase_type());
{
syncable::ReadTransaction trans(FROM_HERE, directory());
- EXPECT_TRUE(directory()->initial_sync_ended_types().Has(NIGORI));
}
}
// Check that we can apply a simple control datatype node successfully.
TEST_F(ApplyControlDataUpdatesTest, ControlApply) {
- EXPECT_FALSE(directory()->initial_sync_ended_types().Has(EXPERIMENTS));
-
std::string experiment_id = "experiment";
sync_pb::EntitySpecifics specifics;
specifics.mutable_experiments()->mutable_keystore_encryption()->
@@ -882,7 +867,6 @@ TEST_F(ApplyControlDataUpdatesTest, ControlApply) {
experiment_id, specifics, false);
ApplyControlDataUpdates(session());
- EXPECT_TRUE(directory()->initial_sync_ended_types().Has(EXPERIMENTS));
EXPECT_FALSE(entry_factory_->GetIsUnappliedForItem(experiment_handle));
EXPECT_TRUE(
entry_factory_->GetLocalSpecificsForItem(experiment_handle).
@@ -891,8 +875,6 @@ TEST_F(ApplyControlDataUpdatesTest, ControlApply) {
// Verify that we apply top level folders before their children.
TEST_F(ApplyControlDataUpdatesTest, ControlApplyParentBeforeChild) {
- EXPECT_FALSE(directory()->initial_sync_ended_types().Has(EXPERIMENTS));
-
std::string parent_id = "parent";
std::string experiment_id = "experiment";
sync_pb::EntitySpecifics specifics;
@@ -904,7 +886,6 @@ TEST_F(ApplyControlDataUpdatesTest, ControlApplyParentBeforeChild) {
parent_id, specifics, true);
ApplyControlDataUpdates(session());
- EXPECT_TRUE(directory()->initial_sync_ended_types().Has(EXPERIMENTS));
EXPECT_FALSE(entry_factory_->GetIsUnappliedForItem(parent_handle));
EXPECT_FALSE(entry_factory_->GetIsUnappliedForItem(experiment_handle));
EXPECT_TRUE(
@@ -915,8 +896,6 @@ TEST_F(ApplyControlDataUpdatesTest, ControlApplyParentBeforeChild) {
// Verify that we handle control datatype conflicts by preserving the server
// data.
TEST_F(ApplyControlDataUpdatesTest, ControlConflict) {
- EXPECT_FALSE(directory()->initial_sync_ended_types().Has(EXPERIMENTS));
-
std::string experiment_id = "experiment";
sync_pb::EntitySpecifics local_specifics, server_specifics;
server_specifics.mutable_experiments()->mutable_keystore_encryption()->
@@ -931,7 +910,6 @@ TEST_F(ApplyControlDataUpdatesTest, ControlConflict) {
local_specifics);
ApplyControlDataUpdates(session());
- EXPECT_TRUE(directory()->initial_sync_ended_types().Has(EXPERIMENTS));
EXPECT_FALSE(entry_factory_->GetIsUnappliedForItem(experiment_handle));
EXPECT_TRUE(
entry_factory_->GetLocalSpecificsForItem(experiment_handle).
diff --git a/sync/engine/apply_updates_and_resolve_conflicts_command.cc b/sync/engine/apply_updates_and_resolve_conflicts_command.cc
index 454ba5a..971f90d 100644
--- a/sync/engine/apply_updates_and_resolve_conflicts_command.cc
+++ b/sync/engine/apply_updates_and_resolve_conflicts_command.cc
@@ -128,20 +128,6 @@ SyncerError ApplyUpdatesAndResolveConflictsCommand::ModelChangingExecuteImpl(
DCHECK(conflict_applicator.simple_conflict_ids().empty());
}
- // This might be the first time we've fully completed a sync cycle, for
- // some subset of the currently synced datatypes.
- if (status->ServerSaysNothingMoreToDownload()) {
- for (ModelTypeSet::Iterator it =
- status->updates_request_types().First(); it.Good(); it.Inc()) {
- // Don't set the flag for control types. We didn't process them here.
- if (IsControlType(it.Get()))
- continue;
-
- // This gets persisted to the directory's backing store.
- dir->set_initial_sync_ended_for_type(it.Get(), true);
- }
- }
-
return SYNCER_OK;
}
diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc
index d92cc0e..df41960 100644
--- a/sync/engine/syncer_unittest.cc
+++ b/sync/engine/syncer_unittest.cc
@@ -297,10 +297,6 @@ class SyncerTest : public testing::Test,
EXPECT_FALSE(client_status.has_hierarchy_conflict_detected());
}
- bool initial_sync_ended_for_type(ModelType type) {
- return directory()->initial_sync_ended_for_type(type);
- }
-
void SyncRepeatedlyToTriggerConflictResolution(SyncSession* session) {
// We should trigger after less than 6 syncs, but extra does no harm.
for (int i = 0 ; i < 6 ; ++i)
@@ -3952,8 +3948,6 @@ TEST_F(SyncerTest, UpdateFailsThenDontCommit) {
// Downloads two updates and applies them successfully.
// This is the "happy path" alternative to ConfigureFailsDontApplyUpdates.
TEST_F(SyncerTest, ConfigureDownloadsTwoBatchesSuccess) {
- EXPECT_FALSE(initial_sync_ended_for_type(BOOKMARKS));
-
syncable::Id node1 = ids_.NewServerId();
syncable::Id node2 = ids_.NewServerId();
@@ -3977,14 +3971,10 @@ TEST_F(SyncerTest, ConfigureDownloadsTwoBatchesSuccess) {
Entry n2(&trans, GET_BY_ID, node2);
ASSERT_TRUE(n2.good());
EXPECT_FALSE(n2.Get(IS_UNAPPLIED_UPDATE));
-
- EXPECT_TRUE(initial_sync_ended_for_type(BOOKMARKS));
}
// Same as the above case, but this time the second batch fails to download.
TEST_F(SyncerTest, ConfigureFailsDontApplyUpdates) {
- EXPECT_FALSE(initial_sync_ended_for_type(BOOKMARKS));
-
syncable::Id node1 = ids_.NewServerId();
syncable::Id node2 = ids_.NewServerId();
@@ -4017,8 +4007,6 @@ TEST_F(SyncerTest, ConfigureFailsDontApplyUpdates) {
// One update remains undownloaded.
mock_server_->ClearUpdatesQueue();
-
- EXPECT_FALSE(initial_sync_ended_for_type(BOOKMARKS));
}
TEST_F(SyncerTest, GetKeySuccess) {
diff --git a/sync/internal_api/js_sync_manager_observer_unittest.cc b/sync/internal_api/js_sync_manager_observer_unittest.cc
index 3668400..ce09797 100644
--- a/sync/internal_api/js_sync_manager_observer_unittest.cc
+++ b/sync/internal_api/js_sync_manager_observer_unittest.cc
@@ -77,8 +77,6 @@ TEST_F(JsSyncManagerObserverTest, OnInitializationComplete) {
TEST_F(JsSyncManagerObserverTest, OnSyncCycleCompleted) {
sessions::SyncSessionSnapshot snapshot(
sessions::ModelNeutralState(),
- false,
- ModelTypeSet(),
ProgressMarkerMap(),
false,
5,
diff --git a/sync/internal_api/public/engine/sync_status.cc b/sync/internal_api/public/engine/sync_status.cc
index f4037da..6291b29 100644
--- a/sync/internal_api/public/engine/sync_status.cc
+++ b/sync/internal_api/public/engine/sync_status.cc
@@ -14,7 +14,6 @@ SyncStatus::SyncStatus()
server_conflicts(0),
committed_count(0),
syncing(false),
- initial_sync_ended(false),
updates_available(0),
updates_received(0),
reflected_updates_received(0),
diff --git a/sync/internal_api/public/engine/sync_status.h b/sync/internal_api/public/engine/sync_status.h
index 5afc356..31cbf4c 100644
--- a/sync/internal_api/public/engine/sync_status.h
+++ b/sync/internal_api/public/engine/sync_status.h
@@ -48,8 +48,6 @@ struct SYNC_EXPORT SyncStatus {
int committed_count;
bool syncing;
- // True after a client has done a first sync.
- bool initial_sync_ended;
// Total updates available. If zero, nothing left to download.
int64 updates_available;
diff --git a/sync/internal_api/public/sessions/sync_session_snapshot.cc b/sync/internal_api/public/sessions/sync_session_snapshot.cc
index d74ffa7..b0e9fac 100644
--- a/sync/internal_api/public/sessions/sync_session_snapshot.cc
+++ b/sync/internal_api/public/sessions/sync_session_snapshot.cc
@@ -12,8 +12,7 @@ namespace syncer {
namespace sessions {
SyncSessionSnapshot::SyncSessionSnapshot()
- : is_share_usable_(false),
- is_silenced_(false),
+ : is_silenced_(false),
num_encryption_conflicts_(0),
num_hierarchy_conflicts_(0),
num_server_conflicts_(0),
@@ -26,8 +25,6 @@ SyncSessionSnapshot::SyncSessionSnapshot()
SyncSessionSnapshot::SyncSessionSnapshot(
const ModelNeutralState& model_neutral_state,
- bool is_share_usable,
- ModelTypeSet initial_sync_ended,
const ProgressMarkerMap& download_progress_markers,
bool is_silenced,
int num_encryption_conflicts,
@@ -41,8 +38,6 @@ SyncSessionSnapshot::SyncSessionSnapshot(
const std::vector<int>& num_entries_by_type,
const std::vector<int>& num_to_delete_entries_by_type)
: model_neutral_state_(model_neutral_state),
- is_share_usable_(is_share_usable),
- initial_sync_ended_(initial_sync_ended),
download_progress_markers_(download_progress_markers),
is_silenced_(is_silenced),
num_encryption_conflicts_(num_encryption_conflicts),
@@ -79,9 +74,6 @@ DictionaryValue* SyncSessionSnapshot::ToValue() const {
value->SetInteger(
"numServerChangesRemaining",
static_cast<int>(model_neutral_state_.num_server_changes_remaining));
- value->SetBoolean("isShareUsable", is_share_usable_);
- value->Set("initialSyncEnded",
- ModelTypeSetToValue(initial_sync_ended_));
value->Set("downloadProgressMarkers",
ProgressMarkerMapToValue(download_progress_markers_).release());
value->SetBoolean("isSilenced", is_silenced_);
@@ -130,14 +122,6 @@ int64 SyncSessionSnapshot::num_server_changes_remaining() const {
return model_neutral_state().num_server_changes_remaining;
}
-bool SyncSessionSnapshot::is_share_usable() const {
- return is_share_usable_;
-}
-
-ModelTypeSet SyncSessionSnapshot::initial_sync_ended() const {
- return initial_sync_ended_;
-}
-
const ProgressMarkerMap&
SyncSessionSnapshot::download_progress_markers() const {
return download_progress_markers_;
diff --git a/sync/internal_api/public/sessions/sync_session_snapshot.h b/sync/internal_api/public/sessions/sync_session_snapshot.h
index 25195a7..be29d35 100644
--- a/sync/internal_api/public/sessions/sync_session_snapshot.h
+++ b/sync/internal_api/public/sessions/sync_session_snapshot.h
@@ -32,8 +32,6 @@ class SYNC_EXPORT SyncSessionSnapshot {
SyncSessionSnapshot();
SyncSessionSnapshot(
const ModelNeutralState& model_neutral_state,
- bool is_share_usable,
- ModelTypeSet initial_sync_ended,
const ProgressMarkerMap& download_progress_markers,
bool is_silenced,
int num_encryption_conflicts,
@@ -57,8 +55,6 @@ class SYNC_EXPORT SyncSessionSnapshot {
return model_neutral_state_;
}
int64 num_server_changes_remaining() const;
- bool is_share_usable() const;
- ModelTypeSet initial_sync_ended() const;
const ProgressMarkerMap& download_progress_markers() const;
bool is_silenced() const;
int num_encryption_conflicts() const;
@@ -77,8 +73,6 @@ class SYNC_EXPORT SyncSessionSnapshot {
private:
ModelNeutralState model_neutral_state_;
- bool is_share_usable_;
- ModelTypeSet initial_sync_ended_;
ProgressMarkerMap download_progress_markers_;
bool is_silenced_;
int num_encryption_conflicts_;
diff --git a/sync/internal_api/public/sessions/sync_session_snapshot_unittest.cc b/sync/internal_api/public/sessions/sync_session_snapshot_unittest.cc
index 5a97a16..9301ffd 100644
--- a/sync/internal_api/public/sessions/sync_session_snapshot_unittest.cc
+++ b/sync/internal_api/public/sessions/sync_session_snapshot_unittest.cc
@@ -33,12 +33,6 @@ TEST_F(SyncSessionSnapshotTest, SyncSessionSnapshotToValue) {
model_neutral.num_local_overwrites = 15;
model_neutral.num_server_overwrites = 18;
- const bool kIsShareUsable = true;
-
- const ModelTypeSet initial_sync_ended(BOOKMARKS, PREFERENCES);
- scoped_ptr<ListValue> expected_initial_sync_ended_value(
- ModelTypeSetToValue(initial_sync_ended));
-
ProgressMarkerMap download_progress_markers;
download_progress_markers[BOOKMARKS] = "test";
download_progress_markers[APPS] = "apps";
@@ -59,8 +53,6 @@ TEST_F(SyncSessionSnapshotTest, SyncSessionSnapshotToValue) {
expected_sources_list_value->Append(source.ToValue());
SyncSessionSnapshot snapshot(model_neutral,
- kIsShareUsable,
- initial_sync_ended,
download_progress_markers,
kIsSilenced,
kNumEncryptionConflicts,
@@ -74,7 +66,7 @@ TEST_F(SyncSessionSnapshotTest, SyncSessionSnapshotToValue) {
std::vector<int>(MODEL_TYPE_COUNT,0),
std::vector<int>(MODEL_TYPE_COUNT, 0));
scoped_ptr<DictionaryValue> value(snapshot.ToValue());
- EXPECT_EQ(20u, value->size());
+ EXPECT_EQ(18u, value->size());
ExpectDictIntegerValue(model_neutral.num_successful_commits,
*value, "numSuccessfulCommits");
ExpectDictIntegerValue(model_neutral.num_successful_bookmark_commits,
@@ -91,9 +83,6 @@ TEST_F(SyncSessionSnapshotTest, SyncSessionSnapshotToValue) {
*value, "numServerOverwrites");
ExpectDictIntegerValue(model_neutral.num_server_changes_remaining,
*value, "numServerChangesRemaining");
- ExpectDictBooleanValue(kIsShareUsable, *value, "isShareUsable");
- ExpectDictListValue(*expected_initial_sync_ended_value, *value,
- "initialSyncEnded");
ExpectDictDictionaryValue(*expected_download_progress_markers_value,
*value, "downloadProgressMarkers");
ExpectDictBooleanValue(kIsSilenced, *value, "isSilenced");
diff --git a/sync/internal_api/public/test/test_entry_factory.h b/sync/internal_api/public/test/test_entry_factory.h
index d5bc911..071a501 100644
--- a/sync/internal_api/public/test/test_entry_factory.h
+++ b/sync/internal_api/public/test/test_entry_factory.h
@@ -56,6 +56,11 @@ class TestEntryFactory {
int64 CreateSyncedItem(const std::string& name,
ModelType model_type, bool is_folder);
+ // Creates a root node that IS_UNAPPLIED. Smiilar to what one would find in
+ // the database between the ProcessUpdates of an initial datatype configure
+ // cycle and the ApplyUpdates step of the same sync cycle.
+ int64 CreateUnappliedRootNode(ModelType model_type);
+
// Looks up the item referenced by |meta_handle|. If successful, overwrites
// the server specifics with |specifics|, sets
// IS_UNAPPLIED_UPDATES/IS_UNSYNCED appropriately, and returns true.
diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc
index ddab0bc..fe5e33b 100644
--- a/sync/internal_api/sync_manager_impl.cc
+++ b/sync/internal_api/sync_manager_impl.cc
@@ -277,7 +277,7 @@ void SyncManagerImpl::ThrowUnrecoverableError() {
}
ModelTypeSet SyncManagerImpl::InitialSyncEndedTypes() {
- return directory()->initial_sync_ended_types();
+ return directory()->InitialSyncEndedTypes();
}
ModelTypeSet SyncManagerImpl::GetTypesWithEmptyProgressMarkerToken(
@@ -572,6 +572,8 @@ bool SyncManagerImpl::PurgePartiallySyncedTypes() {
partially_synced_types.RemoveAll(GetTypesWithEmptyProgressMarkerToken(
ModelTypeSet::All()));
+ DVLOG(1) << "Purging partially synced types "
+ << ModelTypeSetToString(partially_synced_types);
UMA_HISTOGRAM_COUNTS("Sync.PartiallySyncedTypes",
partially_synced_types.Size());
if (partially_synced_types.Empty())
diff --git a/sync/internal_api/sync_manager_impl_unittest.cc b/sync/internal_api/sync_manager_impl_unittest.cc
index e77eb71..edacea0 100644
--- a/sync/internal_api/sync_manager_impl_unittest.cc
+++ b/sync/internal_api/sync_manager_impl_unittest.cc
@@ -32,6 +32,7 @@
#include "sync/internal_api/public/http_post_provider_interface.h"
#include "sync/internal_api/public/read_node.h"
#include "sync/internal_api/public/read_transaction.h"
+#include "sync/internal_api/public/test/test_entry_factory.h"
#include "sync/internal_api/public/test/test_internal_components_factory.h"
#include "sync/internal_api/public/test/test_user_share.h"
#include "sync/internal_api/public/write_node.h"
@@ -60,6 +61,7 @@
#include "sync/syncable/entry.h"
#include "sync/syncable/mutable_entry.h"
#include "sync/syncable/nigori_util.h"
+#include "sync/syncable/read_transaction.h"
#include "sync/syncable/syncable_id.h"
#include "sync/syncable/write_transaction.h"
#include "sync/test/callback_counter.h"
@@ -84,11 +86,12 @@ using testing::StrictMock;
namespace syncer {
using sessions::SyncSessionSnapshot;
+using syncable::GET_BY_HANDLE;
using syncable::IS_DEL;
using syncable::IS_UNSYNCED;
-using syncable::kEncryptedString;
using syncable::NON_UNIQUE_NAME;
using syncable::SPECIFICS;
+using syncable::kEncryptedString;
namespace {
@@ -888,7 +891,6 @@ class SyncManagerTest : public testing::Test,
bool SetUpEncryption(NigoriStatus nigori_status,
EncryptionStatus encryption_status) {
UserShare* share = sync_manager_.GetUserShare();
- share->directory->set_initial_sync_ended_for_type(NIGORI, true);
// We need to create the nigori node as if it were an applied server update.
int64 nigori_id = GetIdForDataType(NIGORI);
@@ -1012,10 +1014,6 @@ class SyncManagerTest : public testing::Test,
}
}
- void SetInitialSyncEndedForType(ModelType type, bool value) {
- sync_manager_.directory()->set_initial_sync_ended_for_type(type, value);
- }
-
InternalComponentsFactory::Switches GetSwitches() const {
return switches_;
}
@@ -2855,7 +2853,6 @@ TEST_F(SyncManagerTestWithMockScheduler, MAYBE_BasicConfiguration) {
for (ModelTypeSet::Iterator iter = ModelTypeSet::All().First(); iter.Good();
iter.Inc()) {
SetProgressMarkerForType(iter.Get(), true);
- SetInitialSyncEndedForType(iter.Get(), true);
}
CallbackCounter ready_task_counter, retry_task_counter;
@@ -2906,10 +2903,8 @@ TEST_F(SyncManagerTestWithMockScheduler, ReConfiguration) {
iter.Inc()) {
if (!disabled_types.Has(iter.Get())) {
SetProgressMarkerForType(iter.Get(), true);
- SetInitialSyncEndedForType(iter.Get(), true);
} else {
SetProgressMarkerForType(iter.Get(), false);
- SetInitialSyncEndedForType(iter.Get(), false);
}
}
@@ -2933,8 +2928,6 @@ TEST_F(SyncManagerTestWithMockScheduler, ReConfiguration) {
EXPECT_EQ(new_routing_info, params.routing_info);
// Verify only the recently disabled types were purged.
- EXPECT_TRUE(sync_manager_.InitialSyncEndedTypes().Equals(
- Difference(ModelTypeSet::All(), disabled_types)));
EXPECT_TRUE(sync_manager_.GetTypesWithEmptyProgressMarkerToken(
ModelTypeSet::All()).Equals(disabled_types));
}
@@ -2968,51 +2961,81 @@ TEST_F(SyncManagerTestWithMockScheduler, ConfigurationRetry) {
EXPECT_EQ(new_routing_info, params.routing_info);
}
-// Test that PurgePartiallySyncedTypes purges only those types that don't
-// have empty progress marker and don't have initial sync ended set.
+// Test that PurgePartiallySyncedTypes purges only those types that have not
+// fully completed their initial download and apply.
TEST_F(SyncManagerTest, PurgePartiallySyncedTypes) {
+ ModelSafeRoutingInfo routing_info;
+ GetModelSafeRoutingInfo(&routing_info);
+ ModelTypeSet enabled_types = GetRoutingInfoTypes(routing_info);
+
UserShare* share = sync_manager_.GetUserShare();
- // Set Nigori and Bookmarks to be partial types.
- sync_pb::DataTypeProgressMarker nigori_marker;
- nigori_marker.set_data_type_id(
- GetSpecificsFieldNumberFromModelType(NIGORI));
- nigori_marker.set_token("token");
- sync_pb::DataTypeProgressMarker bookmark_marker;
- bookmark_marker.set_data_type_id(
- GetSpecificsFieldNumberFromModelType(BOOKMARKS));
- bookmark_marker.set_token("token");
- share->directory->SetDownloadProgress(NIGORI, nigori_marker);
- share->directory->SetDownloadProgress(BOOKMARKS, bookmark_marker);
-
- // Set Preferences to be a full type.
- sync_pb::DataTypeProgressMarker pref_marker;
- pref_marker.set_data_type_id(
- GetSpecificsFieldNumberFromModelType(PREFERENCES));
- pref_marker.set_token("token");
- share->directory->SetDownloadProgress(PREFERENCES, pref_marker);
- share->directory->set_initial_sync_ended_for_type(PREFERENCES, true);
+ // The test harness automatically initializes all types in the routing info.
+ // Check that autofill is not among them.
+ ASSERT_FALSE(enabled_types.Has(AUTOFILL));
- ModelTypeSet partial_types =
- sync_manager_.GetTypesWithEmptyProgressMarkerToken(ModelTypeSet::All());
- EXPECT_FALSE(partial_types.Has(NIGORI));
- EXPECT_FALSE(partial_types.Has(BOOKMARKS));
- EXPECT_FALSE(partial_types.Has(PREFERENCES));
+ // Further ensure that the test harness did not create its root node.
+ {
+ syncable::ReadTransaction trans(FROM_HERE, share->directory.get());
+ syncable::Entry autofill_root_node(&trans, syncable::GET_BY_SERVER_TAG,
+ ModelTypeToRootTag(AUTOFILL));
+ ASSERT_FALSE(autofill_root_node.good());
+ }
+
+ // One more redundant check.
+ ASSERT_FALSE(sync_manager_.InitialSyncEndedTypes().Has(AUTOFILL));
+
+ // Give autofill a progress marker.
+ sync_pb::DataTypeProgressMarker autofill_marker;
+ autofill_marker.set_data_type_id(
+ GetSpecificsFieldNumberFromModelType(AUTOFILL));
+ autofill_marker.set_token("token");
+ share->directory->SetDownloadProgress(AUTOFILL, autofill_marker);
+
+ // Also add a pending autofill root node update from the server.
+ TestEntryFactory factory_(share->directory.get());
+ int autofill_meta = factory_.CreateUnappliedRootNode(AUTOFILL);
+
+ // Preferences is an enabled type. Check that the harness initialized it.
+ ASSERT_TRUE(enabled_types.Has(PREFERENCES));
+ ASSERT_TRUE(sync_manager_.InitialSyncEndedTypes().Has(PREFERENCES));
+ // Give preferencse a progress marker.
+ sync_pb::DataTypeProgressMarker prefs_marker;
+ prefs_marker.set_data_type_id(
+ GetSpecificsFieldNumberFromModelType(PREFERENCES));
+ prefs_marker.set_token("token");
+ share->directory->SetDownloadProgress(PREFERENCES, prefs_marker);
+
+ // Add a fully synced preferences node under the root.
+ std::string pref_client_tag = "prefABC";
+ std::string pref_hashed_tag = "hashXYZ";
+ sync_pb::EntitySpecifics pref_specifics;
+ AddDefaultFieldValue(PREFERENCES, &pref_specifics);
+ int pref_meta = MakeServerNode(
+ share, PREFERENCES, pref_client_tag, pref_hashed_tag, pref_specifics);
+
+ // And now, the purge.
EXPECT_TRUE(sync_manager_.PurgePartiallySyncedTypes());
- // Ensure only bookmarks and nigori lost their progress marker. Preferences
- // should still have it.
- partial_types =
+ // Ensure that autofill lost its progress marker, but preferences did not.
+ ModelTypeSet empty_tokens =
sync_manager_.GetTypesWithEmptyProgressMarkerToken(ModelTypeSet::All());
- EXPECT_TRUE(partial_types.Has(NIGORI));
- EXPECT_TRUE(partial_types.Has(BOOKMARKS));
- EXPECT_FALSE(partial_types.Has(PREFERENCES));
+ EXPECT_TRUE(empty_tokens.Has(AUTOFILL));
+ EXPECT_FALSE(empty_tokens.Has(PREFERENCES));
+
+ // Ensure that autofill lots its node, but preferences did not.
+ {
+ syncable::ReadTransaction trans(FROM_HERE, share->directory.get());
+ syncable::Entry autofill_node(&trans, GET_BY_HANDLE, autofill_meta);
+ syncable::Entry pref_node(&trans, GET_BY_HANDLE, pref_meta);
+ EXPECT_FALSE(autofill_node.good());
+ EXPECT_TRUE(pref_node.good());
+ }
}
// Test CleanupDisabledTypes properly purges all disabled types as specified
-// by the previous and current enabled params. Enabled partial types should not
-// be purged.
+// by the previous and current enabled params.
// Fails on Windows: crbug.com/139726
#if defined(OS_WIN)
#define MAYBE_PurgeDisabledTypes DISABLED_PurgeDisabledTypes
@@ -3024,21 +3047,20 @@ TEST_F(SyncManagerTest, MAYBE_PurgeDisabledTypes) {
GetModelSafeRoutingInfo(&routing_info);
ModelTypeSet enabled_types = GetRoutingInfoTypes(routing_info);
ModelTypeSet disabled_types = Difference(ModelTypeSet::All(), enabled_types);
- ModelTypeSet partial_enabled_types(PASSWORDS);
- // Set data for all non-partial types.
+ // The harness should have initialized the enabled_types for us.
+ EXPECT_TRUE(enabled_types.Equals(sync_manager_.InitialSyncEndedTypes()));
+
+ // Set progress markers for all types.
for (ModelTypeSet::Iterator iter = ModelTypeSet::All().First(); iter.Good();
iter.Inc()) {
SetProgressMarkerForType(iter.Get(), true);
- if (!partial_enabled_types.Has(iter.Get()))
- SetInitialSyncEndedForType(iter.Get(), true);
}
// Verify all the enabled types remain after cleanup, and all the disabled
// types were purged.
sync_manager_.PurgeDisabledTypes(ModelTypeSet::All(), enabled_types);
- EXPECT_TRUE(enabled_types.Equals(
- Union(sync_manager_.InitialSyncEndedTypes(), partial_enabled_types)));
+ EXPECT_TRUE(enabled_types.Equals(sync_manager_.InitialSyncEndedTypes()));
EXPECT_TRUE(disabled_types.Equals(
sync_manager_.GetTypesWithEmptyProgressMarkerToken(ModelTypeSet::All())));
@@ -3050,8 +3072,7 @@ TEST_F(SyncManagerTest, MAYBE_PurgeDisabledTypes) {
// Verify only the non-disabled types remain after cleanup.
sync_manager_.PurgeDisabledTypes(enabled_types, new_enabled_types);
- EXPECT_TRUE(new_enabled_types.Equals(
- Union(sync_manager_.InitialSyncEndedTypes(), partial_enabled_types)));
+ EXPECT_TRUE(new_enabled_types.Equals(sync_manager_.InitialSyncEndedTypes()));
EXPECT_TRUE(disabled_types.Equals(
sync_manager_.GetTypesWithEmptyProgressMarkerToken(ModelTypeSet::All())));
}
diff --git a/sync/internal_api/test/test_entry_factory.cc b/sync/internal_api/test/test_entry_factory.cc
index 3cf0dcf..f578412 100644
--- a/sync/internal_api/test/test_entry_factory.cc
+++ b/sync/internal_api/test/test_entry_factory.cc
@@ -164,6 +164,28 @@ int64 TestEntryFactory::CreateSyncedItem(
return entry.Get(syncable::META_HANDLE);
}
+int64 TestEntryFactory::CreateUnappliedRootNode(
+ ModelType model_type) {
+ syncable::WriteTransaction trans(FROM_HERE, syncable::UNITTEST, directory_);
+ sync_pb::EntitySpecifics specifics;
+ AddDefaultFieldValue(model_type, &specifics);
+ syncable::Id node_id = TestIdFactory::MakeServer("xyz");
+ syncable::MutableEntry entry(&trans, syncable::CREATE_NEW_UPDATE_ITEM,
+ node_id);
+ DCHECK(entry.good());
+ // Make it look like sort of like a pending creation from the server.
+ // The SERVER_PARENT_ID and UNIQUE_CLIENT_TAG aren't quite right, but
+ // it's good enough for our purposes.
+ entry.Put(syncable::SERVER_VERSION, 1);
+ entry.Put(syncable::IS_UNAPPLIED_UPDATE, true);
+ entry.Put(syncable::SERVER_IS_DIR, false);
+ entry.Put(syncable::SERVER_PARENT_ID, TestIdFactory::root());
+ entry.Put(syncable::SERVER_SPECIFICS, specifics);
+ entry.Put(syncable::NON_UNIQUE_NAME, "xyz");
+
+ return entry.Get(syncable::META_HANDLE);
+}
+
bool TestEntryFactory::SetServerSpecificsForItem(
int64 meta_handle,
const sync_pb::EntitySpecifics specifics) {
diff --git a/sync/internal_api/test/test_user_share.cc b/sync/internal_api/test/test_user_share.cc
index 5c7856e..a6147c4 100644
--- a/sync/internal_api/test/test_user_share.cc
+++ b/sync/internal_api/test/test_user_share.cc
@@ -10,6 +10,7 @@
#include "sync/syncable/write_transaction.h"
#include "sync/test/engine/test_directory_setter_upper.h"
#include "sync/test/engine/test_id_factory.h"
+#include "sync/test/engine/test_syncable_utils.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace syncer {
@@ -53,27 +54,8 @@ syncable::TestTransactionObserver* TestUserShare::transaction_observer() {
/* static */
bool TestUserShare::CreateRoot(ModelType model_type, UserShare* user_share) {
syncer::syncable::Directory* directory = user_share->directory.get();
-
- std::string tag_name = syncer::ModelTypeToRootTag(model_type);
-
syncable::WriteTransaction wtrans(FROM_HERE, syncable::UNITTEST, directory);
- syncable::MutableEntry node(&wtrans,
- syncable::CREATE,
- wtrans.root_id(),
- tag_name);
- node.Put(syncable::UNIQUE_SERVER_TAG, tag_name);
- node.Put(syncable::IS_DIR, true);
- node.Put(syncable::SERVER_IS_DIR, false);
- node.Put(syncable::IS_UNSYNCED, false);
- node.Put(syncable::IS_UNAPPLIED_UPDATE, false);
- node.Put(syncable::SERVER_VERSION, 20);
- node.Put(syncable::BASE_VERSION, 20);
- node.Put(syncable::IS_DEL, false);
- node.Put(syncable::ID, syncer::TestIdFactory::MakeServer(tag_name));
- sync_pb::EntitySpecifics specifics;
- syncer::AddDefaultFieldValue(model_type, &specifics);
- node.Put(syncable::SPECIFICS, specifics);
-
+ CreateTypeRoot(&wtrans, directory, model_type);
return true;
}
diff --git a/sync/sessions/sync_session.cc b/sync/sessions/sync_session.cc
index 0732329..f9241d9 100644
--- a/sync/sessions/sync_session.cc
+++ b/sync/sessions/sync_session.cc
@@ -150,17 +150,9 @@ void SyncSession::RebaseRoutingInfoWithLatest(
SyncSessionSnapshot SyncSession::TakeSnapshot() const {
syncable::Directory* dir = context_->directory();
- bool is_share_useable = true;
- ModelTypeSet initial_sync_ended;
ProgressMarkerMap download_progress_markers;
for (int i = FIRST_REAL_MODEL_TYPE; i < MODEL_TYPE_COUNT; ++i) {
ModelType type(ModelTypeFromInt(i));
- if (routing_info_.count(type) != 0) {
- if (dir->initial_sync_ended_for_type(type))
- initial_sync_ended.Put(type);
- else
- is_share_useable = false;
- }
dir->GetDownloadProgressAsString(type, &download_progress_markers[type]);
}
@@ -171,8 +163,6 @@ SyncSessionSnapshot SyncSession::TakeSnapshot() const {
SyncSessionSnapshot snapshot(
status_controller_->model_neutral_state(),
- is_share_useable,
- initial_sync_ended,
download_progress_markers,
delegate_->IsSyncingCurrentlySilenced(),
status_controller_->num_encryption_conflicts(),
diff --git a/sync/syncable/directory.cc b/sync/syncable/directory.cc
index 21e05eb..82eb709c 100644
--- a/sync/syncable/directory.cc
+++ b/sync/syncable/directory.cc
@@ -596,7 +596,6 @@ bool Directory::PurgeEntriesWithTypeIn(ModelTypeSet types) {
// Ensure meta tracking for these data types reflects the deleted state.
for (ModelTypeSet::Iterator it = types.First();
it.Good(); it.Inc()) {
- set_initial_sync_ended_for_type_unsafe(it.Get(), false);
kernel_->persisted_info.reset_download_progress(it.Get());
kernel_->persisted_info.transaction_version[it.Get()] = 0;
}
@@ -667,14 +666,30 @@ void Directory::IncrementTransactionVersion(ModelType type) {
kernel_->persisted_info.transaction_version[type]++;
}
-ModelTypeSet Directory::initial_sync_ended_types() const {
- ScopedKernelLock lock(this);
- return kernel_->persisted_info.initial_sync_ended;
+ModelTypeSet Directory::InitialSyncEndedTypes() {
+ syncable::ReadTransaction trans(FROM_HERE, this);
+ const ModelTypeSet all_types = ModelTypeSet::All();
+ ModelTypeSet initial_sync_ended_types;
+ for (ModelTypeSet::Iterator i = all_types.First(); i.Good(); i.Inc()) {
+ if (InitialSyncEndedForType(&trans, i.Get())) {
+ initial_sync_ended_types.Put(i.Get());
+ }
+ }
+ return initial_sync_ended_types;
}
-bool Directory::initial_sync_ended_for_type(ModelType type) const {
- ScopedKernelLock lock(this);
- return kernel_->persisted_info.initial_sync_ended.Has(type);
+bool Directory::InitialSyncEndedForType(ModelType type) {
+ syncable::ReadTransaction trans(FROM_HERE, this);
+ return InitialSyncEndedForType(&trans, type);
+}
+
+bool Directory::InitialSyncEndedForType(
+ BaseTransaction* trans, ModelType type) {
+ // True iff the type's root node has been received and applied.
+ syncable::Entry entry(trans,
+ syncable::GET_BY_SERVER_TAG,
+ ModelTypeToRootTag(type));
+ return entry.good() && entry.Get(syncable::BASE_VERSION) != CHANGES_VERSION;
}
template <class T> void Directory::TestAndSet(
@@ -685,23 +700,6 @@ template <class T> void Directory::TestAndSet(
}
}
-void Directory::set_initial_sync_ended_for_type(ModelType type, bool x) {
- ScopedKernelLock lock(this);
- set_initial_sync_ended_for_type_unsafe(type, x);
-}
-
-void Directory::set_initial_sync_ended_for_type_unsafe(ModelType type,
- bool x) {
- if (kernel_->persisted_info.initial_sync_ended.Has(type) == x)
- return;
- if (x) {
- kernel_->persisted_info.initial_sync_ended.Put(type);
- } else {
- kernel_->persisted_info.initial_sync_ended.Remove(type);
- }
- kernel_->info_status = KERNEL_SHARE_INFO_DIRTY;
-}
-
void Directory::SetNotificationStateUnsafe(
const std::string& notification_state) {
if (notification_state == kernel_->persisted_info.notification_state)
diff --git a/sync/syncable/directory.h b/sync/syncable/directory.h
index 2b0bd3c4..0eb0ae7 100644
--- a/sync/syncable/directory.h
+++ b/sync/syncable/directory.h
@@ -169,8 +169,6 @@ class SYNC_EXPORT Directory {
// TODO(hatiaol): implement detection and fixing of out-of-sync models.
// Bug 154858.
int64 transaction_version[MODEL_TYPE_COUNT];
- // true iff we ever reached the end of the changelog.
- ModelTypeSet initial_sync_ended;
// The store birthday we were given by the server. Contents are opaque to
// the client.
std::string store_birthday;
@@ -265,9 +263,9 @@ class SYNC_EXPORT Directory {
int64 GetTransactionVersion(ModelType type) const;
void IncrementTransactionVersion(ModelType type);
- ModelTypeSet initial_sync_ended_types() const;
- bool initial_sync_ended_for_type(ModelType type) const;
- void set_initial_sync_ended_for_type(ModelType type, bool value);
+ ModelTypeSet InitialSyncEndedTypes();
+ bool InitialSyncEndedForType(ModelType type);
+ bool InitialSyncEndedForType(BaseTransaction* trans, ModelType type);
const std::string& name() const { return kernel_->name; }
@@ -491,7 +489,6 @@ class SYNC_EXPORT Directory {
// Internal setters that do not acquire a lock internally. These are unsafe
// on their own; caller must guarantee exclusive access manually by holding
// a ScopedKernelLock.
- void set_initial_sync_ended_for_type_unsafe(ModelType type, bool x);
void SetNotificationStateUnsafe(const std::string& notification_state);
Directory& operator = (const Directory&);
diff --git a/sync/syncable/directory_backing_store.cc b/sync/syncable/directory_backing_store.cc
index d072011..a609527 100644
--- a/sync/syncable/directory_backing_store.cc
+++ b/sync/syncable/directory_backing_store.cc
@@ -40,7 +40,7 @@ static const string::size_type kUpdateStatementBufferSize = 2048;
// Increment this version whenever updating DB tables.
extern const int32 kCurrentDBVersion; // Global visibility for our unittest.
-const int32 kCurrentDBVersion = 84;
+const int32 kCurrentDBVersion = 85;
// Iterate over the fields of |entry| and bind each to |statement| for
// updating. Returns the number of args bound.
@@ -197,8 +197,11 @@ bool DirectoryBackingStore::SaveChanges(
// Back out early if there is nothing to write.
bool save_info =
(Directory::KERNEL_SHARE_INFO_DIRTY == snapshot.kernel_info_status);
- if (snapshot.dirty_metas.size() < 1 && !save_info)
+ if (snapshot.dirty_metas.empty()
+ && snapshot.metahandles_to_purge.empty()
+ && !save_info) {
return true;
+ }
sql::Transaction transaction(db_.get());
if (!transaction.Begin())
@@ -236,9 +239,8 @@ bool DirectoryBackingStore::SaveChanges(
sql::Statement s2(db_->GetCachedStatement(
SQL_FROM_HERE,
"INSERT OR REPLACE "
- "INTO models (model_id, progress_marker, initial_sync_ended, "
- " transaction_version) "
- "VALUES (?, ?, ?, ?)"));
+ "INTO models (model_id, progress_marker, transaction_version) "
+ "VALUES (?, ?, ?)"));
for (int i = FIRST_REAL_MODEL_TYPE; i < MODEL_TYPE_COUNT; ++i) {
// We persist not ModelType but rather a protobuf-derived ID.
@@ -247,8 +249,7 @@ bool DirectoryBackingStore::SaveChanges(
info.download_progress[i].SerializeToString(&progress_marker);
s2.BindBlob(0, model_id.data(), model_id.length());
s2.BindBlob(1, progress_marker.data(), progress_marker.length());
- s2.BindBool(2, info.initial_sync_ended.Has(ModelTypeFromInt(i)));
- s2.BindInt64(3, info.transaction_version[i]);
+ s2.BindInt64(2, info.transaction_version[i]);
if (!s2.Run())
return false;
DCHECK_EQ(db_->GetLastChangeCount(), 1);
@@ -370,6 +371,12 @@ bool DirectoryBackingStore::InitializeTables() {
version_on_disk = 84;
}
+ // Version 85 migration removes the initial_sync_ended bits.
+ if (version_on_disk == 84) {
+ if (MigrateVersion84To85())
+ version_on_disk = 85;
+ }
+
// If one of the migrations requested it, drop columns that aren't current.
// It's only safe to do this after migrating all the way to the current
// version.
@@ -379,13 +386,6 @@ bool DirectoryBackingStore::InitializeTables() {
}
// A final, alternative catch-all migration to simply re-sync everything.
- //
- // TODO(rlarocque): It's wrong to recreate the database here unless the higher
- // layers were expecting us to do so. See crbug.com/103824. We must leave
- // this code as is for now because this is the code that ends up creating the
- // database in the first time sync case, where the higher layers are expecting
- // us to create a fresh database. The solution to this should be to implement
- // crbug.com/105018.
if (version_on_disk != kCurrentDBVersion) {
if (version_on_disk > kCurrentDBVersion)
return false;
@@ -508,7 +508,7 @@ bool DirectoryBackingStore::LoadInfo(Directory::KernelLoadInfo* info) {
{
sql::Statement s(
db_->GetUniqueStatement(
- "SELECT model_id, progress_marker, initial_sync_ended, "
+ "SELECT model_id, progress_marker, "
"transaction_version FROM models"));
while (s.Step()) {
@@ -517,9 +517,7 @@ bool DirectoryBackingStore::LoadInfo(Directory::KernelLoadInfo* info) {
if (type != UNSPECIFIED && type != TOP_LEVEL_FOLDER) {
info->kernel_info.download_progress[type].ParseFromArray(
s.ColumnBlob(1), s.ColumnByteLength(1));
- if (s.ColumnBool(2))
- info->kernel_info.initial_sync_ended.Put(type);
- info->kernel_info.transaction_version[type] = s.ColumnInt64(3);
+ info->kernel_info.transaction_version[type] = s.ColumnInt64(2);
}
}
if (!s.Succeeded())
@@ -914,7 +912,7 @@ bool DirectoryBackingStore::MigrateVersion74To75() {
// Move aside the old table and create a new empty one at the current schema.
if (!db_->Execute("ALTER TABLE models RENAME TO temp_models"))
return false;
- if (!CreateModelsTable())
+ if (!CreateV75ModelsTable())
return false;
sql::Statement query(db_->GetUniqueStatement(
@@ -1065,14 +1063,6 @@ bool DirectoryBackingStore::MigrateVersion80To81() {
}
bool DirectoryBackingStore::MigrateVersion81To82() {
- // Version 82 added transaction_version to kernel info. But if user is
- // migrating from 74 or before, 74->75 migration would recreate models table
- // that already has transaction_version column.
- if (db_->DoesColumnExist("models", "transaction_version")) {
- SetVersion(82);
- return true;
- }
-
if (!db_->Execute(
"ALTER TABLE models ADD COLUMN transaction_version BIGINT default 0"))
return false;
@@ -1104,10 +1094,28 @@ bool DirectoryBackingStore::MigrateVersion83To84() {
query.append(ComposeCreateTableColumnSpecs());
if (!db_->Execute(query.c_str()))
return false;
+
SetVersion(84);
return true;
}
+bool DirectoryBackingStore::MigrateVersion84To85() {
+ // Version 84 removes the initial_sync_ended flag.
+ if (!db_->Execute("ALTER TABLE models RENAME TO temp_models"))
+ return false;
+ if (!CreateModelsTable())
+ return false;
+ if (!db_->Execute("INSERT INTO models SELECT "
+ "model_id, progress_marker, transaction_version "
+ "FROM temp_models")) {
+ return false;
+ }
+ SafeDropTable("temp_models");
+
+ SetVersion(85);
+ return true;
+}
+
bool DirectoryBackingStore::CreateTables() {
DVLOG(1) << "First run, creating tables";
// Create two little tables share_version and share_info
@@ -1213,10 +1221,22 @@ bool DirectoryBackingStore::CreateV71ModelsTable() {
"initial_sync_ended BOOLEAN default 0)");
}
+bool DirectoryBackingStore::CreateV75ModelsTable() {
+ // This is an old schema for the Models table, used from versions 75 to 80.
+ return db_->Execute(
+ "CREATE TABLE models ("
+ "model_id BLOB primary key, "
+ "progress_marker BLOB, "
+ // Gets set if the syncer ever gets updates from the
+ // server and the server returns 0. Lets us detect the
+ // end of the initial sync.
+ "initial_sync_ended BOOLEAN default 0)");
+}
+
bool DirectoryBackingStore::CreateModelsTable() {
// This is the current schema for the Models table, from version 81
// onward. If you change the schema, you'll probably want to double-check
- // the use of this function in the v74-v75 migration.
+ // the use of this function in the v84-v85 migration.
return db_->Execute(
"CREATE TABLE models ("
"model_id BLOB primary key, "
@@ -1224,7 +1244,6 @@ bool DirectoryBackingStore::CreateModelsTable() {
// Gets set if the syncer ever gets updates from the
// server and the server returns 0. Lets us detect the
// end of the initial sync.
- "initial_sync_ended BOOLEAN default 0, "
"transaction_version BIGINT default 0)");
}
diff --git a/sync/syncable/directory_backing_store.h b/sync/syncable/directory_backing_store.h
index 4b604ab..8170733 100644
--- a/sync/syncable/directory_backing_store.h
+++ b/sync/syncable/directory_backing_store.h
@@ -84,6 +84,7 @@ class DirectoryBackingStore : public base::NonThreadSafe {
bool CreateMetasTable(bool is_temporary);
bool CreateModelsTable();
bool CreateV71ModelsTable();
+ bool CreateV75ModelsTable();
// We don't need to load any synced and applied deleted entries, we can
// in fact just purge them forever on startup.
@@ -160,6 +161,7 @@ class DirectoryBackingStore : public base::NonThreadSafe {
bool MigrateVersion81To82();
bool MigrateVersion82To83();
bool MigrateVersion83To84();
+ bool MigrateVersion84To85();
scoped_ptr<sql::Connection> db_;
sql::Statement save_entry_statement_;
diff --git a/sync/syncable/directory_backing_store_unittest.cc b/sync/syncable/directory_backing_store_unittest.cc
index fe8e923..1b89ae0 100644
--- a/sync/syncable/directory_backing_store_unittest.cc
+++ b/sync/syncable/directory_backing_store_unittest.cc
@@ -68,9 +68,11 @@ class MigrationTest : public testing::TestWithParam<int> {
void SetUpVersion81Database(sql::Connection* connection);
void SetUpVersion82Database(sql::Connection* connection);
void SetUpVersion83Database(sql::Connection* connection);
+ void SetUpVersion84Database(sql::Connection* connection);
+ void SetUpVersion85Database(sql::Connection* connection);
void SetUpCurrentDatabaseAndCheckVersion(sql::Connection* connection) {
- SetUpVersion77Database(connection); // Prepopulates data.
+ SetUpVersion85Database(connection); // Prepopulates data.
scoped_ptr<TestDirectoryBackingStore> dbs(
new TestDirectoryBackingStore(GetUsername(), connection));
@@ -1968,7 +1970,7 @@ void MigrationTest::SetUpVersion82Database(sql::Connection* connection) {
ASSERT_TRUE(connection->BeginTransaction());
ASSERT_TRUE(connection->Execute(
"CREATE TABLE share_version (id VARCHAR(128) primary key, data INT);"
- "INSERT INTO 'share_version' VALUES('nick@chromium.org',81);"
+ "INSERT INTO 'share_version' VALUES('nick@chromium.org',82);"
"CREATE TABLE models (model_id BLOB primary key, progress_marker BLOB, in"
"itial_sync_ended BOOLEAN default 0, transaction_version BIGINT "
"default 0);"
@@ -2074,7 +2076,7 @@ void MigrationTest::SetUpVersion83Database(sql::Connection* connection) {
ASSERT_TRUE(connection->BeginTransaction());
ASSERT_TRUE(connection->Execute(
"CREATE TABLE share_version (id VARCHAR(128) primary key, data INT);"
- "INSERT INTO 'share_version' VALUES('nick@chromium.org',81);"
+ "INSERT INTO 'share_version' VALUES('nick@chromium.org',83);"
"CREATE TABLE models (model_id BLOB primary key, progress_marker BLOB, in"
"itial_sync_ended BOOLEAN default 0, transaction_version BIGINT "
"default 0);"
@@ -2091,6 +2093,251 @@ void MigrationTest::SetUpVersion83Database(sql::Connection* connection) {
"server_is_del bit default 0,non_unique_name varchar,server_non_uniqu"
"e_name varchar(255),unique_server_tag varchar,unique_client_tag varc"
"har,specifics blob,server_specifics blob, base_server_specifics BLOB"
+ ", server_ordinal_in_parent blob, transaction_version bigint default "
+ "0);"
+ "CREATE TABLE 'share_info' (id TEXT primary key, name TEXT, store_birthda"
+ "y TEXT, db_create_version TEXT, db_create_time INT, next_id INT defa"
+ "ult -2, cache_guid TEXT , notification_state BLOB, bag_of_chips "
+ "blob);"
+ "INSERT INTO 'share_info' VALUES('nick@chromium.org','nick@chromium.org',"
+ "'c27e9f59-08ca-46f8-b0cc-f16a2ed778bb','Unknown',1263522064,"
+ "-131078,'9010788312004066376x-6609234393368420856x',NULL, NULL);"));
+
+ const char* insert_stmts[V80_ROW_COUNT] = {
+ "INSERT INTO 'metas' VALUES(1,-1,0,0," META_PROTO_TIMES_VALS(1) ",'r','"
+ "r','r','r','r',0,0,0,1,0,0,NULL,NULL,NULL,NULL,X'',X'',NULL,?,0);",
+ "INSERT INTO 'metas' VALUES(2,669,669,4,"
+ META_PROTO_TIMES_VALS(2) ",'s_ID_2','s_ID_9','s_ID_9','s_ID_2','s_ID_"
+ "2',0,0,1,0,0,1,'Deleted Item','Deleted Item',NULL,NULL,X'C28810220A1"
+ "6687474703A2F2F7777772E676F6F676C652E636F6D2F12084141534741534741',X"
+ "'C28810260A17687474703A2F2F7777772E676F6F676C652E636F6D2F32120B41534"
+ "14447414447414447',NULL,?,0);",
+ "INSERT INTO 'metas' VALUES(4,681,681,3,"
+ META_PROTO_TIMES_VALS(4) ",'s_ID_4','s_ID_9','s_ID_9','s_ID_4','s_ID_"
+ "4',0,0,1,0,0,1,'Welcome to Chromium','Welcome to Chromium',NULL,NULL"
+ ",X'C28810350A31687474703A2F2F7777772E676F6F676C652E636F6D2F6368726F6"
+ "D652F696E746C2F656E2F77656C636F6D652E68746D6C1200',X'C28810350A31687"
+ "474703A2F2F7777772E676F6F676C652E636F6D2F6368726F6D652F696E746C2F656"
+ "E2F77656C636F6D652E68746D6C1200',NULL,?,0);",
+ "INSERT INTO 'metas' VALUES(5,677,677,7,"
+ META_PROTO_TIMES_VALS(5) ",'s_ID_5','s_ID_9','s_ID_9','s_ID_5','s_ID_"
+ "5',0,0,1,0,0,1,'Google','Google',NULL,NULL,X'C28810220A16687474703A2"
+ "F2F7777772E676F6F676C652E636F6D2F12084147415347415347',X'C28810220A1"
+ "6687474703A2F2F7777772E676F6F676C652E636F6D2F12084147464447415347',N"
+ "ULL,?,0);",
+ "INSERT INTO 'metas' VALUES(6,694,694,6,"
+ META_PROTO_TIMES_VALS(6) ",'s_ID_6','s_ID_9','s_ID_9','r','r',0,0,0,1"
+ ",1,0,'The Internet','The Internet',NULL,NULL,X'C2881000',X'C2881000'"
+ ",NULL,?,0);",
+ "INSERT INTO 'metas' VALUES(7,663,663,0,"
+ META_PROTO_TIMES_VALS(7) ",'s_ID_7','r','r','r','r',0,0,0,1,1,0,'Goog"
+ "le Chrome','Google Chrome','google_chrome',NULL,NULL,NULL,NULL,?,0);"
+ "",
+ "INSERT INTO 'metas' VALUES(8,664,664,0,"
+ META_PROTO_TIMES_VALS(8) ",'s_ID_8','s_ID_7','s_ID_7','r','r',0,0,0,1"
+ ",1,0,'Bookmarks','Bookmarks','google_chrome_bookmarks',NULL,X'C28810"
+ "00',X'C2881000',NULL,?,0);",
+ "INSERT INTO 'metas' VALUES(9,665,665,1,"
+ META_PROTO_TIMES_VALS(9) ",'s_ID_9','s_ID_8','s_ID_8','r','s_ID_10',0"
+ ",0,0,1,1,0,'Bookmark Bar','Bookmark Bar','bookmark_bar',NULL,X'C2881"
+ "000',X'C2881000',NULL,?,0);",
+ "INSERT INTO 'metas' VALUES(10,666,666,2,"
+ META_PROTO_TIMES_VALS(10) ",'s_ID_10','s_ID_8','s_ID_8','s_ID_9','r',"
+ "0,0,0,1,1,0,'Other Bookmarks','Other Bookmarks','other_bookmarks',NU"
+ "LL,X'C2881000',X'C2881000',NULL,?,0);",
+ "INSERT INTO 'metas' VALUES(11,683,683,8,"
+ META_PROTO_TIMES_VALS(11) ",'s_ID_11','s_ID_6','s_ID_6','r','s_ID_13'"
+ ",0,0,0,0,0,0,'Home (The Chromium Projects)','Home (The Chromium Proj"
+ "ects)',NULL,NULL,X'C28810220A18687474703A2F2F6465762E6368726F6D69756"
+ "D2E6F72672F1206414741545741',X'C28810290A1D687474703A2F2F6465762E636"
+ "8726F6D69756D2E6F72672F6F7468657212084146414756415346',NULL,?,0);",
+ "INSERT INTO 'metas' VALUES(12,685,685,9,"
+ META_PROTO_TIMES_VALS(12) ",'s_ID_12','s_ID_6','s_ID_6','s_ID_13','s_"
+ "ID_14',0,0,0,1,1,0,'Extra Bookmarks','Extra Bookmarks',NULL,NULL,X'C"
+ "2881000',X'C2881000',NULL,?,0);",
+ "INSERT INTO 'metas' VALUES(13,687,687,10,"
+ META_PROTO_TIMES_VALS(13) ",'s_ID_13','s_ID_6','s_ID_6','s_ID_11','s_"
+ "ID_12',0,0,0,0,0,0,'ICANN | Internet Corporation for Assigned Names "
+ "and Numbers','ICANN | Internet Corporation for Assigned Names and Nu"
+ "mbers',NULL,NULL,X'C28810240A15687474703A2F2F7777772E6963616E6E2E636"
+ "F6D2F120B504E474158463041414646',X'C28810200A15687474703A2F2F7777772"
+ "E6963616E6E2E636F6D2F120744414146415346',NULL,?,0);",
+ "INSERT INTO 'metas' VALUES(14,692,692,11,"
+ META_PROTO_TIMES_VALS(14) ",'s_ID_14','s_ID_6','s_ID_6','s_ID_12','r'"
+ ",0,0,0,0,0,0,'The WebKit Open Source Project','The WebKit Open Sourc"
+ "e Project',NULL,NULL,X'C288101A0A12687474703A2F2F7765626B69742E6F726"
+ "72F1204504E4758',X'C288101C0A13687474703A2F2F7765626B69742E6F72672F7"
+ "81205504E473259',NULL,?,0);" };
+
+ for (int i = 0; i < V80_ROW_COUNT; i++) {
+ sql::Statement s(connection->GetUniqueStatement(insert_stmts[i]));
+ std::string ord = V81_Ordinal(i);
+ s.BindBlob(0, ord.data(), ord.length());
+ ASSERT_TRUE(s.Run());
+ s.Reset(true);
+ }
+ ASSERT_TRUE(connection->CommitTransaction());
+}
+
+void MigrationTest::SetUpVersion84Database(sql::Connection* connection) {
+ ASSERT_TRUE(connection->is_open());
+ ASSERT_TRUE(connection->BeginTransaction());
+ ASSERT_TRUE(connection->Execute(
+ "CREATE TABLE share_version (id VARCHAR(128) primary key, data INT);"
+ "INSERT INTO 'share_version' VALUES('nick@chromium.org',84);"
+ "CREATE TABLE models (model_id BLOB primary key, progress_marker BLOB, in"
+ "itial_sync_ended BOOLEAN default 0, transaction_version BIGINT "
+ "default 0);"
+ "INSERT INTO 'models' VALUES(X'C2881000',X'0888810218B605',1, 1);"
+ "CREATE TABLE 'metas'(metahandle bigint primary key ON CONFLICT FAIL,base"
+ "_version bigint default -1,server_version bigint default 0, "
+ "local_external_id bigint default 0"
+ ",mtime bigint default 0,server_mtime bigint default 0,ctime bigint d"
+ "efault 0,server_ctime bigint default 0,id varchar(255) default 'r',p"
+ "arent_id varchar(255) default 'r',server_parent_id varchar(255) defa"
+ "ult 'r',prev_id varchar(255) default 'r',next_id varchar(255) defaul"
+ "t 'r',is_unsynced bit default 0,is_unapplied_update bit default 0,is"
+ "_del bit default 0,is_dir bit default 0,server_is_dir bit default 0,"
+ "server_is_del bit default 0,non_unique_name varchar,server_non_uniqu"
+ "e_name varchar(255),unique_server_tag varchar,unique_client_tag varc"
+ "har,specifics blob,server_specifics blob, base_server_specifics BLOB"
+ ", server_ordinal_in_parent blob, transaction_version bigint default "
+ "0);"
+ "CREATE TABLE 'deleted_metas'"
+ "(metahandle bigint primary key ON CONFLICT FAIL,base"
+ "_version bigint default -1,server_version bigint default 0, "
+ "local_external_id bigint default 0"
+ ",mtime bigint default 0,server_mtime bigint default 0,ctime bigint d"
+ "efault 0,server_ctime bigint default 0,id varchar(255) default 'r',p"
+ "arent_id varchar(255) default 'r',server_parent_id varchar(255) defa"
+ "ult 'r',prev_id varchar(255) default 'r',next_id varchar(255) defaul"
+ "t 'r',is_unsynced bit default 0,is_unapplied_update bit default 0,is"
+ "_del bit default 0,is_dir bit default 0,server_is_dir bit default 0,"
+ "server_is_del bit default 0,non_unique_name varchar,server_non_uniqu"
+ "e_name varchar(255),unique_server_tag varchar,unique_client_tag varc"
+ "har,specifics blob,server_specifics blob, base_server_specifics BLOB"
+ ", server_ordinal_in_parent blob, transaction_verion bigint default 0"
+ ");"
+ "CREATE TABLE 'share_info' (id TEXT primary key, name TEXT, store_birthda"
+ "y TEXT, db_create_version TEXT, db_create_time INT, next_id INT defa"
+ "ult -2, cache_guid TEXT , notification_state BLOB, bag_of_chips "
+ "blob);"
+ "INSERT INTO 'share_info' VALUES('nick@chromium.org','nick@chromium.org',"
+ "'c27e9f59-08ca-46f8-b0cc-f16a2ed778bb','Unknown',1263522064,"
+ "-131078,'9010788312004066376x-6609234393368420856x',NULL, NULL);"));
+
+ const char* insert_stmts[V80_ROW_COUNT] = {
+ "INSERT INTO 'metas' VALUES(1,-1,0,0," META_PROTO_TIMES_VALS(1) ",'r','"
+ "r','r','r','r',0,0,0,1,0,0,NULL,NULL,NULL,NULL,X'',X'',NULL,?,0);",
+ "INSERT INTO 'metas' VALUES(2,669,669,4,"
+ META_PROTO_TIMES_VALS(2) ",'s_ID_2','s_ID_9','s_ID_9','s_ID_2','s_ID_"
+ "2',0,0,1,0,0,1,'Deleted Item','Deleted Item',NULL,NULL,X'C28810220A1"
+ "6687474703A2F2F7777772E676F6F676C652E636F6D2F12084141534741534741',X"
+ "'C28810260A17687474703A2F2F7777772E676F6F676C652E636F6D2F32120B41534"
+ "14447414447414447',NULL,?,0);",
+ "INSERT INTO 'metas' VALUES(4,681,681,3,"
+ META_PROTO_TIMES_VALS(4) ",'s_ID_4','s_ID_9','s_ID_9','s_ID_4','s_ID_"
+ "4',0,0,1,0,0,1,'Welcome to Chromium','Welcome to Chromium',NULL,NULL"
+ ",X'C28810350A31687474703A2F2F7777772E676F6F676C652E636F6D2F6368726F6"
+ "D652F696E746C2F656E2F77656C636F6D652E68746D6C1200',X'C28810350A31687"
+ "474703A2F2F7777772E676F6F676C652E636F6D2F6368726F6D652F696E746C2F656"
+ "E2F77656C636F6D652E68746D6C1200',NULL,?,0);",
+ "INSERT INTO 'metas' VALUES(5,677,677,7,"
+ META_PROTO_TIMES_VALS(5) ",'s_ID_5','s_ID_9','s_ID_9','s_ID_5','s_ID_"
+ "5',0,0,1,0,0,1,'Google','Google',NULL,NULL,X'C28810220A16687474703A2"
+ "F2F7777772E676F6F676C652E636F6D2F12084147415347415347',X'C28810220A1"
+ "6687474703A2F2F7777772E676F6F676C652E636F6D2F12084147464447415347',N"
+ "ULL,?,0);",
+ "INSERT INTO 'metas' VALUES(6,694,694,6,"
+ META_PROTO_TIMES_VALS(6) ",'s_ID_6','s_ID_9','s_ID_9','r','r',0,0,0,1"
+ ",1,0,'The Internet','The Internet',NULL,NULL,X'C2881000',X'C2881000'"
+ ",NULL,?,0);",
+ "INSERT INTO 'metas' VALUES(7,663,663,0,"
+ META_PROTO_TIMES_VALS(7) ",'s_ID_7','r','r','r','r',0,0,0,1,1,0,'Goog"
+ "le Chrome','Google Chrome','google_chrome',NULL,NULL,NULL,NULL,?,0);"
+ "",
+ "INSERT INTO 'metas' VALUES(8,664,664,0,"
+ META_PROTO_TIMES_VALS(8) ",'s_ID_8','s_ID_7','s_ID_7','r','r',0,0,0,1"
+ ",1,0,'Bookmarks','Bookmarks','google_chrome_bookmarks',NULL,X'C28810"
+ "00',X'C2881000',NULL,?,0);",
+ "INSERT INTO 'metas' VALUES(9,665,665,1,"
+ META_PROTO_TIMES_VALS(9) ",'s_ID_9','s_ID_8','s_ID_8','r','s_ID_10',0"
+ ",0,0,1,1,0,'Bookmark Bar','Bookmark Bar','bookmark_bar',NULL,X'C2881"
+ "000',X'C2881000',NULL,?,0);",
+ "INSERT INTO 'metas' VALUES(10,666,666,2,"
+ META_PROTO_TIMES_VALS(10) ",'s_ID_10','s_ID_8','s_ID_8','s_ID_9','r',"
+ "0,0,0,1,1,0,'Other Bookmarks','Other Bookmarks','other_bookmarks',NU"
+ "LL,X'C2881000',X'C2881000',NULL,?,0);",
+ "INSERT INTO 'metas' VALUES(11,683,683,8,"
+ META_PROTO_TIMES_VALS(11) ",'s_ID_11','s_ID_6','s_ID_6','r','s_ID_13'"
+ ",0,0,0,0,0,0,'Home (The Chromium Projects)','Home (The Chromium Proj"
+ "ects)',NULL,NULL,X'C28810220A18687474703A2F2F6465762E6368726F6D69756"
+ "D2E6F72672F1206414741545741',X'C28810290A1D687474703A2F2F6465762E636"
+ "8726F6D69756D2E6F72672F6F7468657212084146414756415346',NULL,?,0);",
+ "INSERT INTO 'metas' VALUES(12,685,685,9,"
+ META_PROTO_TIMES_VALS(12) ",'s_ID_12','s_ID_6','s_ID_6','s_ID_13','s_"
+ "ID_14',0,0,0,1,1,0,'Extra Bookmarks','Extra Bookmarks',NULL,NULL,X'C"
+ "2881000',X'C2881000',NULL,?,0);",
+ "INSERT INTO 'metas' VALUES(13,687,687,10,"
+ META_PROTO_TIMES_VALS(13) ",'s_ID_13','s_ID_6','s_ID_6','s_ID_11','s_"
+ "ID_12',0,0,0,0,0,0,'ICANN | Internet Corporation for Assigned Names "
+ "and Numbers','ICANN | Internet Corporation for Assigned Names and Nu"
+ "mbers',NULL,NULL,X'C28810240A15687474703A2F2F7777772E6963616E6E2E636"
+ "F6D2F120B504E474158463041414646',X'C28810200A15687474703A2F2F7777772"
+ "E6963616E6E2E636F6D2F120744414146415346',NULL,?,0);",
+ "INSERT INTO 'metas' VALUES(14,692,692,11,"
+ META_PROTO_TIMES_VALS(14) ",'s_ID_14','s_ID_6','s_ID_6','s_ID_12','r'"
+ ",0,0,0,0,0,0,'The WebKit Open Source Project','The WebKit Open Sourc"
+ "e Project',NULL,NULL,X'C288101A0A12687474703A2F2F7765626B69742E6F726"
+ "72F1204504E4758',X'C288101C0A13687474703A2F2F7765626B69742E6F72672F7"
+ "81205504E473259',NULL,?,0);" };
+
+ for (int i = 0; i < V80_ROW_COUNT; i++) {
+ sql::Statement s(connection->GetUniqueStatement(insert_stmts[i]));
+ std::string ord = V81_Ordinal(i);
+ s.BindBlob(0, ord.data(), ord.length());
+ ASSERT_TRUE(s.Run());
+ s.Reset(true);
+ }
+ ASSERT_TRUE(connection->CommitTransaction());
+}
+
+void MigrationTest::SetUpVersion85Database(sql::Connection* connection) {
+ ASSERT_TRUE(connection->is_open());
+ ASSERT_TRUE(connection->BeginTransaction());
+ ASSERT_TRUE(connection->Execute(
+ "CREATE TABLE share_version (id VARCHAR(128) primary key, data INT);"
+ "INSERT INTO 'share_version' VALUES('nick@chromium.org',85);"
+ "CREATE TABLE models (model_id BLOB primary key, progress_marker BLOB, "
+ "transaction_version BIGINT default 0);"
+ "INSERT INTO 'models' VALUES(X'C2881000',X'0888810218B605', 1);"
+ "CREATE TABLE 'metas'(metahandle bigint primary key ON CONFLICT FAIL,base"
+ "_version bigint default -1,server_version bigint default 0, "
+ "local_external_id bigint default 0"
+ ",mtime bigint default 0,server_mtime bigint default 0,ctime bigint d"
+ "efault 0,server_ctime bigint default 0,id varchar(255) default 'r',p"
+ "arent_id varchar(255) default 'r',server_parent_id varchar(255) defa"
+ "ult 'r',prev_id varchar(255) default 'r',next_id varchar(255) defaul"
+ "t 'r',is_unsynced bit default 0,is_unapplied_update bit default 0,is"
+ "_del bit default 0,is_dir bit default 0,server_is_dir bit default 0,"
+ "server_is_del bit default 0,non_unique_name varchar,server_non_uniqu"
+ "e_name varchar(255),unique_server_tag varchar,unique_client_tag varc"
+ "har,specifics blob,server_specifics blob, base_server_specifics BLOB"
+ ", server_ordinal_in_parent blob, transaction_version bigint default "
+ "0);"
+ "CREATE TABLE 'deleted_metas'"
+ "(metahandle bigint primary key ON CONFLICT FAIL,base"
+ "_version bigint default -1,server_version bigint default 0, "
+ "local_external_id bigint default 0"
+ ",mtime bigint default 0,server_mtime bigint default 0,ctime bigint d"
+ "efault 0,server_ctime bigint default 0,id varchar(255) default 'r',p"
+ "arent_id varchar(255) default 'r',server_parent_id varchar(255) defa"
+ "ult 'r',prev_id varchar(255) default 'r',next_id varchar(255) defaul"
+ "t 'r',is_unsynced bit default 0,is_unapplied_update bit default 0,is"
+ "_del bit default 0,is_dir bit default 0,server_is_dir bit default 0,"
+ "server_is_del bit default 0,non_unique_name varchar,server_non_uniqu"
+ "e_name varchar(255),unique_server_tag varchar,unique_client_tag varc"
+ "har,specifics blob,server_specifics blob, base_server_specifics BLOB"
", server_ordinal_in_parent blob, transaction_verion bigint default 0"
");"
"CREATE TABLE 'share_info' (id TEXT primary key, name TEXT, store_birthda"
@@ -2632,6 +2879,19 @@ TEST_F(DirectoryBackingStoreTest, MigrateVersion83To84) {
ASSERT_TRUE(connection.DoesTableExist("deleted_metas"));
}
+TEST_F(DirectoryBackingStoreTest, MigrateVersion84To85) {
+ sql::Connection connection;
+ ASSERT_TRUE(connection.OpenInMemory());
+ SetUpVersion84Database(&connection);
+ ASSERT_TRUE(connection.DoesColumnExist("models", "initial_sync_ended"));
+
+ scoped_ptr<TestDirectoryBackingStore> dbs(
+ new TestDirectoryBackingStore(GetUsername(), &connection));
+ ASSERT_TRUE(dbs->MigrateVersion84To85());
+ ASSERT_EQ(85, dbs->GetVersion());
+ ASSERT_FALSE(connection.DoesColumnExist("models", "initial_sync_ended"));
+}
+
TEST_P(MigrationTest, ToCurrentVersion) {
sql::Connection connection;
ASSERT_TRUE(connection.OpenInMemory());
@@ -2687,6 +2947,9 @@ TEST_P(MigrationTest, ToCurrentVersion) {
case 83:
SetUpVersion83Database(&connection);
break;
+ case 84:
+ SetUpVersion84Database(&connection);
+ break;
default:
// If you see this error, it may mean that you've increased the
// database version number but you haven't finished adding unit tests
@@ -2772,6 +3035,12 @@ TEST_P(MigrationTest, ToCurrentVersion) {
// Column added in version 83.
ASSERT_TRUE(connection.DoesColumnExist("metas", "transaction_version"));
+ // Table added in version 84.
+ ASSERT_TRUE(connection.DoesTableExist("deleted_metas"));
+
+ // Column removed in version 85.
+ ASSERT_FALSE(connection.DoesColumnExist("models", "initial_sync_ended"));
+
// Check download_progress state (v75 migration)
ASSERT_EQ(694,
dir_info.kernel_info.download_progress[BOOKMARKS]
diff --git a/sync/syncable/syncable_unittest.cc b/sync/syncable/syncable_unittest.cc
index 2a089ac..077afcd 100644
--- a/sync/syncable/syncable_unittest.cc
+++ b/sync/syncable/syncable_unittest.cc
@@ -460,9 +460,9 @@ class SyncableDirectoryTest : public testing::Test {
ReadTransaction trans(FROM_HERE, dir_.get());
MetahandleSet all_set;
dir_->GetAllMetaHandles(&trans, &all_set);
- EXPECT_EQ(3U, all_set.size());
+ EXPECT_EQ(4U, all_set.size());
if (before_reload)
- EXPECT_EQ(4U, dir_->kernel_->metahandles_to_purge->size());
+ EXPECT_EQ(6U, dir_->kernel_->metahandles_to_purge->size());
for (MetahandleSet::iterator iter = all_set.begin();
iter != all_set.end(); ++iter) {
Entry e(&trans, GET_BY_HANDLE, *iter);
@@ -481,10 +481,10 @@ class SyncableDirectoryTest : public testing::Test {
for (ModelTypeSet::Iterator it = types_to_purge.First();
it.Good(); it.Inc()) {
- EXPECT_FALSE(dir_->initial_sync_ended_for_type(it.Get()));
+ EXPECT_FALSE(dir_->InitialSyncEndedForType(it.Get()));
}
EXPECT_FALSE(types_to_purge.Has(BOOKMARKS));
- EXPECT_TRUE(dir_->initial_sync_ended_for_type(BOOKMARKS));
+ EXPECT_TRUE(dir_->InitialSyncEndedForType(BOOKMARKS));
}
FakeEncryptor encryptor_;
@@ -1550,9 +1550,6 @@ TEST_F(OnDiskSyncableDirectoryTest, TestPurgeEntriesWithTypeIn) {
AddDefaultFieldValue(BOOKMARKS, &bookmark_specs);
AddDefaultFieldValue(PREFERENCES, &preference_specs);
AddDefaultFieldValue(AUTOFILL, &autofill_specs);
- dir_->set_initial_sync_ended_for_type(BOOKMARKS, true);
- dir_->set_initial_sync_ended_for_type(PREFERENCES, true);
- dir_->set_initial_sync_ended_for_type(AUTOFILL, true);
ModelTypeSet types_to_purge(PREFERENCES, AUTOFILL);
@@ -1560,6 +1557,15 @@ TEST_F(OnDiskSyncableDirectoryTest, TestPurgeEntriesWithTypeIn) {
// Create some items for each type.
{
WriteTransaction trans(FROM_HERE, UNITTEST, dir_.get());
+
+ // Make it look like these types have completed initial sync.
+ CreateTypeRoot(&trans, dir_.get(), BOOKMARKS);
+ CreateTypeRoot(&trans, dir_.get(), PREFERENCES);
+ CreateTypeRoot(&trans, dir_.get(), AUTOFILL);
+
+ // Add more nodes for this type. Technically, they should be placed under
+ // the proper type root nodes but the assertions in this test won't notice
+ // if their parent isn't quite right.
MutableEntry item1(&trans, CREATE, trans.root_id(), "Item");
ASSERT_TRUE(item1.good());
item1.Put(SPECIFICS, bookmark_specs);
@@ -1602,7 +1608,7 @@ TEST_F(OnDiskSyncableDirectoryTest, TestPurgeEntriesWithTypeIn) {
ReadTransaction trans(FROM_HERE, dir_.get());
MetahandleSet all_set;
GetAllMetaHandles(&trans, &all_set);
- ASSERT_EQ(7U, all_set.size());
+ ASSERT_EQ(10U, all_set.size());
}
dir_->PurgeEntriesWithTypeIn(types_to_purge);
@@ -1615,7 +1621,6 @@ TEST_F(OnDiskSyncableDirectoryTest, TestPurgeEntriesWithTypeIn) {
}
TEST_F(OnDiskSyncableDirectoryTest, TestShareInfo) {
- dir_->set_initial_sync_ended_for_type(AUTOFILL, true);
dir_->set_store_birthday("Jan 31st");
dir_->SetNotificationState("notification_state");
const char* const bag_of_chips_array = "\0bag of chips";
@@ -1624,8 +1629,6 @@ TEST_F(OnDiskSyncableDirectoryTest, TestShareInfo) {
dir_->set_bag_of_chips(bag_of_chips_string);
{
ReadTransaction trans(FROM_HERE, dir_.get());
- EXPECT_TRUE(dir_->initial_sync_ended_for_type(AUTOFILL));
- EXPECT_FALSE(dir_->initial_sync_ended_for_type(BOOKMARKS));
EXPECT_EQ("Jan 31st", dir_->store_birthday());
EXPECT_EQ("notification_state", dir_->GetNotificationState());
EXPECT_EQ(bag_of_chips_string, dir_->bag_of_chips());
@@ -1639,8 +1642,6 @@ TEST_F(OnDiskSyncableDirectoryTest, TestShareInfo) {
dir_->SaveChanges();
{
ReadTransaction trans(FROM_HERE, dir_.get());
- EXPECT_TRUE(dir_->initial_sync_ended_for_type(AUTOFILL));
- EXPECT_FALSE(dir_->initial_sync_ended_for_type(BOOKMARKS));
EXPECT_EQ("April 10th", dir_->store_birthday());
EXPECT_EQ("notification_state2", dir_->GetNotificationState());
EXPECT_EQ(bag_of_chips2_string, dir_->bag_of_chips());
@@ -1650,8 +1651,6 @@ TEST_F(OnDiskSyncableDirectoryTest, TestShareInfo) {
SaveAndReloadDir();
{
ReadTransaction trans(FROM_HERE, dir_.get());
- EXPECT_TRUE(dir_->initial_sync_ended_for_type(AUTOFILL));
- EXPECT_FALSE(dir_->initial_sync_ended_for_type(BOOKMARKS));
EXPECT_EQ("April 10th", dir_->store_birthday());
EXPECT_EQ("notification_state2", dir_->GetNotificationState());
EXPECT_EQ(bag_of_chips2_string, dir_->bag_of_chips());
diff --git a/sync/test/engine/test_syncable_utils.cc b/sync/test/engine/test_syncable_utils.cc
index 3873420..e5ff285 100644
--- a/sync/test/engine/test_syncable_utils.cc
+++ b/sync/test/engine/test_syncable_utils.cc
@@ -9,6 +9,9 @@
#include "sync/syncable/base_transaction.h"
#include "sync/syncable/directory.h"
#include "sync/syncable/entry.h"
+#include "sync/syncable/mutable_entry.h"
+#include "sync/syncable/write_transaction.h"
+#include "sync/test/engine/test_id_factory.h"
using std::string;
@@ -62,5 +65,29 @@ Id GetOnlyEntryWithName(BaseTransaction* rtrans,
return GetFirstEntryWithName(rtrans, parent_id, name);
}
+void CreateTypeRoot(WriteTransaction* trans,
+ syncable::Directory *dir,
+ ModelType type) {
+ std::string tag_name = syncer::ModelTypeToRootTag(type);
+ syncable::MutableEntry node(trans,
+ syncable::CREATE,
+ TestIdFactory::root(),
+ tag_name);
+ DCHECK(node.good());
+ node.Put(syncable::UNIQUE_SERVER_TAG, tag_name);
+ node.Put(syncable::IS_DIR, true);
+ node.Put(syncable::SERVER_IS_DIR, false);
+ node.Put(syncable::IS_UNSYNCED, false);
+ node.Put(syncable::IS_UNAPPLIED_UPDATE, false);
+ node.Put(syncable::SERVER_VERSION, 20);
+ node.Put(syncable::BASE_VERSION, 20);
+ node.Put(syncable::IS_DEL, false);
+ node.Put(syncable::ID, syncer::TestIdFactory::MakeServer(tag_name));
+ sync_pb::EntitySpecifics specifics;
+ syncer::AddDefaultFieldValue(type, &specifics);
+ node.Put(syncable::SERVER_SPECIFICS, specifics);
+ node.Put(syncable::SPECIFICS, specifics);
+}
+
} // namespace syncable
} // namespace syncer
diff --git a/sync/test/engine/test_syncable_utils.h b/sync/test/engine/test_syncable_utils.h
index 5076ebb..7f162f7 100644
--- a/sync/test/engine/test_syncable_utils.h
+++ b/sync/test/engine/test_syncable_utils.h
@@ -10,11 +10,15 @@
#include <string>
+#include "sync/internal_api/public/base/model_type.h"
+
namespace syncer {
namespace syncable {
class BaseTransaction;
+class Directory;
class Id;
+class WriteTransaction;
// Count the number of entries with a given name inside of a parent.
// Useful to check folder structure and for porting older tests that
@@ -34,6 +38,10 @@ Id GetOnlyEntryWithName(BaseTransaction* rtrans,
const syncable::Id& parent_id,
const std::string& name);
+void CreateTypeRoot(WriteTransaction* trans,
+ syncable::Directory *dir,
+ ModelType type);
+
} // namespace syncable
} // namespace syncer
diff --git a/sync/test/test_directory_backing_store.h b/sync/test/test_directory_backing_store.h
index d58e7b2..033e99e 100644
--- a/sync/test/test_directory_backing_store.h
+++ b/sync/test/test_directory_backing_store.h
@@ -46,6 +46,7 @@ class TestDirectoryBackingStore : public DirectoryBackingStore {
FRIEND_TEST_ALL_PREFIXES(DirectoryBackingStoreTest, MigrateVersion81To82);
FRIEND_TEST_ALL_PREFIXES(DirectoryBackingStoreTest, MigrateVersion82To83);
FRIEND_TEST_ALL_PREFIXES(DirectoryBackingStoreTest, MigrateVersion83To84);
+ FRIEND_TEST_ALL_PREFIXES(DirectoryBackingStoreTest, MigrateVersion84To85);
FRIEND_TEST_ALL_PREFIXES(DirectoryBackingStoreTest, DetectInvalidOrdinal);
FRIEND_TEST_ALL_PREFIXES(DirectoryBackingStoreTest, ModelTypeIds);
FRIEND_TEST_ALL_PREFIXES(DirectoryBackingStoreTest, Corruption);