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

Unified Diff: chrome/browser/supervised_user/supervised_user_settings_service.cc

Issue 1563833002: [Part-2]Add Statistics for SupervisedUserSettingService during merging and syncing data. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Changes as per review comments. Created 4 years, 11 months 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 side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | chrome/browser/supervised_user/supervised_user_settings_service_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/supervised_user/supervised_user_settings_service.cc
diff --git a/chrome/browser/supervised_user/supervised_user_settings_service.cc b/chrome/browser/supervised_user/supervised_user_settings_service.cc
index db09106b4e5df4dd6792f75746f69d280395fa07..bd8aa424c3765185664aedab4f0f1dfb7b26a856 100644
--- a/chrome/browser/supervised_user/supervised_user_settings_service.cc
+++ b/chrome/browser/supervised_user/supervised_user_settings_service.cc
@@ -78,7 +78,7 @@ void SupervisedUserSettingsService::Init(
// is fixed.
CHECK(store_->IsInitializationComplete());
} else {
- store_->ReadPrefsAsync(NULL);
+ store_->ReadPrefsAsync(nullptr);
}
}
@@ -134,7 +134,7 @@ void SupervisedUserSettingsService::UploadItem(const std::string& key,
DCHECK(!SettingShouldApplyToPrefs(key));
std::string key_suffix = key;
- base::DictionaryValue* dict = NULL;
+ base::DictionaryValue* dict = nullptr;
if (sync_processor_) {
content::RecordAction(UserMetricsAction("ManagedUsers_UploadItem_Syncing"));
dict = GetDictionaryAndSplitKey(&key_suffix);
@@ -163,7 +163,7 @@ void SupervisedUserSettingsService::SetLocalSetting(
if (value)
local_settings_->SetWithoutPathExpansion(key, value.release());
else
- local_settings_->RemoveWithoutPathExpansion(key, NULL);
+ local_settings_->RemoveWithoutPathExpansion(key, nullptr);
InformSubscribers();
}
@@ -193,8 +193,27 @@ SyncMergeResult SupervisedUserSettingsService::MergeDataAndStartSyncing(
sync_processor_ = std::move(sync_processor);
error_handler_ = std::move(error_handler);
+ std::map<std::string, const base::Value*> seen_keys;
+ int num_before_association = GetAtomicSettings()->size();
+ for (base::DictionaryValue::Iterator it(*GetAtomicSettings()); !it.IsAtEnd();
+ it.Advance()) {
+ seen_keys[it.key()] = &it.value();
+ }
+ for (base::DictionaryValue::Iterator it(*GetSplitSettings()); !it.IsAtEnd();
+ it.Advance()) {
+ const base::DictionaryValue* dict = nullptr;
+ it.value().GetAsDictionary(&dict);
+ num_before_association += dict->size();
+ for (base::DictionaryValue::Iterator jt(*dict); !jt.IsAtEnd();
+ jt.Advance()) {
+ seen_keys[MakeSplitSettingKey(it.key(), jt.key())] = &jt.value();
+ }
+ }
+
// Clear all atomic and split settings, then recreate them from Sync data.
Clear();
+ int num_added = 0;
+ int num_modified = 0;
for (const SyncData& sync_data : initial_sync_data) {
DCHECK_EQ(SUPERVISED_USER_SETTINGS, sync_data.GetDataType());
const ::sync_pb::ManagedUserSettingSpecifics& supervised_user_setting =
@@ -202,9 +221,17 @@ SyncMergeResult SupervisedUserSettingsService::MergeDataAndStartSyncing(
scoped_ptr<base::Value> value =
JSONReader::Read(supervised_user_setting.value());
std::string name_suffix = supervised_user_setting.name();
+ std::string name_key = name_suffix;
base::DictionaryValue* dict = GetDictionaryAndSplitKey(&name_suffix);
dict->SetWithoutPathExpansion(name_suffix, value.release());
+ std::map<std::string, const base::Value*>::iterator it =
Bernhard Bauer 2016/01/18 17:56:56 You can use auto for this.
Deepak 2016/01/19 09:54:02 Done.
+ seen_keys.find(name_key);
+ if (it == seen_keys.end())
+ num_added++;
+ else if (it->second != value.get())
Bernhard Bauer 2016/01/18 17:56:56 This will compare the values by their address. You
Deepak 2016/01/19 09:54:02 Done.
+ num_modified++;
}
+
store_->ReportValueChanged(kAtomicSettings,
WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS);
store_->ReportValueChanged(kSplitSettings,
@@ -218,6 +245,7 @@ SyncMergeResult SupervisedUserSettingsService::MergeDataAndStartSyncing(
for (base::DictionaryValue::Iterator it(*queued_items); !it.IsAtEnd();
it.Advance()) {
std::string key_suffix = it.key();
+ std::string name_key = key_suffix;
base::DictionaryValue* dict = GetDictionaryAndSplitKey(&key_suffix);
SyncData data = CreateSyncDataForSetting(it.key(), it.value());
SyncChange::SyncChangeType change_type =
@@ -225,6 +253,14 @@ SyncMergeResult SupervisedUserSettingsService::MergeDataAndStartSyncing(
: SyncChange::ACTION_ADD;
change_list.push_back(SyncChange(FROM_HERE, change_type, data));
dict->SetWithoutPathExpansion(key_suffix, it.value().DeepCopy());
+ if (seen_keys.find(name_key) == seen_keys.end()) {
Bernhard Bauer 2016/01/18 17:56:56 So, queued items are simply items added by the use
Deepak 2016/01/19 09:54:02 I agree with you, It means: ----------------------
+ num_added++;
+ } else {
+ for (DictionaryValue::Iterator jt(*dict); !jt.IsAtEnd(); jt.Advance()) {
+ if (seen_keys[name_key] != &jt.value())
+ num_modified++;
+ }
+ }
}
queued_items->Clear();
@@ -237,7 +273,20 @@ SyncMergeResult SupervisedUserSettingsService::MergeDataAndStartSyncing(
sync_processor_->ProcessSyncChanges(FROM_HERE, change_list));
}
- // TODO(bauerb): Statistics?
+ // Calculating number of items after association.
+ int num_after_association = GetAtomicSettings()->size();
+ for (base::DictionaryValue::Iterator it(*GetSplitSettings()); !it.IsAtEnd();
+ it.Advance()) {
+ const base::DictionaryValue* dict = nullptr;
+ it.value().GetAsDictionary(&dict);
+ num_after_association += dict->size();
+ }
+
+ result.set_num_items_added(num_added);
+ result.set_num_items_modified(num_modified);
+ result.set_num_items_deleted(num_before_association - num_modified);
+ result.set_num_items_before_association(num_before_association);
+ result.set_num_items_after_association(num_after_association);
return result;
}
@@ -257,7 +306,7 @@ SyncDataList SupervisedUserSettingsService::GetAllSyncData(
}
for (base::DictionaryValue::Iterator it(*GetSplitSettings()); !it.IsAtEnd();
it.Advance()) {
- const base::DictionaryValue* dict = NULL;
+ const base::DictionaryValue* dict = nullptr;
it.value().GetAsDictionary(&dict);
for (base::DictionaryValue::Iterator jt(*dict);
!jt.IsAtEnd(); jt.Advance()) {
@@ -298,7 +347,7 @@ SyncError SupervisedUserSettingsService::ProcessSyncChanges(
case SyncChange::ACTION_DELETE: {
DLOG_IF(WARNING, !dict->HasKey(key)) << "Trying to delete nonexistent "
<< "key " << key;
- dict->RemoveWithoutPathExpansion(key, NULL);
+ dict->RemoveWithoutPathExpansion(key, nullptr);
break;
}
case SyncChange::ACTION_INVALID: {
@@ -337,8 +386,8 @@ void SupervisedUserSettingsService::OnInitializationCompleted(bool success) {
base::DictionaryValue* SupervisedUserSettingsService::GetOrCreateDictionary(
const std::string& key) const {
- base::Value* value = NULL;
- base::DictionaryValue* dict = NULL;
+ base::Value* value = nullptr;
+ base::DictionaryValue* dict = nullptr;
if (store_->GetMutableValue(key, &value)) {
bool success = value->GetAsDictionary(&dict);
DCHECK(success);
@@ -372,7 +421,7 @@ base::DictionaryValue* SupervisedUserSettingsService::GetDictionaryAndSplitKey(
base::DictionaryValue* split_settings = GetSplitSettings();
std::string prefix = key->substr(0, pos);
- base::DictionaryValue* dict = NULL;
+ base::DictionaryValue* dict = nullptr;
if (!split_settings->GetDictionary(prefix, &dict)) {
dict = new base::DictionaryValue;
DCHECK(!split_settings->HasKey(prefix));
« no previous file with comments | « no previous file | chrome/browser/supervised_user/supervised_user_settings_service_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698