Allows to manually change the stored permanent country in Variations from the translate-internals d…

components/variations/service/variations_service.cc

Index: components/variations/service/variations_service.cc
diff --git a/components/variations/service/variations_service.cc b/components/variations/service/variations_service.cc
index 9e0c4c75134475d3c52a05a06922c2ff01668938..8f03d7ba20d2f320f526da95e92d9c616c11c730 100644
--- a/components/variations/service/variations_service.cc
+++ b/components/variations/service/variations_service.cc
@@ -862,4 +862,36 @@ std::string VariationsService::GetStoredPermanentCountry() {
   return stored_country;
 }
 
+bool VariationsService::ForceSetStoredPermanentCountry(
+    const std::string& country_override) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  // FIXME: Can we check for valid country?

[Alexei Svitkine (slow), 2016/04/20 20:30:24]

Probably not really feasible.

[hamelphi, 2016/04/20 21:39:47]

Acknowledged.

+  if(country_override.empty()) {

[Alexei Svitkine (slow), 2016/04/20 20:30:24]

Nit: space after if. Also, no {}'s on a one-line if - same on line 884.

You can run "git cl format"

[hamelphi, 2016/04/20 21:39:47]

Done.

+    return false;
+  }
+
+  const base::ListValue* list_value =
+      local_state_->GetList(prefs::kVariationsPermanentConsistencyCountry);
+
+  std::string stored_country;
+  const bool got_stored_country =
+      list_value->GetSize() == 2 && list_value->GetString(1, &stored_country);
+
+  const bool needs_update =
+      !got_stored_country || stored_country != country_override;
+
+  if (!needs_update) {
+    return false;

[Alexei Svitkine (slow), 2016/04/20 20:30:24]

Not sure we actually need this logic. What's the danger of just doing it
unconditionally?

[hamelphi, 2016/04/20 21:39:47]

There is no danger. However, this allows us to tell Js if the pref has been
updated, so that we can tell the user he needs to restart his browser.

[Alexei Svitkine (slow), 2016/04/20 21:46:44]

That makes sense. However, I feel it's strange to handle "failed to set it" and
"it doesn't need to be changed" in the same way. Can these two cases be
differentiated?

On the other hand, maybe we can simply not treat the "country empty" case as a
failure and instead completely clear the pref? This would simulate the
experience a user would have if there was no country pref (e.g. first run).

+  }
+  std::string current_version = version_info::GetVersionNumber();
+
+  base::ListValue new_list_value;
+  new_list_value.AppendString(current_version);
+  new_list_value.AppendString(country_override);
+  local_state_->Set(prefs::kVariationsPermanentConsistencyCountry,
+                    new_list_value);

[Alexei Svitkine (slow), 2016/04/20 20:30:24]

Can you make a helper function for this logic since it seems to be used in the
other function that changes this?

[hamelphi, 2016/04/20 21:39:47]

Done.

+  return true;
+}
+
 }  // namespace variations