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

components/password_manager/core/browser/password_manager_url_collection_experiment.h

Index: components/password_manager/core/browser/password_manager_url_collection_experiment.h
diff --git a/components/password_manager/core/browser/password_manager_url_collection_experiment.h b/components/password_manager/core/browser/password_manager_url_collection_experiment.h
index 96f7c280bd1aaf0f5110a2375d8954e17912b809..1b42f818bbb652f83fe4c492716fab5be5bf5490 100644
--- a/components/password_manager/core/browser/password_manager_url_collection_experiment.h
+++ b/components/password_manager/core/browser/password_manager_url_collection_experiment.h
@@ -5,6 +5,17 @@
 #ifndef COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_PASSWORD_MANAGER_URL_COLLECTION_EXPERIMENT_H_
 #define COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_PASSWORD_MANAGER_URL_COLLECTION_EXPERIMENT_H_
 
+#include <string>
+
+namespace base {
+class Clock;
+class Time;
+}
+
+namespace user_prefs {
+class PrefRegistrySyncable;
+}
+
 class PrefService;
 
 // These functions implement the algorithms according to which the "Allow to
@@ -12,10 +23,35 @@ class PrefService;
 namespace password_manager {
 namespace urls_collection_experiment {
 
+void RegisterPrefs(user_prefs::PrefRegistrySyncable* registry);
+
+// Implements algorithm which determines starting time of a period when we
+// allow to show "Allow to collect URL?" bubble to user, based on stable
+// id |profile_uuid| corresponding to user's profile.
+base::Time DetermineStartOfActivityPeriod(const std::string& profile_uuid,
+                                          int experiment_length_in_days);
 // Based on |prefs| and experiment settings, decides whether to show the
 // "Allow to collect URL?" bubble and should be called before showing it.
 // The default value is false.
-bool ShouldShowBubble(PrefService* prefs);
+bool ShouldShowBubble(PrefService* prefs, const std::string& profile_uuid);

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

The |profile_uuid| parameter needs a solid explanation. First, it needs to be
noted that it is not stored anywhere (and please correct me, if I'm wrong).
Then, it needs to be said what is it used for, and what uniqueness we assume
(when is it OK if for two users these values collide). I'd like to check if the
UUID is generated correctly, or if it could be generated in other ways, but I'm
not sure what is actually expected.

+// Same, but specifies a mock interface for clock functions for testing.
+bool ShouldShowBubble(PrefService* prefs,
+                      const std::string& profile_uuid,
+                      base::Clock* clock);
+// Should be called when user dismisses the "Allow to collect URL?" bubble.
+// It stores the statistics about interactions with the bubble in |prefs|.
+void RecordBubbleClosed(PrefService* prefs);
+
+// The name of the finch experiment controlling the algorithm.
+extern const char kExperimentName[];
+
+extern const char kGroupShowBubble[];
+extern const char kGroupNeverShowBubble[];
+
+// Determines how long experiment is active.

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

grammar: the experiment

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

Done.

+extern const char kParamLengthInDays[];
+// Determines length of time span when experiment is active for user.

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

The word "active" is used in two contexts (active and active for user), without
much hints for the difference in meaning. Please replace "active for user" with
a description of what it really means.
Also, "length of time span" seems to be equivalent to the plain "how long" you
use above. Please try to simplify the language in the comments.

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

Done.

+extern const char kParamTimePeriodInDays[];
 
 }  // namespace urls_collection_experiment
 }  // namespace password_manager