Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1563833002:
[Part-2]Add Statistics for SupervisedUserSettingService during merging and syncing data. (Closed)
|
Created:
4 years, 11 months ago by Deepak Modified:
4 years, 9 months ago Reviewers:
Bernhard Bauer CC:
chromium-reviews, pam+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdding Statistics like number of items added, modified for
SupervisedUserSettingService for SUPERVISED_USER_SETTINGS
while merging and syncing data starts in supervised_user_settings_service.cc.
BUG=572586
Committed: https://crrev.com/d33a1e9898ed8ae0623b5259c0dadd8bfc8381ae
Cr-Commit-Position: refs/heads/master@{#378198}
Patch Set 1 #Patch Set 2 : #Patch Set 3 : #
Total comments: 16
Patch Set 4 : Changes as per review comments. #Patch Set 5 : Changes as per review comments. #
Total comments: 6
Patch Set 6 : #Patch Set 7 : #
Total comments: 6
Patch Set 8 : #
Total comments: 2
Patch Set 9 : #
Total comments: 4
Patch Set 10 : #
Total comments: 4
Patch Set 11 : Changes as per review comments. #
Messages
Total messages: 27 (7 generated)
Description was changed from ========== Adding Statistics like number of items added, modified for SupervisedUserSettingService for SUPERVISED_USER_SETTINGS while merging and syncing data starts. BUG= ========== to ========== Adding Statistics like number of items added, modified for SupervisedUserSettingService for SUPERVISED_USER_SETTINGS while merging and syncing data starts. BUG= ==========
Description was changed from ========== Adding Statistics like number of items added, modified for SupervisedUserSettingService for SUPERVISED_USER_SETTINGS while merging and syncing data starts. BUG= ========== to ========== Adding Statistics like number of items added, modified for SupervisedUserSettingService for SUPERVISED_USER_SETTINGS while merging and syncing data starts. BUG= ==========
Description was changed from ========== Adding Statistics like number of items added, modified for SupervisedUserSettingService for SUPERVISED_USER_SETTINGS while merging and syncing data starts. BUG= ========== to ========== Adding Statistics like number of items added, modified for SupervisedUserSettingService for SUPERVISED_USER_SETTINGS while merging and syncing data starts in supervised_user_settings_service.cc. BUG=572586 ==========
deepak.m1@samsung.com changed reviewers: + bauerb@chromium.org
*Here all the atomic and split settings have been cleared and then getting created based on Sync data. So I am counting entries before association and then calculating the deleted entries by subtracting modified entries from it. * Newly added and modified entries are getting calculated as normal without considering the Queued entries. * Number of entries after association depends upon the number of entries in Sync_data with which MergeDataAndStartSyncing() is getting called. PTAL
https://codereview.chromium.org/1563833002/diff/40001/chrome/browser/supervis... File chrome/browser/supervised_user/supervised_user_settings_service.cc (right): https://codereview.chromium.org/1563833002/diff/40001/chrome/browser/supervis... chrome/browser/supervised_user/supervised_user_settings_service.cc:212: base::JSONWriter::Write(jt.value(), &json_value); You serialize the values and then compare them below. What you could do instead is to store the unserialized base::Value's and compare them to the deserialized ones from Sync (which you already have), so you don't have to do any additional serialization. https://codereview.chromium.org/1563833002/diff/40001/chrome/browser/supervis... chrome/browser/supervised_user/supervised_user_settings_service.cc:236: std::string json_value; This is identical to supervised_user_setting.value() anyway. (But see my comment above for comparing the base::Value's.) https://codereview.chromium.org/1563833002/diff/40001/chrome/browser/supervis... chrome/browser/supervised_user/supervised_user_settings_service.cc:251: // depending on whether they already exist) and move them to split settings. As you mention in the CL message, you ignore queued items... but why? 😃 https://codereview.chromium.org/1563833002/diff/40001/chrome/browser/supervis... chrome/browser/supervised_user/supervised_user_settings_service.cc:280: result.set_num_items_after_association(GetAllSyncData(type).size()); This is clever, but I'm a bit wary that this does more work than necessary, as it creates all theses Sync items that we don't really use (we just count them). It might be easier to just count them directly. https://codereview.chromium.org/1563833002/diff/40001/chrome/browser/supervis... File chrome/browser/supervised_user/supervised_user_settings_service_unittest.cc (right): https://codereview.chromium.org/1563833002/diff/40001/chrome/browser/supervis... chrome/browser/supervised_user/supervised_user_settings_service_unittest.cc:216: const base::Value* value = NULL; nullptr please. https://codereview.chromium.org/1563833002/diff/40001/chrome/browser/supervis... chrome/browser/supervised_user/supervised_user_settings_service_unittest.cc:236: EXPECT_EQ(4, result.num_items_added()); This is somewhat strange. Why do we add four items? I only see three entries in the dictionary. https://codereview.chromium.org/1563833002/diff/40001/chrome/browser/supervis... chrome/browser/supervised_user/supervised_user_settings_service_unittest.cc:248: SupervisedUserSettingsService::MakeSplitSettingKey(kSettingsName, This only uses split settings. It would be good to also exercise the code for some atomic settings and queued items.
PTAL https://codereview.chromium.org/1563833002/diff/40001/chrome/browser/supervis... File chrome/browser/supervised_user/supervised_user_settings_service.cc (right): https://codereview.chromium.org/1563833002/diff/40001/chrome/browser/supervis... chrome/browser/supervised_user/supervised_user_settings_service.cc:212: base::JSONWriter::Write(jt.value(), &json_value); On 2016/01/08 16:04:46, Bernhard Bauer wrote: > You serialize the values and then compare them below. What you could do instead > is to store the unserialized base::Value's and compare them to the deserialized > ones from Sync (which you already have), so you don't have to do any additional > serialization. Done. https://codereview.chromium.org/1563833002/diff/40001/chrome/browser/supervis... chrome/browser/supervised_user/supervised_user_settings_service.cc:236: std::string json_value; On 2016/01/08 16:04:46, Bernhard Bauer wrote: > This is identical to supervised_user_setting.value() anyway. (But see my comment > above for comparing the base::Value's.) Done. https://codereview.chromium.org/1563833002/diff/40001/chrome/browser/supervis... chrome/browser/supervised_user/supervised_user_settings_service.cc:251: // depending on whether they already exist) and move them to split settings. On 2016/01/08 16:04:46, Bernhard Bauer wrote: > As you mention in the CL message, you ignore queued items... but why? 😃 I thought that we should consider that things that are processed via |SyncDataList| passed in the function. I have done changes now to accommodate the queued items also. https://codereview.chromium.org/1563833002/diff/40001/chrome/browser/supervis... chrome/browser/supervised_user/supervised_user_settings_service.cc:280: result.set_num_items_after_association(GetAllSyncData(type).size()); On 2016/01/08 16:04:46, Bernhard Bauer wrote: > This is clever, but I'm a bit wary that this does more work than necessary, as > it creates all theses Sync items that we don't really use (we just count them). > It might be easier to just count them directly. Done. https://codereview.chromium.org/1563833002/diff/40001/chrome/browser/supervis... File chrome/browser/supervised_user/supervised_user_settings_service_unittest.cc (right): https://codereview.chromium.org/1563833002/diff/40001/chrome/browser/supervis... chrome/browser/supervised_user/supervised_user_settings_service_unittest.cc:216: const base::Value* value = NULL; On 2016/01/08 16:04:46, Bernhard Bauer wrote: > nullptr please. Done. https://codereview.chromium.org/1563833002/diff/40001/chrome/browser/supervis... chrome/browser/supervised_user/supervised_user_settings_service_unittest.cc:236: EXPECT_EQ(4, result.num_items_added()); On 2016/01/08 16:04:46, Bernhard Bauer wrote: > This is somewhat strange. Why do we add four items? I only see three entries in > the dictionary. I am adding a entry in sync_data at line 226. and 3 entry in the sync_data via dictionary. So sync_data have total 4 entries. https://codereview.chromium.org/1563833002/diff/40001/chrome/browser/supervis... chrome/browser/supervised_user/supervised_user_settings_service_unittest.cc:248: SupervisedUserSettingsService::MakeSplitSettingKey(kSettingsName, On 2016/01/08 16:04:46, Bernhard Bauer wrote: > This only uses split settings. It would be good to also exercise the code for > some atomic settings and queued items. Done.
ping :)
Sorry for the delay! https://codereview.chromium.org/1563833002/diff/40001/chrome/browser/supervis... File chrome/browser/supervised_user/supervised_user_settings_service.cc (right): https://codereview.chromium.org/1563833002/diff/40001/chrome/browser/supervis... chrome/browser/supervised_user/supervised_user_settings_service.cc:251: // depending on whether they already exist) and move them to split settings. On 2016/01/11 10:38:36, Deepak wrote: > On 2016/01/08 16:04:46, Bernhard Bauer wrote: > > As you mention in the CL message, you ignore queued items... but why? 😃 > > I thought that we should consider that things that are processed via > |SyncDataList| passed in the function. > I have done changes now to accommodate the queued items also. That is the data from Sync. The number of items before and after association is coming from our local data though, and that includes queued items. (Which you still don't include in the number before.) https://codereview.chromium.org/1563833002/diff/40001/chrome/browser/supervis... File chrome/browser/supervised_user/supervised_user_settings_service_unittest.cc (right): https://codereview.chromium.org/1563833002/diff/40001/chrome/browser/supervis... chrome/browser/supervised_user/supervised_user_settings_service_unittest.cc:236: EXPECT_EQ(4, result.num_items_added()); On 2016/01/11 10:38:36, Deepak wrote: > On 2016/01/08 16:04:46, Bernhard Bauer wrote: > > This is somewhat strange. Why do we add four items? I only see three entries > in > > the dictionary. > > I am adding a entry in sync_data at line 226. > and 3 entry in the sync_data via dictionary. > So sync_data have total 4 entries. Oh, right. Could you add a comment that explains that we add the three dictionary entries and this atomic entry? https://codereview.chromium.org/1563833002/diff/80001/chrome/browser/supervis... File chrome/browser/supervised_user/supervised_user_settings_service.cc (right): https://codereview.chromium.org/1563833002/diff/80001/chrome/browser/supervis... chrome/browser/supervised_user/supervised_user_settings_service.cc:227: std::map<std::string, const base::Value*>::iterator it = You can use auto for this. https://codereview.chromium.org/1563833002/diff/80001/chrome/browser/supervis... chrome/browser/supervised_user/supervised_user_settings_service.cc:231: else if (it->second != value.get()) This will compare the values by their address. You want to check them for equality, with the .Equals() method. Please also update the unit test so that it would have caught this. https://codereview.chromium.org/1563833002/diff/80001/chrome/browser/supervis... chrome/browser/supervised_user/supervised_user_settings_service.cc:256: if (seen_keys.find(name_key) == seen_keys.end()) { So, queued items are simply items added by the user that we couldn't send to Sync (because Sync was not ready yet), so we put them into a queue to ensure they won't be removed when we do the association. This means that association usually shouldn't add or modify queued items -- they simply get moved from the queue to atomic or split settings. (Keep in mind that the statistics are what happens to the _local_ data, not what happens to the Sync data -- we have SyncChange for that). So, we only have to handle edge cases here like when an item is queued and also in Sync data – it will initially be counted as an addition, but in the second pass it then turns out that it was already present before.
Thanks for the review. PTAL https://codereview.chromium.org/1563833002/diff/80001/chrome/browser/supervis... File chrome/browser/supervised_user/supervised_user_settings_service.cc (right): https://codereview.chromium.org/1563833002/diff/80001/chrome/browser/supervis... chrome/browser/supervised_user/supervised_user_settings_service.cc:227: std::map<std::string, const base::Value*>::iterator it = On 2016/01/18 17:56:56, Bernhard Bauer wrote: > You can use auto for this. Done. https://codereview.chromium.org/1563833002/diff/80001/chrome/browser/supervis... chrome/browser/supervised_user/supervised_user_settings_service.cc:231: else if (it->second != value.get()) On 2016/01/18 17:56:56, Bernhard Bauer wrote: > This will compare the values by their address. You want to check them for > equality, with the .Equals() method. Please also update the unit test so that it > would have caught this. Done. https://codereview.chromium.org/1563833002/diff/80001/chrome/browser/supervis... chrome/browser/supervised_user/supervised_user_settings_service.cc:256: if (seen_keys.find(name_key) == seen_keys.end()) { On 2016/01/18 17:56:56, Bernhard Bauer wrote: > So, queued items are simply items added by the user that we couldn't send to > Sync (because Sync was not ready yet), so we put them into a queue to ensure > they won't be removed when we do the association. This means that association > usually shouldn't add or modify queued items -- they simply get moved from the > queue to atomic or split settings. (Keep in mind that the statistics are what > happens to the _local_ data, not what happens to the Sync data -- we have > SyncChange for that). > > So, we only have to handle edge cases here like when an item is queued and also > in Sync data – it will initially be counted as an addition, but in the second > pass it then turns out that it was already present before. I agree with you, It means: ------------------------------------------- Part-1 Assume we have earlier 3 items: *1 Atomic Item: "TestSetting:SettingValue" *2 Split Items: X-SuperMoosePowers:foo bar X-SuperMoosePowers:eaudecologne 4711 For these sync is already done. --------------------------------------------- Part-2 Now I am adding 3 items in the Queue as: *1 Atomic Item x-Wombat hurdle *2 Split Item X-SuperMoosePowers:burp baz X-SuperMoosePowers:item second and 2 split items in the SyncDataList: X-SuperMoosePowers:foo burp X-SuperMoosePowers:Item first so when I call SupervisedUserSettingsService::MergeDataAndStartSyncing() then num_before_association will be 3. Then I am adding a new item (TestSetiings:Item first) and modifying 1 item as(TestSetting:foo burp) so num_added = 1 and num_modified = 1 Then when I upload the items from the queue, then I am adding below new items. x-Wombat hurdle X-SuperMoosePowers:burp baz And modifying the previously added item from (TestSettings:item first) to (TestSettings:item second) that comes in adding due to initial_sync_data. so technically I am modifying a item that was in queue by my initial_sync_data.right!! so my num_added will reduce by 1 and num_modified will increase by 1. So we need to consider only the cases that are added as part of initial_sync_data. --------------------------------------------------- so my syncMergeResult will be something like: num_before_association 3 from Part-1 num_added 0 num_modified 2 num_deleted 2 i.e (after_Association - (num_modified+num_added) num_after_association 4 ( counting of all atomic and split items) Please correct me if I misunderstood something. Thanks
https://codereview.chromium.org/1563833002/diff/120001/chrome/browser/supervi... File chrome/browser/supervised_user/supervised_user_settings_service.cc (right): https://codereview.chromium.org/1563833002/diff/120001/chrome/browser/supervi... chrome/browser/supervised_user/supervised_user_settings_service.cc:228: const auto& it = seen_keys.find(name_key); Just auto (the return value of find is an iterator, not a reference to it or to an element). https://codereview.chromium.org/1563833002/diff/120001/chrome/browser/supervi... chrome/browser/supervised_user/supervised_user_settings_service.cc:232: } else if (!Value::Equals(it->second, value.get())) { You can call the .Equals() method on either of these; they shouldn't be null. https://codereview.chromium.org/1563833002/diff/120001/chrome/browser/supervi... chrome/browser/supervised_user/supervised_user_settings_service.cc:260: num_modified++; I don't even think this one is modified locally, because for uploaded items the local version wins.
PTAL https://codereview.chromium.org/1563833002/diff/120001/chrome/browser/supervi... File chrome/browser/supervised_user/supervised_user_settings_service.cc (right): https://codereview.chromium.org/1563833002/diff/120001/chrome/browser/supervi... chrome/browser/supervised_user/supervised_user_settings_service.cc:228: const auto& it = seen_keys.find(name_key); On 2016/01/20 17:03:04, Bernhard Bauer wrote: > Just auto (the return value of find is an iterator, not a reference to it or to > an element). Done. https://codereview.chromium.org/1563833002/diff/120001/chrome/browser/supervi... chrome/browser/supervised_user/supervised_user_settings_service.cc:232: } else if (!Value::Equals(it->second, value.get())) { On 2016/01/20 17:03:04, Bernhard Bauer wrote: > You can call the .Equals() method on either of these; they shouldn't be null. After thinking a little more, I feel no need to check the value info, as when key is present in the map that means it is for the modification only, so removing this unnecessary check. https://codereview.chromium.org/1563833002/diff/120001/chrome/browser/supervi... chrome/browser/supervised_user/supervised_user_settings_service.cc:260: num_modified++; On 2016/01/20 17:03:04, Bernhard Bauer wrote: > I don't even think this one is modified locally, because for uploaded items the > local version wins. Is this means we should consider only addition and modification of the items by the local version/initial_sync_data only and should not consider the items added or modified by the queued items(That are added by the local/initial_sync_data), and queues items will be just uploaded and will not be considered in the statistic calculations. So the values in our test case should be: EXPECT_EQ(1, result.num_items_added()); EXPECT_EQ(2, result.num_items_deleted()); EXPECT_EQ(1, result.num_items_modified()); EXPECT_EQ(3, result.num_items_before_association()); EXPECT_EQ(4, result.num_items_after_association());
https://codereview.chromium.org/1563833002/diff/140001/chrome/browser/supervi... File chrome/browser/supervised_user/supervised_user_settings_service.cc (right): https://codereview.chromium.org/1563833002/diff/140001/chrome/browser/supervi... chrome/browser/supervised_user/supervised_user_settings_service.cc:196: std::map<std::string, const base::Value*> seen_keys; This could now be a set, right? We don't seem to use the value anymore. (And now that I look at it, storing the _address_ of the value is probably wrong if we go and modify preferences below, as that might delete the value, and then we'd have a user-after-free.)
A small doubt: When we are adding 2 items via initial_sync_data, so I have num_added as 2. But one of data item of initaial_sync_item is also getting added by the queued items with different value. Then we should decrements the count of num_added and increment num_modified as now effectively I am doing modification by my local initial_sync_data. And any other addition/modification by the queued items will not be considered for statistics. Please correct me if I misunderstood something.. Rest changes done. PTAL. https://codereview.chromium.org/1563833002/diff/140001/chrome/browser/supervi... File chrome/browser/supervised_user/supervised_user_settings_service.cc (right): https://codereview.chromium.org/1563833002/diff/140001/chrome/browser/supervi... chrome/browser/supervised_user/supervised_user_settings_service.cc:196: std::map<std::string, const base::Value*> seen_keys; On 2016/01/22 18:54:02, Bernhard Bauer wrote: > This could now be a set, right? We don't seem to use the value anymore. > > (And now that I look at it, storing the _address_ of the value is probably wrong > if we go and modify preferences below, as that might delete the value, and then > we'd have a user-after-free.) I agree with you , Now set is enough. Changes done.
Sorry again for the delay! I've been kinda busy, and this turned out to be somewhat tricky to reason about. On 2016/01/24 07:45:12, Deepak wrote: > A small doubt: > When we are adding 2 items via initial_sync_data, > so I have num_added as 2. > But one of data item of initaial_sync_item is also getting added by the queued > items with different value. > > Then we should decrements the count of num_added and increment num_modified as > now effectively I am doing modification by my local initial_sync_data. > And any other addition/modification by the queued items will not be considered > for statistics. > > Please correct me if I misunderstood something.. This matches my understanding, however... The counts in the test fail the sanity check: With 3 items before association, 0 items added, and 2 items deleted, the number after association should work out to 1, not 4! I think there are a couple of issues here: You don't include the number of queued items in the number before association, but you actually carry the local state (stored in |settings_|) over from the first time you associated, and there happen to be 3 items in there, so the observed number before association matches what you expect it to be, even though it is wrong in actuality. I'm actually fine with carrying the state over, because it allows us to test exactly that code path, but we need to get our numbers straight 😃 So, this is the state before association: TestingSetting: "SettingsValue" X-SuperMoosePowers:foo: "bar" X-SuperMoosePowers:eaudecologne: 4711 X-Wombat: "hurdle" X-SuperMoosePowers:burp: "baz" X-SuperMoosePowers:item: "second" And this is the state after association: X-SuperMoosePowers:foo: "burp" X-Wombat: "hurdle" X-SuperMoosePowers:burp: "baz" X-SuperMoosePowers:item: "second" This means we should get: items before association: 6 items after association: 4 items added: 0 items modified: 1 items deleted: 2 Which is close to what we currently have, but the number before association is wrong (because of the bug mentioned above), and the number of modifications (I suspect we are counting a modification where we shouldn't, because the local value wins). https://codereview.chromium.org/1563833002/diff/160001/chrome/browser/supervi... File chrome/browser/supervised_user/supervised_user_settings_service.cc (right): https://codereview.chromium.org/1563833002/diff/160001/chrome/browser/supervi... chrome/browser/supervised_user/supervised_user_settings_service.cc:212: We are still missing queued items in |num_before_association| here, aren't we? https://codereview.chromium.org/1563833002/diff/160001/chrome/browser/supervi... File chrome/browser/supervised_user/supervised_user_settings_service_unittest.cc (right): https://codereview.chromium.org/1563833002/diff/160001/chrome/browser/supervi... chrome/browser/supervised_user/supervised_user_settings_service_unittest.cc:218: base::DictionaryValue dict; Could you move this to right after line 227, so it's clear that the comment refers to this dictionary? Also, if you put the code from here until line 240 into a scope, and then a second scope from line 246 on, you don't need to clear |sync_data| and |dict|, you just declare them again in the second scope. That way, you can be sure that the local variables don't accidentally propagate from the first scope to the second.
@bernhard Thanks for enlighten me, and giving guidance. Changes done as suggested. PTAL https://codereview.chromium.org/1563833002/diff/160001/chrome/browser/supervi... File chrome/browser/supervised_user/supervised_user_settings_service.cc (right): https://codereview.chromium.org/1563833002/diff/160001/chrome/browser/supervi... chrome/browser/supervised_user/supervised_user_settings_service.cc:212: On 2016/01/28 13:56:33, Bernhard Bauer wrote: > We are still missing queued items in |num_before_association| here, aren't we? Done. https://codereview.chromium.org/1563833002/diff/160001/chrome/browser/supervi... File chrome/browser/supervised_user/supervised_user_settings_service_unittest.cc (right): https://codereview.chromium.org/1563833002/diff/160001/chrome/browser/supervi... chrome/browser/supervised_user/supervised_user_settings_service_unittest.cc:218: base::DictionaryValue dict; On 2016/01/28 13:56:33, Bernhard Bauer wrote: > Could you move this to right after line 227, so it's clear that the comment > refers to this dictionary? > > Also, if you put the code from here until line 240 into a scope, and then a > second scope from line 246 on, you don't need to clear |sync_data| and |dict|, > you just declare them again in the second scope. That way, you can be sure that > the local variables don't accidentally propagate from the first scope to the > second. Done.
I think we're almost there :) https://codereview.chromium.org/1563833002/diff/180001/chrome/browser/supervi... File chrome/browser/supervised_user/supervised_user_settings_service.cc (right): https://codereview.chromium.org/1563833002/diff/180001/chrome/browser/supervi... chrome/browser/supervised_user/supervised_user_settings_service.cc:219: if (queued_items) I think this will never be null, right? https://codereview.chromium.org/1563833002/diff/180001/chrome/browser/supervi... File chrome/browser/supervised_user/supervised_user_settings_service_unittest.cc (right): https://codereview.chromium.org/1563833002/diff/180001/chrome/browser/supervi... chrome/browser/supervised_user/supervised_user_settings_service_unittest.cc:245: Can you add a comment that explicitly says we carry over the preference state?
Sorry for the late reply due to health issues. Final changes done as suggested in last review. PTAL Thanks https://codereview.chromium.org/1563833002/diff/180001/chrome/browser/supervi... File chrome/browser/supervised_user/supervised_user_settings_service.cc (right): https://codereview.chromium.org/1563833002/diff/180001/chrome/browser/supervi... chrome/browser/supervised_user/supervised_user_settings_service.cc:219: if (queued_items) On 2016/02/03 11:38:13, Bernhard Bauer wrote: > I think this will never be null, right? Done. https://codereview.chromium.org/1563833002/diff/180001/chrome/browser/supervi... File chrome/browser/supervised_user/supervised_user_settings_service_unittest.cc (right): https://codereview.chromium.org/1563833002/diff/180001/chrome/browser/supervi... chrome/browser/supervised_user/supervised_user_settings_service_unittest.cc:245: On 2016/02/03 11:38:13, Bernhard Bauer wrote: > Can you add a comment that explicitly says we carry over the preference state? Done.
No worries, hope you're feeling better! LGTM
The CQ bit was checked by deepak.m1@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1563833002/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1563833002/200001
On 2016/02/29 11:35:16, Bernhard Bauer wrote: > No worries, hope you're feeling better! > > LGTM Many Thanks Bernhard.
Message was sent while issue was closed.
Description was changed from ========== Adding Statistics like number of items added, modified for SupervisedUserSettingService for SUPERVISED_USER_SETTINGS while merging and syncing data starts in supervised_user_settings_service.cc. BUG=572586 ========== to ========== Adding Statistics like number of items added, modified for SupervisedUserSettingService for SUPERVISED_USER_SETTINGS while merging and syncing data starts in supervised_user_settings_service.cc. BUG=572586 ==========
Message was sent while issue was closed.
Committed patchset #11 (id:200001)
Message was sent while issue was closed.
Description was changed from ========== Adding Statistics like number of items added, modified for SupervisedUserSettingService for SUPERVISED_USER_SETTINGS while merging and syncing data starts in supervised_user_settings_service.cc. BUG=572586 ========== to ========== Adding Statistics like number of items added, modified for SupervisedUserSettingService for SUPERVISED_USER_SETTINGS while merging and syncing data starts in supervised_user_settings_service.cc. BUG=572586 Committed: https://crrev.com/d33a1e9898ed8ae0623b5259c0dadd8bfc8381ae Cr-Commit-Position: refs/heads/master@{#378198} ==========
Message was sent while issue was closed.
Patchset 11 (id:??) landed as https://crrev.com/d33a1e9898ed8ae0623b5259c0dadd8bfc8381ae Cr-Commit-Position: refs/heads/master@{#378198} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
