Checks the age of device before initializing HaTS Notification

chrome/browser/chromeos/hats/hats_notification_controller.cc

Index: chrome/browser/chromeos/hats/hats_notification_controller.cc
diff --git a/chrome/browser/chromeos/hats/hats_notification_controller.cc b/chrome/browser/chromeos/hats/hats_notification_controller.cc
index 8ec8e18e9febb01fdf1d9bc88c26a7a417cecda8..d406b3d9a0a19bf1cdf95cdc22aa627cd2252c07 100644
--- a/chrome/browser/chromeos/hats/hats_notification_controller.cc
+++ b/chrome/browser/chromeos/hats/hats_notification_controller.cc
@@ -8,6 +8,7 @@
 #include "base/feature_list.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/hats/hats_dialog.h"
+#include "chrome/browser/chromeos/login/startup_utils.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/notifications/notification_ui_manager.h"
@@ -40,6 +41,15 @@ bool DidShowSurveyToProfileRecently(Profile* profile,
   return (previous_interaction_timestamp + threshold) > base::Time::Now();
 }
 
+// Returns true if atleast |threshold| time units have passed since OOBE. This
+// is an indirect measure of whether the owner has used the device for atleast
+// |threshold| amount of time.
+bool IsNewDevice(base::TimeDelta threshold) {
+  const base::TimeDelta time_since_oobe =
+      chromeos::StartupUtils::GetTimeSinceOobeFlagFileCreation();
+  return time_since_oobe <= threshold;
+}
+
 }  // namespace
 
 namespace chromeos {
@@ -54,17 +64,36 @@ const char HatsNotificationController::kNotificationId[] = "hats_notification";
 const base::TimeDelta HatsNotificationController::kHatsThresholdTime =
     base::TimeDelta::FromDays(90);
 
+// static
+const base::TimeDelta HatsNotificationController::kHatsNewDeviceThresholdTime =
+    base::TimeDelta::FromDays(7);
+
 HatsNotificationController::HatsNotificationController(Profile* profile)
-    : profile_(profile) {
-  // Add self as an observer to be notified when an internet connection is
-  // available.
-  network_portal_detector::GetInstance()->AddAndFireObserver(this);
+    : profile_(profile), weak_pointer_factory_(this) {
+  base::PostTaskAndReplyWithResult(
+      content::BrowserThread::GetBlockingPool(), FROM_HERE,
+      base::Bind(&IsNewDevice, kHatsNewDeviceThresholdTime),
+      base::Bind(&HatsNotificationController::Initialize,
+                 weak_pointer_factory_.GetWeakPtr()));
 }
 
 HatsNotificationController::~HatsNotificationController() {
   network_portal_detector::GetInstance()->RemoveObserver(this);
 }
 
+void HatsNotificationController::Initialize(bool is_new_device) {
+  if (is_new_device) {

[jdufault, 2016/06/29 19:26:10]

It'd be nice if we could move this is_new_device logic to
ShouldShowSurveyToProfile so that Initialize only does one thing.

Instead of updating the last interaction time, the new device threshold time
could be set to kHatsThresholdTime.

The problem though is that the OOBE creation time read needs to happen on a
different thread. You could make ShouldShowSurveyToProfile an async function
that takes a callback that is invoked with the result. It looks like
UserSessionManager already has a WeakPtrFactory so it shouldn't be too much work
to change.

[malaykeshav, 2016/06/29 19:38:29]

We want to consider the case of a new device as if the user has attempted the
survey to reduce clustering of data point at the same point in time, that is 7
days after getting a device.(Part of requirements discussed offline with
+omrilio@ and +abodenha@)
'ShouldShowSurveyToProfile' manages the check for whether to display the
notification for a given _profile_. Also moving it adds more complexity and hats
related logic to UserSessionManager class. Not sure if thats desirable.

[jdufault, 2016/06/29 22:27:03]

After offline discussion keeping the existing structure lgtm.

+    // If the device is new, then we do not show any survey or notificaiton.

[jdufault, 2016/06/29 22:27:03]

Can you update the comment to be more specific, ie,

// This device has been chosen for a survey, but it is too new. Instead 
// of showing the user the survey, just mark it as completed.

[malaykeshav, 2016/06/29 22:32:34]

+1 
Done

+    // We also update the preference flag as if the user just attempted the
+    // survey.
+    UpdateLastInteractionTime();
+    return;
+  }
+  // Add self as an observer to be notified when an internet connection is

[jdufault, 2016/06/29 19:26:10]

nit: newline before comment

[malaykeshav, 2016/06/29 19:38:29]

Done

+  // available.
+  network_portal_detector::GetInstance()->AddAndFireObserver(this);
+}
+
 // static
 // TODO(malaykeshav): Add check for @google accounts.
 bool HatsNotificationController::ShouldShowSurveyToProfile(Profile* profile) {