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

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

Issue 2568543003: [Sync] Actively guard against provider being cleared in DeviceInfoSyncBridge. (Closed)
Patch Set: Updated comments slightly. 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 }
121 128
122 // Local data should typically be near empty, with the only possible value 129 // Local data should typically be near empty, with the only possible value
123 // corresponding to this device. This is because on signout all device info 130 // corresponding to this device. This is because on signout all device info
124 // data is blown away. However, this simplification is being ignored here and 131 // data is blown away. However, this simplification is being ignored here and
125 // a full difference is going to be calculated to explore what other bridge 132 // a full difference is going to be calculated to explore what other bridge
126 // implementations may look like. 133 // implementations may look like.
127 std::set<std::string> local_guids_to_put; 134 std::set<std::string> local_guids_to_put;
128 for (const auto& kv : all_data_) { 135 for (const auto& kv : all_data_) {
129 local_guids_to_put.insert(kv.first); 136 local_guids_to_put.insert(kv.first);
130 } 137 }
131 138
132 bool has_changes = false; 139 bool has_changes = false;
133 const DeviceInfo* local_info =
134 local_device_info_provider_->GetLocalDeviceInfo();
135 std::string local_guid = local_info->guid(); 140 std::string local_guid = local_info->guid();
136 std::unique_ptr<WriteBatch> batch = store_->CreateWriteBatch(); 141 std::unique_ptr<WriteBatch> batch = store_->CreateWriteBatch();
137 for (const auto& kv : entity_data_map) { 142 for (const auto& kv : entity_data_map) {
138 const DeviceInfoSpecifics& specifics = 143 const DeviceInfoSpecifics& specifics =
139 kv.second.value().specifics.device_info(); 144 kv.second.value().specifics.device_info();
140 DCHECK_EQ(kv.first, specifics.cache_guid()); 145 DCHECK_EQ(kv.first, specifics.cache_guid());
141 if (specifics.cache_guid() == local_guid) { 146 if (specifics.cache_guid() == local_guid) {
142 // Don't Put local data if it's the same as the remote copy. 147 // Don't Put local data if it's the same as the remote copy.
143 if (local_info->Equals(*SpecificsToModel(specifics))) { 148 if (local_info->Equals(*SpecificsToModel(specifics))) {
144 local_guids_to_put.erase(local_guid); 149 local_guids_to_put.erase(local_guid);
(...skipping 14 matching lines...) Expand all
159 164
160 batch->TransferMetadataChanges(std::move(metadata_change_list)); 165 batch->TransferMetadataChanges(std::move(metadata_change_list));
161 CommitAndNotify(std::move(batch), has_changes); 166 CommitAndNotify(std::move(batch), has_changes);
162 return SyncError(); 167 return SyncError();
163 } 168 }
164 169
165 SyncError DeviceInfoSyncBridge::ApplySyncChanges( 170 SyncError DeviceInfoSyncBridge::ApplySyncChanges(
166 std::unique_ptr<MetadataChangeList> metadata_change_list, 171 std::unique_ptr<MetadataChangeList> metadata_change_list,
167 EntityChangeList entity_changes) { 172 EntityChangeList entity_changes) {
168 DCHECK(has_provider_initialized_); 173 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 }
169 181
170 std::unique_ptr<WriteBatch> batch = store_->CreateWriteBatch(); 182 std::unique_ptr<WriteBatch> batch = store_->CreateWriteBatch();
171 bool has_changes = false; 183 bool has_changes = false;
172 for (EntityChange& change : entity_changes) { 184 for (EntityChange& change : entity_changes) {
173 const std::string guid = change.storage_key(); 185 const std::string guid = change.storage_key();
174 // Each device is the authoritative source for itself, ignore any remote 186 // Each device is the authoritative source for itself, ignore any remote
175 // changes that have our local cache guid. 187 // changes that have our local cache guid.
176 if (guid == local_device_info_provider_->GetLocalDeviceInfo()->guid()) { 188 if (guid == local_info->guid()) {
177 continue; 189 continue;
178 } 190 }
179 191
180 if (change.type() == EntityChange::ACTION_DELETE) { 192 if (change.type() == EntityChange::ACTION_DELETE) {
181 has_changes |= DeleteSpecifics(guid, batch.get()); 193 has_changes |= DeleteSpecifics(guid, batch.get());
182 } else { 194 } else {
183 const DeviceInfoSpecifics& specifics = 195 const DeviceInfoSpecifics& specifics =
184 change.data().specifics.device_info(); 196 change.data().specifics.device_info();
185 DCHECK(guid == specifics.cache_guid()); 197 DCHECK(guid == specifics.cache_guid());
186 StoreSpecifics(base::MakeUnique<DeviceInfoSpecifics>(specifics), 198 StoreSpecifics(base::MakeUnique<DeviceInfoSpecifics>(specifics),
(...skipping 121 matching lines...) Expand 10 before | Expand all | Expand 10 after
308 } else { 320 } else {
309 return false; 321 return false;
310 } 322 }
311 } 323 }
312 324
313 void DeviceInfoSyncBridge::OnProviderInitialized() { 325 void DeviceInfoSyncBridge::OnProviderInitialized() {
314 // Now that the provider has initialized, remove the subscription. The bridge 326 // Now that the provider has initialized, remove the subscription. The bridge
315 // should only need to give the processor metadata upon initialization. If 327 // should only need to give the processor metadata upon initialization. If
316 // sync is disabled and enabled, our provider will try to retrigger this 328 // sync is disabled and enabled, our provider will try to retrigger this
317 // event, but we do not want to send any more metadata to the processor. 329 // 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.
318 subscription_.reset(); 332 subscription_.reset();
319 333
320 has_provider_initialized_ = true; 334 has_provider_initialized_ = true;
321 LoadMetadataIfReady(); 335 LoadMetadataIfReady();
322 } 336 }
323 337
324 void DeviceInfoSyncBridge::OnStoreCreated( 338 void DeviceInfoSyncBridge::OnStoreCreated(
325 Result result, 339 Result result,
326 std::unique_ptr<ModelTypeStore> store) { 340 std::unique_ptr<ModelTypeStore> store) {
327 if (result == Result::SUCCESS) { 341 if (result == Result::SUCCESS) {
(...skipping 52 matching lines...) Expand 10 before | Expand all | Expand 10 after
380 // On initial syncing we will have a change processor here, but it will not be 394 // On initial syncing we will have a change processor here, but it will not be
381 // tracking changes. We need to persist a copy of our local device info to 395 // tracking changes. We need to persist a copy of our local device info to
382 // disk, but the Put call to the processor will be ignored. That should be 396 // disk, but the Put call to the processor will be ignored. That should be
383 // fine however, as the discrepancy will be picked up later in merge. We don't 397 // fine however, as the discrepancy will be picked up later in merge. We don't
384 // bother trying to track this case and act intelligently because simply not 398 // bother trying to track this case and act intelligently because simply not
385 // much of a benefit in doing so. 399 // much of a benefit in doing so.
386 DCHECK(has_provider_initialized_); 400 DCHECK(has_provider_initialized_);
387 401
388 const DeviceInfo* current_info = 402 const DeviceInfo* current_info =
389 local_device_info_provider_->GetLocalDeviceInfo(); 403 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 }
390 auto iter = all_data_.find(current_info->guid()); 413 auto iter = all_data_.find(current_info->guid());
391 414
392 // Convert to DeviceInfo for Equals function. 415 // Convert to DeviceInfo for Equals function.
393 if (iter != all_data_.end() && 416 if (iter != all_data_.end() &&
394 current_info->Equals(*SpecificsToModel(*iter->second))) { 417 current_info->Equals(*SpecificsToModel(*iter->second))) {
395 const TimeDelta pulse_delay(DeviceInfoUtil::CalculatePulseDelay( 418 const TimeDelta pulse_delay(DeviceInfoUtil::CalculatePulseDelay(
396 GetLastUpdateTime(*iter->second), Time::Now())); 419 GetLastUpdateTime(*iter->second), Time::Now()));
397 if (!pulse_delay.is_zero()) { 420 if (!pulse_delay.is_zero()) {
398 pulse_timer_.Start(FROM_HERE, pulse_delay, 421 pulse_timer_.Start(FROM_HERE, pulse_delay,
399 base::Bind(&DeviceInfoSyncBridge::SendLocalData, 422 base::Bind(&DeviceInfoSyncBridge::SendLocalData,
(...skipping 50 matching lines...) Expand 10 before | Expand all | Expand 10 after
450 } 473 }
451 474
452 void DeviceInfoSyncBridge::ReportStartupErrorToSync(const std::string& msg) { 475 void DeviceInfoSyncBridge::ReportStartupErrorToSync(const std::string& msg) {
453 // TODO(skym): Shouldn't need to log this here, reporting should always log. 476 // TODO(skym): Shouldn't need to log this here, reporting should always log.
454 LOG(WARNING) << msg; 477 LOG(WARNING) << msg;
455 change_processor()->OnMetadataLoaded( 478 change_processor()->OnMetadataLoaded(
456 change_processor()->CreateAndUploadError(FROM_HERE, msg), nullptr); 479 change_processor()->CreateAndUploadError(FROM_HERE, msg), nullptr);
457 } 480 }
458 481
459 } // namespace syncer 482 } // 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