Reset counters for all language when reset translate settings

components/translate/core/browser/translate_prefs.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/translate/core/browser/translate_prefs.h"
 
 #include <set>
 #include <utility>
 
 #include "base/memory/ptr_util.h"
@@ skipping 162 matching lines @@
 
 std::string TranslatePrefs::GetCountry() const {
   return country_;
 }
 
 void TranslatePrefs::ResetToDefaults() {
   ClearBlockedLanguages();
   ClearBlacklistedSites();
   ClearWhitelistedLanguagePairs();
 
-  std::vector<std::string> languages;
-  GetLanguageList(&languages);
-  for (std::vector<std::string>::const_iterator it = languages.begin();
-       it != languages.end(); ++it) {
-    const std::string& language = *it;
-    ResetTranslationAcceptedCount(language);
-    ResetTranslationDeniedCount(language);
-    ResetTranslationIgnoredCount(language);
-  }
-
+  prefs_->ClearPref(kPrefTranslateDeniedCount);

[Leo, 2017/06/07 00:21:23]

I think it's better to figure out the root cause why "Japanese" is missed in
GetLanguageList(&languages). which mentioned in the bug.

Seems removing the loop condition is a little aggressive. 

[Marti Wong, 2017/06/07 01:06:42]

In the repro case of crbug.com/728930, Japanese is missing because:
GetLanguageList here will get the languages from accept_languages_pref_
And accepted languages means "languages the user understands or does not want
translated".
(These are English, Chinese in the repro case.)

But Denied, Ignored, Accepted counters are for those languages user might not
understand.
(So Japanese is not in included in that For loop.)

If we don't do ClearPref here, we will need 3 For loops, one For loop for each
counter.

Pseudocode:

DictionaryValue acceptedCountDict = GetDictionary of AcceptedCount;
foreach ( language in acceptedCountDict )
   prefs->ResetTranslationAcceptedCount(language);

DictionaryValue deniedCountDict = GetDictionary of DeniedCount;
foreach ( language in deniedCountDict )
   prefs->ResetTranslationDeniedCount(language);

DictionaryValue ignoredCountDict = GetDictionary of IgnoredCount;
foreach ( language in ignoredCountDict )
   prefs->ResetTranslationIgnoredCount(language);

The outcome will be the same.
But the code will be more complicated and less efficient.
Any thoughts?

[groby-ooo-7-16, 2017/06/07 16:43:49]

Hm. It might be worth digging through history for the original author and asking
them why they choose the loop, instead of clearing the entire pref. (At first
glance, ClearPref seems the right choice - but I'm not sure if that's missing
some subtlety.)

[Marti Wong, 2017/06/08 03:30:50]

In case there is any reason we need to use For-loop instead of ClearPref, we
could do that by the following code.  I will ask
Miguel (who is the reviewer of the CL creating ResetToDefaults()) to see if
there is any subtle reason for using For-loop.

  const base::DictionaryValue* deniedCountDictionary =
      prefs_->GetDictionary(kPrefTranslateDeniedCount);
  for (base::DictionaryValue::Iterator iter(*deniedCountDictionary);
       !iter.IsAtEnd(); iter.Advance()) {
    ResetTranslationDeniedCount(iter.key());
  }

  const base::DictionaryValue* ignoredCountDictionary =
      prefs_->GetDictionary(kPrefTranslateIgnoredCount);
  for (base::DictionaryValue::Iterator iter(*ignoredCountDictionary);
       !iter.IsAtEnd(); iter.Advance()) {
    ResetTranslationIgnoredCount(iter.key());
  }

  const base::DictionaryValue* acceptedCountDictionary =
      prefs_->GetDictionary(kPrefTranslateAcceptedCount);
  for (base::DictionaryValue::Iterator iter(*acceptedCountDictionary);
       !iter.IsAtEnd(); iter.Advance()) {
    ResetTranslationAcceptedCount(iter.key());
  }

+  prefs_->ClearPref(kPrefTranslateIgnoredCount);
+  prefs_->ClearPref(kPrefTranslateAcceptedCount);
   prefs_->ClearPref(kPrefTranslateLastDeniedTimeForLanguage);
   prefs_->ClearPref(kPrefTranslateTooOftenDeniedForLanguage);
 }
 
 bool TranslatePrefs::IsBlockedLanguage(
     const std::string& original_language) const {
   return IsValueBlacklisted(kPrefTranslateBlockedLanguages, original_language);
 }
 
 void TranslatePrefs::BlockLanguage(const std::string& original_language) {
@@ skipping 435 matching lines @@
     out_value->push_back(it);
   std::sort(out_value->begin(), out_value->end(),
             [](const LanguageAndProbability& left,
                const LanguageAndProbability& right) {
               return left.second > right.second;
             });
   return confidence;
 }
 
 }  // namespace translate