Obfuscated Gaia ID fetcher

chrome/browser/extensions/api/push_messaging/push_messaging_api.h

Index: chrome/browser/extensions/api/push_messaging/push_messaging_api.h
diff --git a/chrome/browser/extensions/api/push_messaging/push_messaging_api.h b/chrome/browser/extensions/api/push_messaging/push_messaging_api.h
index 5d8e95cc92354a4aad3fbf48def392ed1988d9ac..f78fc6fc6f05089605f6b867b7ee493fb946f2db 100644
--- a/chrome/browser/extensions/api/push_messaging/push_messaging_api.h
+++ b/chrome/browser/extensions/api/push_messaging/push_messaging_api.h
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// This file includes the JavaScript interface for Push Messaging

[dcheng, 2012/08/16 19:12:43]

Unnecessary comment.

[Pete Williamson, 2012/08/19 22:05:14]

Done.

+
 #ifndef CHROME_BROWSER_EXTENSIONS_API_PUSH_MESSAGING_PUSH_MESSAGING_API_H__
 #define CHROME_BROWSER_EXTENSIONS_API_PUSH_MESSAGING_PUSH_MESSAGING_API_H__
 
@@ -10,11 +12,15 @@
 #include "base/compiler_specific.h"
 #include "base/gtest_prod_util.h"
 #include "chrome/browser/extensions/api/push_messaging/invalidation_handler_observer.h"
+#include "chrome/browser/extensions/api/push_messaging/obfuscated_gaia_id_fetcher.h"
+#include "chrome/browser/extensions/extension_function.h"
 
 class Profile;
 
 namespace extensions {
 
+class ObfuscatedGaiaIdFetcher;
+
 // Observes a single InvalidationHandler and generates onMessage events.
 class PushMessagingEventRouter : public InvalidationHandlerObserver {
  public:
@@ -36,6 +42,33 @@ class PushMessagingEventRouter : public InvalidationHandlerObserver {
   DISALLOW_COPY_AND_ASSIGN(PushMessagingEventRouter);
 };
 
+// Implements the getChannelId function, callabel from JavaScript
+// this relies on the ObfuscatedGaiaIdFetcher to get the GAIA ID
+// that we use to build the channel ID.

[dcheng, 2012/08/16 19:12:43]

I would probably leave out this comment as it merely describes what a reader can
see by looking at line 61 and line 67.

[Pete Williamson, 2012/08/19 22:05:14]

Sure, I'll do that, but I have a question on Philosophy - I use comments to
quickly skim what a code is supposed to be doing when I do maintenance.  Is the
way we do things at Google and on Chrome to not comment what the code does? 
Personally, I think it makes the code better and more readable, but I will
absolutely go along with the way we do things here.

[dcheng, 2012/08/19 22:38:35]

It's OK to comment on the 'what' if it's something subtle or tricky. In this
case, the class name pretty much encapsulates what the class does--it's the
implementation of the GetChannelId extension function. By definition, an
extension function is callable from Javascript.

Specific comments on details of the implementation probably belong in the .cc.
However, something that might belong in this class comment might describe how
the channel ID is derived (I would probably avoid specific mention of
ObfuscatedGaiaIdFetcher).

+class PushMessagingGetChannelIdFunction : public AsyncExtensionFunction {

[Munjal (Google), 2012/08/15 23:04:19]

This class should implement the delegate interface of ObfuscatedGaiaIdFetcher
instead of ObfuscatedGaiaIdFetcher being aware of this class.

[Pete Williamson, 2012/08/19 22:05:14]

My first implementation did that, but after a discussion with Daniel, we
refactored the code to the current way so that I would not need to call AddRef. 
  After we discuss this in person, I'll fix this to do whatever we decide is the
best way.

+  public:

[Munjal (Google), 2012/08/15 23:04:19]

Indentation is off. Should be one space for public:, private: etc.

[Pete Williamson, 2012/08/19 22:05:14]

Oops, my bad, I let the editor indent "public:" for me.  Fixed.

+    PushMessagingGetChannelIdFunction();
+    virtual void OnObfuscatedGaiaIdFetchSuccess(
+        const std::string& access_token);
+    virtual void OnObfuscatedGaiaIdFetchFailure(

[Munjal (Google), 2012/08/15 23:04:19]

Move these methods to private once they are in the delegate class.

[Pete Williamson, 2012/08/19 22:05:14]

Noted.  Will fix when we decide the right approach

+        const GoogleServiceAuthError& error);
+
+  protected:
+    virtual ~PushMessagingGetChannelIdFunction();
+
+    // ExtensionFunction:
+    virtual bool RunImpl() OVERRIDE;
+    DECLARE_EXTENSION_FUNCTION_NAME("experimental.pushMessaging.getChannelId");
+
+  private:
+    void GetChannelIdOnIOThread();
+    void RespondOnUIThread(const std::string& gaiaId, const long status);
+    void StartGaiaIdFetch(Profile* profile);
+    ObfuscatedGaiaIdFetcher* fetcher_;
+
+    DISALLOW_COPY_AND_ASSIGN(PushMessagingGetChannelIdFunction);
+};
+
 }  // namespace extension
 
 #endif  // CHROME_BROWSER_EXTENSIONS_API_PUSH_MESSAGING_PUSH_MESSAGING_API_H__