Only show G+ promo for users logged into G+

chrome/browser/web_resource/notification_promo.cc

Index: chrome/browser/web_resource/notification_promo.cc
===================================================================
--- chrome/browser/web_resource/notification_promo.cc	(revision 108876)
+++ chrome/browser/web_resource/notification_promo.cc	(working copy)
@@ -4,14 +4,20 @@
 
 #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.
@@ -33,6 +39,10 @@
 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;
@@ -52,9 +62,10 @@
 
 }  // namespace
 
-NotificationPromo::NotificationPromo(PrefService* prefs, Delegate* delegate)
-    : prefs_(prefs),
+NotificationPromo::NotificationPromo(Profile* profile, Delegate* delegate)
+    : profile_(profile),
       delegate_(delegate),
+      prefs_(profile_->GetPrefs()),
       start_(0.0),
       end_(0.0),
       build_(0),
@@ -64,10 +75,15 @@
       group_(0),
       views_(0),
       text_(),
-      closed_(false) {
-  DCHECK(prefs);
+      closed_(false),
+      gplus_(false),
+      feature_mask_(0) {
+  DCHECK(profile);
+  DCHECK(prefs_);
 }
 
+NotificationPromo::~NotificationPromo() {}
+
 void NotificationPromo::InitFromJson(const DictionaryValue& json) {
   DictionaryValue* dict;
   if (json.GetDictionary(kHeaderProperty, &dict)) {
@@ -81,10 +97,36 @@
       }
     }
   }
+  content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE,
+        base::Bind(&NotificationPromo::GetCookies, base::Unretained(this),
+                   base::Unretained(profile_->GetRequestContext())));
+}
 
-  CheckForNewNotification();
+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>::const_iterator current = cookie_list.begin();
+       current != cookie_list.end();
+       ++current) {
+    if ((*current).find(kGPlusDomainSecureCookieId) == 0) {
+      found_cookie = true;
+      break;
+    }
+  }
+  content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
+      base::Bind(&NotificationPromo::CheckForNewNotification, this,
+                 found_cookie));
 }
 
+void NotificationPromo::GetCookies(net::URLRequestContextGetter* getter) {
+    DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
+    getter->GetURLRequestContext()->cookie_store()->
+        GetCookiesWithOptionsAsync(
+            GURL(kGPlusDomainUrl), net::CookieOptions(),
+            base::Bind(&NotificationPromo::GetCookiesCallback, this));
+}
+
 void NotificationPromo::Parse(const DictionaryValue* dict) {
   std::string key;
   if (dict->GetString(kIdentifierProperty, &key)) {
@@ -110,6 +152,7 @@
   time_slice_ = GetNextQuestionValue(question, &index, &err);
   max_group_ = GetNextQuestionValue(question, &index, &err);
   max_views_ = GetNextQuestionValue(question, &index, &err);
+  feature_mask_ = GetNextQuestionValue(question, &index, &err);
 
   if (err ||
       OutOfBounds(build_, PromoResourceService::NO_BUILD,
@@ -130,14 +173,17 @@
   }
 }
 
-void NotificationPromo::CheckForNewNotification() {
+void NotificationPromo::CheckForNewNotification(bool found_cookie) {
+  gplus_ = found_cookie;

[achuithb, 2011/11/16 23:17:33]

Instead of this, you would do:
if (found_cookie)
  feature_mask_ |= FEATURE_GPLUS;

[achuithb, 2011/11/16 23:27:03]

Ok, I see the problem. I'm sorry, I'm re-using feature_mask_. 
gplus_ = found_cookie;
is ok.

   const double old_start = GetTimeFromPrefs(prefs_, prefs::kNTPPromoStart);
   const double old_end = GetTimeFromPrefs(prefs_, prefs::kNTPPromoEnd);
+  const bool old_gplus = prefs_->GetBoolean(prefs::kNTPPromoIsLoggedInToPlus);

[achuithb, 2011/11/16 23:17:33]

don't need this.

[Cait (Slow), 2011/11/17 00:05:59]

I had this in here to force a refresh of the promo if the users gplus status
changed (i.e. they weren't logged in before, but now they are, or vice versa).
Otherwise, when we update cookies, the new gplus value will get set, but the
promo won't get updated until the cache is invalidated for a another reason.

[achuithb, 2011/11/17 00:09:34]

Ah, ok. That makes sense. Could you add a comment noting this?

   const bool has_views = prefs_->HasPrefPath(prefs::kNTPPromoViewsMax);

[achuithb, 2011/11/16 23:17:33]

don't need this line either.

[Cait (Slow), 2011/11/17 00:05:59]

Done.

-
+  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.

[achuithb, 2011/11/16 23:17:33]

please change this comment:
s/max_views/feature_mask

[Cait (Slow), 2011/11/17 00:05:59]

Done.

-  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_)

[achuithb, 2011/11/16 23:27:03]

Just old_feature_mask should be sufficient.

[Cait (Slow), 2011/11/17 00:05:59]

Done.

     OnNewNotification();
 }
 
@@ -187,9 +233,21 @@
   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_);
@@ -203,6 +261,8 @@
   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() {
@@ -235,6 +295,12 @@
 
   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 {
@@ -244,7 +310,8 @@
       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);

[achuithb, 2011/11/16 23:27:03]

I think this condition should be:
(feature_mask_ & FEATURE_GPLUS) == !gplus_

[Cait (Slow), 2011/11/17 00:05:59]

Done.

 }
 
 void NotificationPromo::HandleClosed() {
@@ -305,5 +372,7 @@
          group_ == other.group_ &&
          views_ == other.views_ &&
          text_ == other.text_ &&
-         closed_ == other.closed_;
+         closed_ == other.closed_ &&
+         gplus_ == other.gplus_ &&
+         feature_mask_ == other.feature_mask_;
 }