Only show G+ promo for users logged into G+

chrome/browser/web_resource/notification_promo.cc

 // Copyright (c) 2011 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 "chrome/browser/web_resource/notification_promo.h"
 
+#include "base/bind.h"
 #include "base/rand_util.h"
 #include "base/string_number_conversions.h"
+#include "base/string_split.h"
 #include "base/time.h"
 #include "base/values.h"
 #include "chrome/browser/prefs/pref_service.h"
+#include "chrome/browser/profiles/profile_impl.h"
 #include "chrome/browser/web_resource/promo_resource_service.h"
 #include "chrome/common/pref_names.h"
+#include "googleurl/src/gurl.h"
+#include "net/base/cookie_store.h"
+
 
 namespace {
 
 // Maximum number of views.
 static const int kMaxViews = 1000;
 
 // Maximum number of hours for each time slice (4 weeks).
 static const int kMaxTimeSliceHours = 24 * 7 * 4;
 
 bool OutOfBounds(int var, int min, int max) {
   return var < min || var > max;
 }
 
 static const char kHeaderProperty[] = "topic";
 static const char kArrayProperty[] = "answers";
 static const char kIdentifierProperty[] = "name";
 static const char kStartPropertyValue[] = "promo_start";
 static const char kEndPropertyValue[] = "promo_end";
 static const char kTextProperty[] = "tooltip";
 static const char kTimeProperty[] = "inproduct";
 static const char kParamsProperty[] = "question";
 
+static const char kGPlusDomainUrl[] = "http://plus.google.com/";
+static const char kGPlusDomainSecureCookieId[] = "SID=";
+static const char kSplitStringToken = ';';
+
 // Time getters.
 double GetTimeFromDict(const DictionaryValue* dict) {
   std::string time_str;
   if (!dict->GetString(kTimeProperty, &time_str))
     return 0.0;
 
   base::Time time;
   if (time_str.empty() || !base::Time::FromString(time_str.c_str(), &time))
     return 0.0;
 
   return time.ToDoubleT();
 }
 
 double GetTimeFromPrefs(PrefService* prefs, const char* pref) {
   return prefs->HasPrefPath(pref) ? prefs->GetDouble(pref) : 0.0;
 }
 
 }  // namespace
 
-NotificationPromo::NotificationPromo(PrefService* prefs, Delegate* delegate)
-    : prefs_(prefs),
-      delegate_(delegate),
+NotificationPromo::NotificationPromo(Profile* profile, Delegate* delegate)
+    : delegate_(delegate),
+      profile_(profile),

[achuithb, 2011/11/16 20:33:11]

Maybe keep this order consistent with the order of the params? It's cleaner.
profile_ should be first.

[Cait (Slow), 2011/11/16 23:05:24]

Done.

       start_(0.0),
       end_(0.0),
       build_(0),
       time_slice_(0),
       max_group_(0),
       max_views_(0),
       group_(0),
       views_(0),
       text_(),
-      closed_(false) {
-  DCHECK(prefs);
+      closed_(false),
+      gplus_(false),
+      feature_mask_(0) {
+  DCHECK(profile);
+  prefs_ = profile_->GetPrefs();

[achuithb, 2011/11/16 20:33:11]

Could you please put this in the initializer list with the others?
profile_(profile),
prefs_(profile->GetPrefs()),
delegate_(delegate),
etc.

[Cait (Slow), 2011/11/16 23:05:24]

Done.

+  DCHECK(prefs_);
 }
 
+NotificationPromo::~NotificationPromo() {}
+
 void NotificationPromo::InitFromJson(const DictionaryValue& json) {
   DictionaryValue* dict;
   if (json.GetDictionary(kHeaderProperty, &dict)) {
     ListValue* answers;
     if (dict->GetList(kArrayProperty, &answers)) {
       for (ListValue::const_iterator it = answers->begin();
            it != answers->end();
            ++it) {
         if ((*it)->IsType(Value::TYPE_DICTIONARY))
           Parse(static_cast<DictionaryValue*>(*it));
       }
     }
   }
 
-  CheckForNewNotification();
+  GetCookies(profile_->GetRequestContext());

[achuithb, 2011/11/16 20:33:11]

Could you please call PostTask to BrowserThread::IO here instead?

[Cait (Slow), 2011/11/16 23:05:24]

Done.

+}
+
+void NotificationPromo::GetCookiesCallback(const std::string& cookies) {
+  std::vector<std::string> cookie_list;
+  bool found_cookie = false;
+  base::SplitString(cookies, kSplitStringToken, &cookie_list);
+  for (std::vector<std::string>::iterator current = cookie_list.begin();

[achuithb, 2011/11/16 20:33:11]

Please use const_iterator instead.

[Cait (Slow), 2011/11/16 23:05:24]

Done.

+       current != cookie_list.end();
+       ++current) {
+    size_t position = (*current).find(kGPlusDomainSecureCookieId);
+    if (position == 0) {

[achuithb, 2011/11/16 20:33:11]

I think the position var is unnecessary:
if ((*current).find(...) == 0)

[Cait (Slow), 2011/11/16 23:05:24]

Done.

+      found_cookie = true;
+      break;
+    }
+  }
+  gplus_ = found_cookie;

[achuithb, 2011/11/16 20:33:11]

Please remove this line. It's not safe to do this (modify state of
NotificationPromo) on the IO thread, and you're doing this in
CheckForNewNotification already.

[Cait (Slow), 2011/11/16 23:05:24]

Done.

+  content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
+      base::Bind(&NotificationPromo::CheckForNewNotification,
+                 base::Unretained(this), found_cookie));

[achuithb, 2011/11/16 20:33:11]

Please do not use base::Unretained here. 
base::Bind(&NotificationPromo::CheckForNewNotification, this, found_cookie);

[Cait (Slow), 2011/11/16 23:05:24]

Done.

+}
+
+void NotificationPromo::GetCookies(net::URLRequestContextGetter* getter) {
+  if (content::BrowserThread::CurrentlyOn(content::BrowserThread::IO)) {

[achuithb, 2011/11/16 20:33:11]

Please replace this if with a DCHECK like:
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO);

[Cait (Slow), 2011/11/16 23:05:24]

Done.

+    getter->GetURLRequestContext()->cookie_store()->
+        GetCookiesWithOptionsAsync(
+            GURL(kGPlusDomainUrl), net::CookieOptions(),
+            base::Bind(&NotificationPromo::GetCookiesCallback,
+                       base::Unretained(this)));

[achuithb, 2011/11/16 20:33:11]

Please do not use base::Unretained here; you should just do:
base::Bind(&NotificationPromo::GetCookiesCallback, this);

[Cait (Slow), 2011/11/16 23:05:24]

Done.

+  } else {
+    content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE,

[achuithb, 2011/11/16 20:33:11]

Please PostTask to the IO thread at the calling site. You don't need this code.

[Cait (Slow), 2011/11/16 23:05:24]

Done.

+        base::Bind(&NotificationPromo::GetCookies, base::Unretained(this),
+                   base::Unretained(getter)));
+  }
 }
 
 void NotificationPromo::Parse(const DictionaryValue* dict) {
   std::string key;
   if (dict->GetString(kIdentifierProperty, &key)) {
     if (key == kStartPropertyValue) {
       ParseParams(dict);
       dict->GetString(kTextProperty, &text_);
       start_ = GetTimeFromDict(dict);
     } else if (key == kEndPropertyValue) {
       end_ = GetTimeFromDict(dict);
     }
   }
 }
 
 void NotificationPromo::ParseParams(const DictionaryValue* dict) {
   std::string question;
   if (!dict->GetString(kParamsProperty, &question))
     return;
 
   size_t index = 0;
   bool err = false;
 
   build_ = GetNextQuestionValue(question, &index, &err);
   time_slice_ = GetNextQuestionValue(question, &index, &err);
   max_group_ = GetNextQuestionValue(question, &index, &err);
   max_views_ = GetNextQuestionValue(question, &index, &err);
+  if (index < question.length())

[achuithb, 2011/11/16 20:33:11]

You don't need this. GetNextQuestionValue handles this case.

[Cait (Slow), 2011/11/16 23:05:24]

Done.

+    feature_mask_ = GetNextQuestionValue(question, &index, &err);
 
   if (err ||
       OutOfBounds(build_, PromoResourceService::NO_BUILD,
           PromoResourceService::ALL_BUILDS) ||
       OutOfBounds(time_slice_, 0, kMaxTimeSliceHours) ||
       OutOfBounds(max_group_, 0, kMaxGroupSize) ||
       OutOfBounds(max_views_, 0, kMaxViews)) {
     // If values are not valid, do not show promo notification.
     DLOG(ERROR) << "Invalid server data, question=" << question <<
         ", build=" << build_ <<
         ", time_slice=" << time_slice_ <<
         ", max_group=" << max_group_ <<
         ", max_views=" << max_views_;
     build_ = PromoResourceService::NO_BUILD;
     time_slice_ = 0;
     max_group_ = 0;
     max_views_ = 0;
   }
 }
 
-void NotificationPromo::CheckForNewNotification() {
+void NotificationPromo::CheckForNewNotification(bool found_cookie) {
+  gplus_ = found_cookie;
   const double old_start = GetTimeFromPrefs(prefs_, prefs::kNTPPromoStart);
   const double old_end = GetTimeFromPrefs(prefs_, prefs::kNTPPromoEnd);
+  const bool old_gplus = prefs_->GetBoolean(prefs::kNTPPromoIsLoggedInToPlus);
   const bool has_views = prefs_->HasPrefPath(prefs::kNTPPromoViewsMax);
-
+  const int old_feature_mask = prefs_->GetInteger(prefs::kNTPPromoFeatureMask);
   // Trigger a new notification if the times have changed, or if
   // we previously never wrote out a max_views preference.
-  if (old_start != start_ || old_end != end_ || !has_views)
+  if (old_start != start_ || old_end != end_ || !has_views
+      || old_gplus != gplus_ || old_feature_mask != feature_mask_)
     OnNewNotification();
 }
 
 void NotificationPromo::OnNewNotification() {
   group_ = NewGroup();
   WritePrefs();
   if (delegate_)
     delegate_->OnNewNotification(StartTimeWithOffset(), end_);
 }
 
@@ skipping 29 matching lines @@
                             PrefService::UNSYNCABLE_PREF);
   prefs->RegisterIntegerPref(prefs::kNTPPromoGroup,
                              0,
                              PrefService::UNSYNCABLE_PREF);
   prefs->RegisterIntegerPref(prefs::kNTPPromoViews,
                              0,
                              PrefService::UNSYNCABLE_PREF);
   prefs->RegisterBooleanPref(prefs::kNTPPromoClosed,
                              false,
                              PrefService::UNSYNCABLE_PREF);
+  prefs->RegisterBooleanPref(prefs::kNTPPromoIsLoggedInToPlus,
+                             false,
+                             PrefService::UNSYNCABLE_PREF);
+  prefs->RegisterIntegerPref(prefs::kNTPPromoFeatureMask,
+                             0,
+                             PrefService::UNSYNCABLE_PREF);
+}
+
+// static
+NotificationPromo* NotificationPromo::Factory(Profile *profile,
+    NotificationPromo::Delegate * delegate) {
+  return new NotificationPromo(profile, delegate);
 }
 
 
 void NotificationPromo::WritePrefs() {
   prefs_->SetDouble(prefs::kNTPPromoStart, start_);
   prefs_->SetDouble(prefs::kNTPPromoEnd, end_);
 
   prefs_->SetInteger(prefs::kNTPPromoBuild, build_);
   prefs_->SetInteger(prefs::kNTPPromoGroupTimeSlice, time_slice_);
   prefs_->SetInteger(prefs::kNTPPromoGroupMax, max_group_);
   prefs_->SetInteger(prefs::kNTPPromoViewsMax, max_views_);
 
   prefs_->SetString(prefs::kNTPPromoLine, text_);
   prefs_->SetInteger(prefs::kNTPPromoGroup, group_);
   prefs_->SetInteger(prefs::kNTPPromoViews, views_);
   prefs_->SetBoolean(prefs::kNTPPromoClosed, closed_);
+  prefs_->SetBoolean(prefs::kNTPPromoIsLoggedInToPlus, gplus_);
+  prefs_->SetInteger(prefs::kNTPPromoFeatureMask, feature_mask_);
 }
 
 void NotificationPromo::InitFromPrefs() {
   if (prefs_->HasPrefPath(prefs::kNTPPromoStart))
     start_ = prefs_->GetDouble(prefs::kNTPPromoStart);
 
   if (prefs_->HasPrefPath(prefs::kNTPPromoEnd))
     end_ = prefs_->GetDouble(prefs::kNTPPromoEnd);
 
   if (prefs_->HasPrefPath(prefs::kNTPPromoBuild))
@@ skipping 12 matching lines @@
     text_ = prefs_->GetString(prefs::kNTPPromoLine);
 
   if (prefs_->HasPrefPath(prefs::kNTPPromoGroup))
     group_ = prefs_->GetInteger(prefs::kNTPPromoGroup);
 
   if (prefs_->HasPrefPath(prefs::kNTPPromoViews))
     views_ = prefs_->GetInteger(prefs::kNTPPromoViews);
 
   if (prefs_->HasPrefPath(prefs::kNTPPromoClosed))
     closed_ = prefs_->GetBoolean(prefs::kNTPPromoClosed);
+
+  if (prefs_->HasPrefPath(prefs::kNTPPromoIsLoggedInToPlus))
+    gplus_ = prefs_->GetBoolean(prefs::kNTPPromoIsLoggedInToPlus);
+
+  if (prefs_->HasPrefPath(prefs::kNTPPromoFeatureMask))
+    feature_mask_ = prefs_->GetInteger(prefs::kNTPPromoFeatureMask);
 }
 
 bool NotificationPromo::CanShow() const {
   return !closed_ &&
       !text_.empty() &&
       group_ < max_group_ &&
       views_ < max_views_ &&
       IsBuildAllowed(build_) &&
       base::Time::FromDoubleT(StartTimeWithOffset()) < base::Time::Now() &&
-      base::Time::FromDoubleT(end_) > base::Time::Now();
+      base::Time::FromDoubleT(end_) > base::Time::Now() &&
+      ((feature_mask_ & !gplus_) == 0));
 }
 
 void NotificationPromo::HandleClosed() {
   prefs_->SetBoolean(prefs::kNTPPromoClosed, true);
 }
 
 bool NotificationPromo::HandleViewed() {
   if (prefs_->HasPrefPath(prefs::kNTPPromoViewsMax))
     max_views_ = prefs_->GetInteger(prefs::kNTPPromoViewsMax);
 
@@ skipping 40 matching lines @@
          delegate_ == other.delegate_ &&
          start_ == other.start_ &&
          end_ == other.end_ &&
          build_ == other.build_ &&
          time_slice_ == other.time_slice_ &&
          max_group_ == other.max_group_ &&
          max_views_ == other.max_views_ &&
          group_ == other.group_ &&
          views_ == other.views_ &&
          text_ == other.text_ &&
-         closed_ == other.closed_;
+         closed_ == other.closed_ &&
+         gplus_ == other.gplus_;

[achuithb, 2011/11/16 20:33:11]

feature_mask_ is needed here too.

[Cait (Slow), 2011/11/16 23:05:24]

Done.

 }