Only show G+ promo for users logged into G+

chrome/browser/web_resource/notification_promo.h

 // 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.
 
 #ifndef CHROME_BROWSER_WEB_RESOURCE_NOTIFICATION_PROMO_H_
 #define CHROME_BROWSER_WEB_RESOURCE_NOTIFICATION_PROMO_H_
 #pragma once
 
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/gtest_prod_util.h"
+#include "net/url_request/url_request_context.h"
 
 namespace base {
   class DictionaryValue;
 }
 
+namespace net {
+  class URLRequestContextGetter;
+}
+
 class PrefService;
-
+class Profile;
 // Helper class for PromoResourceService that parses promo notification info
 // from json or prefs.
-class NotificationPromo {
+class NotificationPromo
+  : public base::RefCountedThreadSafe<NotificationPromo> {
  public:
   class Delegate {
    public:
      virtual ~Delegate() {}
      virtual void OnNewNotification(double start, double end) = 0;
      // For testing.
      virtual bool IsBuildAllowed(int builds_targeted) const { return false; }
   };
 
-  explicit NotificationPromo(PrefService* prefs, Delegate* delegate);
+  // Static factory for creating new promo objects.
+  static NotificationPromo* Factory(Profile* profile, Delegate* delegate);
+
 
   // Initialize from json/prefs.
   void InitFromJson(const base::DictionaryValue& json);
   void InitFromPrefs();
 
   // Can this promo be shown?
   bool CanShow() const;
 
   // Calculates promo notification start time with group-based time slice
   // offset.
   double StartTimeWithOffset() const;
 
   // Helpers for NewTabPageHandler.
   void HandleClosed();
   bool HandleViewed();  // returns true if views exceeds maximum allowed.
 
   // Register preferences.
   static void RegisterUserPrefs(PrefService* prefs);
 
+ protected:
+  NotificationPromo(Profile* profile, Delegate* delegate);

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

please make these private (the ctor, friend decl and dtor. I think friend
declaration comes before ctor).

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

Done.

+
+  friend class base::RefCountedThreadSafe<NotificationPromo>;
+  virtual ~NotificationPromo();
+
  private:
   // For testing.
   friend class NotificationPromoTestDelegate;
   FRIEND_TEST_ALL_PREFIXES(PromoResourceServiceTest, GetNextQuestionValueTest);
   FRIEND_TEST_ALL_PREFIXES(PromoResourceServiceTest, NewGroupTest);
 
   // Users are randomly assigned to one of kMaxGroupSize + 1 buckets, in order
   // to be able to roll out promos slowly, or display different promos to
   // different groups.
   static const int kMaxGroupSize = 99;
 

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

Please add an enum here like:
enum FEATURE {
  NO_FEATURE = 0,
  FEATURE_GPLUS = 1,
};

Please get rid of the gplus_ variable and preference. You can add an accessor
like:
bool gplus() const { return feature_mask_ & FEATURE_GPLUS; }

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

I agree that this is a cleaner way to go, but it seems like the gplus_ variable
is still needed to record whether or not we found a G+ cookie?

   // Parse the answers array element. Set the data members of this instance
   // and trigger OnNewNotification callback if necessary.
   void Parse(const base::DictionaryValue* dict);
 
   // Set promo notification params from a question string, which is of the form
   // <build_type>:<time_slice>:<max_group>:<max_views>
   void ParseParams(const base::DictionaryValue* dict);
 
   // Check if this promo notification is new based on start/end times,
   // and trigger events accordingly.
-  void CheckForNewNotification();
+  void CheckForNewNotification(bool found_cookie);
 
   // Actions on receiving a new promo notification.
   void OnNewNotification();
 
+  void GetCookiesCallback(const std::string& cookies);
+
+  void GetCookies(net::URLRequestContextGetter* getter);
+
   // Create a new promo notification group.
   static int NewGroup();
 
   // Returns an int converted from the question substring starting at index
   // till the next colon. Sets index to the location right after the colon.
   // Returns 0 if *err is true, and sets *err to true upon error.
   static int GetNextQuestionValue(const std::string& question,
                                   size_t* index,
                                   bool* err);
 
   // Flush data members to prefs for storage.
   void WritePrefs();
 
   // Match our channel with specified build type.
   bool IsBuildAllowed(int builds_allowed) const;
 
   // For testing.
   bool operator==(const NotificationPromo& other) const;
 
   PrefService* prefs_;
+  Profile* profile_;

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

I think profile_ should be the first data member

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

Done.

   Delegate* delegate_;
 
   double start_;
   double end_;
 
   int build_;
   int time_slice_;
   int max_group_;
   int max_views_;
+  int feature_mask_;
 
   int group_;
   int views_;
   std::string text_;
   bool closed_;
+  bool gplus_;
 
   DISALLOW_COPY_AND_ASSIGN(NotificationPromo);
 };
 
 #endif  // CHROME_BROWSER_WEB_RESOURCE_NOTIFICATION_PROMO_H_