Decision whenever "Allow to collect URL?" bubble should be shown or not

components/password_manager/core/browser/password_manager_url_collection_experiment.cc

Index: components/password_manager/core/browser/password_manager_url_collection_experiment.cc
diff --git a/components/password_manager/core/browser/password_manager_url_collection_experiment.cc b/components/password_manager/core/browser/password_manager_url_collection_experiment.cc
index b8d66c5e87d38d9a328cc0c149cd260f379546f6..99f7d3935af7457f568f95846700ab2b068a8cd2 100644
--- a/components/password_manager/core/browser/password_manager_url_collection_experiment.cc
+++ b/components/password_manager/core/browser/password_manager_url_collection_experiment.cc
@@ -2,17 +2,111 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/prefs/pref_service.h"
 #include "components/password_manager/core/browser/password_manager_url_collection_experiment.h"
 
+#include "base/hash.h"
+#include "base/metrics/field_trial.h"
+#include "base/prefs/pref_service.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/time/clock.h"
+#include "base/time/default_clock.h"
+#include "base/time/time.h"
+#include "components/password_manager/core/common/password_manager_pref_names.h"
+#include "components/pref_registry/pref_registry_syncable.h"
+#include "components/variations/entropy_provider.h"
+#include "components/variations/variations_associated_data.h"
+
 namespace password_manager {
 namespace urls_collection_experiment {
 
-bool ShouldShowBubble(PrefService* prefs) {
-  // TODO(melandory) Make descision based on Finch experiment.
+namespace {
+
+const char kExperimentStartDate[] = "Mon, 15 Dec 00:00:00 2014 GMT";
+
+bool ExtractExperimentParams(int* experiment_length_in_days,
+                             base::TimeDelta* time_delta) {
+  std::map<std::string, std::string> params;
+  bool retrieved = variations::GetVariationParams(kExperimentName, &params);
+  if (!retrieved)

[vabr (Chromium), 2014/12/10 18:19:54]

nit: You could also insert the GetVariationParams call in the if. It seems like
|retrieved| is not used below.

[melandory, 2014/12/11 08:37:38]

Done.

+    return false;
+  int days = 0;
+  if (!base::StringToInt(params[kParamLengthInDays],
+                         experiment_length_in_days) ||
+      !base::StringToInt(params[kParamTimePeriodInDays], &days))
+    return false;
+  *time_delta = base::TimeDelta::FromDays(days);
+  return true;
+}
+
+bool ShowBubbleExperiment(PrefService* prefs,

[vabr (Chromium), 2014/12/10 18:19:54]

This should be called "ShouldShowBubbleIfExperimentActive", because it answers
that question. It does not show "bubble experiment".

[melandory, 2014/12/11 08:37:38]

Done.

+                          const std::string& profile_uuid,
+                          base::Clock* clock) {
+  if (prefs->GetBoolean(prefs::kWasAllowToCollectURLBubbleShown))
+    return false;
+  int experiment_length_in_days;
+  base::TimeDelta time_delta;
+  if (!ExtractExperimentParams(&experiment_length_in_days, &time_delta))
+    return false;
+  base::Time beginning;
+  base::Time::FromString(kExperimentStartDate, &beginning);
+  base::Time now = clock->Now();
+  base::Time experiment_active_to_user_start =

[vabr (Chromium), 2014/12/10 18:19:55]

I might be hitting the issue of parsing long names again, but should this be
"active for user" rather than "active to user"?

[melandory, 2014/12/11 08:37:38]

Done. You are right.

+      DetermineStartOfActivityPeriod(profile_uuid, experiment_length_in_days);
+  bool greater_than_now = experiment_active_to_user_start >= now;

[vabr (Chromium), 2014/12/10 18:19:54]

You can drop both these bools, because you seem to recompute them in the return
anyway. :)
And yes, I think the recomputing in the return is fine and clear, no need to
cache in variables.

[melandory, 2014/12/11 08:37:38]

Done. Sorry was using them for debug purposes and forgot to remove then.

+  bool less_than = experiment_active_to_user_start < now + time_delta;
+  return experiment_active_to_user_start >= now &&
+         experiment_active_to_user_start < now + time_delta;
+}
+
+}  // namespace
+
+void RegisterPrefs(user_prefs::PrefRegistrySyncable* registry) {
+  registry->RegisterBooleanPref(
+      password_manager::prefs::kWasAllowToCollectURLBubbleShown,
+      false,  // bubble hasn't been shown yet
+      user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
+}
+
+const char kExperimentName[] = "AskToSubmitURLBubble";
+const char kGroupShowBubble[] = "ShowBubble";
+const char kGroupNeverShowBubble[] = "NeverShowBubble";
+const char kParamLengthInDays[] = "experiment_length_in_days";
+const char kParamTimePeriodInDays[] = "experiment_active_for_user_period";
+
+base::Time DetermineStartOfActivityPeriod(const std::string& profile_uuid,
+                                          int experiment_length_in_days) {
+  const base::FieldTrial::EntropyProvider* entropy_provider =
+      new metrics::SHA1EntropyProvider(profile_uuid);
+  base::Time beginning;
+  base::Time::FromString(kExperimentStartDate, &beginning);
+  return beginning + base::TimeDelta::FromDays(
+                         experiment_length_in_days *
+                         entropy_provider->GetEntropyForTrial(
+                             kExperimentName, base::Hash(profile_uuid)));
+}
+
+bool ShouldShowBubble(PrefService* prefs, const std::string& profile_uuid) {
+  base::DefaultClock clock;
+  return ShouldShowBubble(prefs, profile_uuid, &clock);
+}
+
+bool ShouldShowBubble(PrefService* prefs,
+                      const std::string& profile_uuid,
+                      base::Clock* clock) {
+  if (base::FieldTrialList::TrialExists(kExperimentName)) {

[melandory, 2014/12/11 08:37:38]

asvitkine@: "get the group for "AskToSubmitURLBubble" field trial and get the
value of the "ShowBubble" param for that group" - giving you the correct result
without you needing to use the raw FieldTrialList API directly."

[melandory, 2014/12/11 17:05:40]

Done.

+    std::string group_name =
+        base::FieldTrialList::FindFullName(kExperimentName);
+    if (group_name == kGroupShowBubble)
+      return ShowBubbleExperiment(prefs, profile_uuid, clock);
+  }
   // "Do not show" is the default case.
   return false;
 }
 
+void RecordBubbleClosed(PrefService* prefs) {
+  prefs->SetBoolean(password_manager::prefs::kWasAllowToCollectURLBubbleShown,
+                    true);
+}
+
 }  // namespace urls_collection_experiment
 }  // namespace password_manager