Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(128)

Side by Side Diff: components/sync/device_info/device_info_sync_bridge.cc

Issue 2587343005: Revert "[Sync] Actively guard against provider being cleared in DeviceInfoSyncBridge." (Closed)
Patch Set: Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « no previous file | components/sync/device_info/device_info_sync_bridge_unittest.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright 2015 The Chromium Authors. All rights reserved. 1 // Copyright 2015 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "components/sync/device_info/device_info_sync_bridge.h" 5 #include "components/sync/device_info/device_info_sync_bridge.h"
6 6
7 #include <stdint.h> 7 #include <stdint.h>
8 8
9 #include <algorithm> 9 #include <algorithm>
10 #include <set> 10 #include <set>
(...skipping 100 matching lines...) Expand 10 before | Expand all | Expand 10 after
111 std::unique_ptr<MetadataChangeList> 111 std::unique_ptr<MetadataChangeList>
112 DeviceInfoSyncBridge::CreateMetadataChangeList() { 112 DeviceInfoSyncBridge::CreateMetadataChangeList() {
113 return WriteBatch::CreateMetadataChangeList(); 113 return WriteBatch::CreateMetadataChangeList();
114 } 114 }
115 115
116 SyncError DeviceInfoSyncBridge::MergeSyncData( 116 SyncError DeviceInfoSyncBridge::MergeSyncData(
117 std::unique_ptr<MetadataChangeList> metadata_change_list, 117 std::unique_ptr<MetadataChangeList> metadata_change_list,
118 EntityDataMap entity_data_map) { 118 EntityDataMap entity_data_map) {
119 DCHECK(has_provider_initialized_); 119 DCHECK(has_provider_initialized_);
120 DCHECK(change_processor()->IsTrackingMetadata()); 120 DCHECK(change_processor()->IsTrackingMetadata());
121 const DeviceInfo* local_info =
122 local_device_info_provider_->GetLocalDeviceInfo();
123 // If our dependency was yanked out from beneath us, we cannot correctly
124 // handle this request, and all our data will be deleted soon.
125 if (local_info == nullptr) {
126 return SyncError();
127 }
128 121
129 // Local data should typically be near empty, with the only possible value 122 // Local data should typically be near empty, with the only possible value
130 // corresponding to this device. This is because on signout all device info 123 // corresponding to this device. This is because on signout all device info
131 // data is blown away. However, this simplification is being ignored here and 124 // data is blown away. However, this simplification is being ignored here and
132 // a full difference is going to be calculated to explore what other bridge 125 // a full difference is going to be calculated to explore what other bridge
133 // implementations may look like. 126 // implementations may look like.
134 std::set<std::string> local_guids_to_put; 127 std::set<std::string> local_guids_to_put;
135 for (const auto& kv : all_data_) { 128 for (const auto& kv : all_data_) {
136 local_guids_to_put.insert(kv.first); 129 local_guids_to_put.insert(kv.first);
137 } 130 }
138 131
139 bool has_changes = false; 132 bool has_changes = false;
133 const DeviceInfo* local_info =
134 local_device_info_provider_->GetLocalDeviceInfo();
140 std::string local_guid = local_info->guid(); 135 std::string local_guid = local_info->guid();
141 std::unique_ptr<WriteBatch> batch = store_->CreateWriteBatch(); 136 std::unique_ptr<WriteBatch> batch = store_->CreateWriteBatch();
142 for (const auto& kv : entity_data_map) { 137 for (const auto& kv : entity_data_map) {
143 const DeviceInfoSpecifics& specifics = 138 const DeviceInfoSpecifics& specifics =
144 kv.second.value().specifics.device_info(); 139 kv.second.value().specifics.device_info();
145 DCHECK_EQ(kv.first, specifics.cache_guid()); 140 DCHECK_EQ(kv.first, specifics.cache_guid());
146 if (specifics.cache_guid() == local_guid) { 141 if (specifics.cache_guid() == local_guid) {
147 // Don't Put local data if it's the same as the remote copy. 142 // Don't Put local data if it's the same as the remote copy.
148 if (local_info->Equals(*SpecificsToModel(specifics))) { 143 if (local_info->Equals(*SpecificsToModel(specifics))) {
149 local_guids_to_put.erase(local_guid); 144 local_guids_to_put.erase(local_guid);
(...skipping 14 matching lines...) Expand all
164 159
165 batch->TransferMetadataChanges(std::move(metadata_change_list)); 160 batch->TransferMetadataChanges(std::move(metadata_change_list));
166 CommitAndNotify(std::move(batch), has_changes); 161 CommitAndNotify(std::move(batch), has_changes);
167 return SyncError(); 162 return SyncError();
168 } 163 }
169 164
170 SyncError DeviceInfoSyncBridge::ApplySyncChanges( 165 SyncError DeviceInfoSyncBridge::ApplySyncChanges(
171 std::unique_ptr<MetadataChangeList> metadata_change_list, 166 std::unique_ptr<MetadataChangeList> metadata_change_list,
172 EntityChangeList entity_changes) { 167 EntityChangeList entity_changes) {
173 DCHECK(has_provider_initialized_); 168 DCHECK(has_provider_initialized_);
174 const DeviceInfo* local_info =
175 local_device_info_provider_->GetLocalDeviceInfo();
176 // If our dependency was yanked out from beneath us, we cannot correctly
177 // handle this request, and all our data will be deleted soon.
178 if (local_info == nullptr) {
179 return SyncError();
180 }
181 169
182 std::unique_ptr<WriteBatch> batch = store_->CreateWriteBatch(); 170 std::unique_ptr<WriteBatch> batch = store_->CreateWriteBatch();
183 bool has_changes = false; 171 bool has_changes = false;
184 for (EntityChange& change : entity_changes) { 172 for (EntityChange& change : entity_changes) {
185 const std::string guid = change.storage_key(); 173 const std::string guid = change.storage_key();
186 // Each device is the authoritative source for itself, ignore any remote 174 // Each device is the authoritative source for itself, ignore any remote
187 // changes that have our local cache guid. 175 // changes that have our local cache guid.
188 if (guid == local_info->guid()) { 176 if (guid == local_device_info_provider_->GetLocalDeviceInfo()->guid()) {
189 continue; 177 continue;
190 } 178 }
191 179
192 if (change.type() == EntityChange::ACTION_DELETE) { 180 if (change.type() == EntityChange::ACTION_DELETE) {
193 has_changes |= DeleteSpecifics(guid, batch.get()); 181 has_changes |= DeleteSpecifics(guid, batch.get());
194 } else { 182 } else {
195 const DeviceInfoSpecifics& specifics = 183 const DeviceInfoSpecifics& specifics =
196 change.data().specifics.device_info(); 184 change.data().specifics.device_info();
197 DCHECK(guid == specifics.cache_guid()); 185 DCHECK(guid == specifics.cache_guid());
198 StoreSpecifics(base::MakeUnique<DeviceInfoSpecifics>(specifics), 186 StoreSpecifics(base::MakeUnique<DeviceInfoSpecifics>(specifics),
(...skipping 121 matching lines...) Expand 10 before | Expand all | Expand 10 after
320 } else { 308 } else {
321 return false; 309 return false;
322 } 310 }
323 } 311 }
324 312
325 void DeviceInfoSyncBridge::OnProviderInitialized() { 313 void DeviceInfoSyncBridge::OnProviderInitialized() {
326 // Now that the provider has initialized, remove the subscription. The bridge 314 // Now that the provider has initialized, remove the subscription. The bridge
327 // should only need to give the processor metadata upon initialization. If 315 // should only need to give the processor metadata upon initialization. If
328 // sync is disabled and enabled, our provider will try to retrigger this 316 // sync is disabled and enabled, our provider will try to retrigger this
329 // event, but we do not want to send any more metadata to the processor. 317 // event, but we do not want to send any more metadata to the processor.
330 // TODO(skym, crbug.com/672600): Handle re-initialization and start the pulse
331 // timer.
332 subscription_.reset(); 318 subscription_.reset();
333 319
334 has_provider_initialized_ = true; 320 has_provider_initialized_ = true;
335 LoadMetadataIfReady(); 321 LoadMetadataIfReady();
336 } 322 }
337 323
338 void DeviceInfoSyncBridge::OnStoreCreated( 324 void DeviceInfoSyncBridge::OnStoreCreated(
339 Result result, 325 Result result,
340 std::unique_ptr<ModelTypeStore> store) { 326 std::unique_ptr<ModelTypeStore> store) {
341 if (result == Result::SUCCESS) { 327 if (result == Result::SUCCESS) {
(...skipping 52 matching lines...) Expand 10 before | Expand all | Expand 10 after
394 // On initial syncing we will have a change processor here, but it will not be 380 // On initial syncing we will have a change processor here, but it will not be
395 // tracking changes. We need to persist a copy of our local device info to 381 // tracking changes. We need to persist a copy of our local device info to
396 // disk, but the Put call to the processor will be ignored. That should be 382 // disk, but the Put call to the processor will be ignored. That should be
397 // fine however, as the discrepancy will be picked up later in merge. We don't 383 // fine however, as the discrepancy will be picked up later in merge. We don't
398 // bother trying to track this case and act intelligently because simply not 384 // bother trying to track this case and act intelligently because simply not
399 // much of a benefit in doing so. 385 // much of a benefit in doing so.
400 DCHECK(has_provider_initialized_); 386 DCHECK(has_provider_initialized_);
401 387
402 const DeviceInfo* current_info = 388 const DeviceInfo* current_info =
403 local_device_info_provider_->GetLocalDeviceInfo(); 389 local_device_info_provider_->GetLocalDeviceInfo();
404 // Must ensure |pulse_timer_| is started even if sync is in the process of
405 // being disabled. TODO(skym, crbug.com/672600): Remove this timer Start(), as
406 // it should be started when the provider re-initializes instead.
407 if (current_info == nullptr) {
408 pulse_timer_.Start(FROM_HERE, DeviceInfoUtil::kPulseInterval,
409 base::Bind(&DeviceInfoSyncBridge::SendLocalData,
410 base::Unretained(this)));
411 return;
412 }
413 auto iter = all_data_.find(current_info->guid()); 390 auto iter = all_data_.find(current_info->guid());
414 391
415 // Convert to DeviceInfo for Equals function. 392 // Convert to DeviceInfo for Equals function.
416 if (iter != all_data_.end() && 393 if (iter != all_data_.end() &&
417 current_info->Equals(*SpecificsToModel(*iter->second))) { 394 current_info->Equals(*SpecificsToModel(*iter->second))) {
418 const TimeDelta pulse_delay(DeviceInfoUtil::CalculatePulseDelay( 395 const TimeDelta pulse_delay(DeviceInfoUtil::CalculatePulseDelay(
419 GetLastUpdateTime(*iter->second), Time::Now())); 396 GetLastUpdateTime(*iter->second), Time::Now()));
420 if (!pulse_delay.is_zero()) { 397 if (!pulse_delay.is_zero()) {
421 pulse_timer_.Start(FROM_HERE, pulse_delay, 398 pulse_timer_.Start(FROM_HERE, pulse_delay,
422 base::Bind(&DeviceInfoSyncBridge::SendLocalData, 399 base::Bind(&DeviceInfoSyncBridge::SendLocalData,
(...skipping 50 matching lines...) Expand 10 before | Expand all | Expand 10 after
473 } 450 }
474 451
475 void DeviceInfoSyncBridge::ReportStartupErrorToSync(const std::string& msg) { 452 void DeviceInfoSyncBridge::ReportStartupErrorToSync(const std::string& msg) {
476 // TODO(skym): Shouldn't need to log this here, reporting should always log. 453 // TODO(skym): Shouldn't need to log this here, reporting should always log.
477 LOG(WARNING) << msg; 454 LOG(WARNING) << msg;
478 change_processor()->OnMetadataLoaded( 455 change_processor()->OnMetadataLoaded(
479 change_processor()->CreateAndUploadError(FROM_HERE, msg), nullptr); 456 change_processor()->CreateAndUploadError(FROM_HERE, msg), nullptr);
480 } 457 }
481 458
482 } // namespace syncer 459 } // namespace syncer
OLDNEW
« no previous file with comments | « no previous file | components/sync/device_info/device_info_sync_bridge_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698