Refactor WebApkServiceConnectionManager.

chrome/browser/notifications/notification_platform_bridge_android.h

Index: chrome/browser/notifications/notification_platform_bridge_android.h
diff --git a/chrome/browser/notifications/notification_platform_bridge_android.h b/chrome/browser/notifications/notification_platform_bridge_android.h
index 9b2af9ae127a4370a8c9cb89f597386daf500f29..21825913a070434256b7b7a09ef9ffbbb4e31c38 100644
--- a/chrome/browser/notifications/notification_platform_bridge_android.h
+++ b/chrome/browser/notifications/notification_platform_bridge_android.h
@@ -13,6 +13,7 @@
 
 #include "base/android/scoped_java_ref.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "chrome/browser/notifications/displayed_notifications_dispatch_callback.h"
 #include "chrome/browser/notifications/notification_common.h"
 #include "chrome/browser/notifications/notification_platform_bridge.h"
@@ -62,6 +63,13 @@ class NotificationPlatformBridgeAndroid : public NotificationPlatformBridge {
       const base::android::JavaParamRef<jstring>& java_tag,
       jboolean by_user);
 
+  // Called after querying the webapk package is done on Java side.
+  void OnQueryWebApkPackage(

[pkotwicz, 2017/07/14 23:20:04]

The order of the functions in the .h file should match the order of the
functions in the .cc file

[Xi Han, 2017/07/17 20:45:31]

The following four functions are overriding functions of
NotificationPlatformBridge, and it is better to keep them together. After them,
are public static functions. So I put OnQueryWebApkPackage() before them.

[pkotwicz, 2017/07/18 17:30:46]

You can change the order of the functions in the .cc file to match the order in
the .h file as well :)

[Xi Han, 2017/07/18 22:36:51]

In .cc file,  the OnQueryWebApkPackage() is called in the Display(), so I feel
it is more naturally to have it after Display().

[pkotwicz, 2017/07/19 16:09:13]

From the Chromium style guide: "Function declaration order should match function
definition order."
(https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md)

[Xi Han, 2017/07/21 20:36:33]

Done.

+      JNIEnv* env,
+      const base::android::JavaParamRef<jobject>& java_object,
+      const base::android::JavaParamRef<jstring>& java_webapk_package,
+      const jlong jcallback_pointer);
+
   // NotificationPlatformBridge implementation.
   void Display(NotificationCommon::Type notification_type,
                const std::string& notification_id,
@@ -97,6 +105,12 @@ class NotificationPlatformBridgeAndroid : public NotificationPlatformBridge {
     std::string webapk_package;
   };
 
+  void OnQueryWebApkPackageInternal(const std::string& notification_id,
+                                    const std::string& profile_id,
+                                    bool incognito,
+                                    const Notification& notification,
+                                    const std::string& webapk_package);

[pkotwicz, 2017/07/18 17:30:46]

This function does not seem to be implemented in the .cc file. Remove

[Xi Han, 2017/07/18 22:36:51]

I renamed it to DisplayInternal() in patch set 9, so that is why you didn't see
it in the .cc file.

+
   // Mapping of notification id to renegerated notification info.
   // TODO(peter): Remove this map once notification delegate ids for Web
   // notifications are created by the content/ layer.
@@ -105,6 +119,9 @@ class NotificationPlatformBridgeAndroid : public NotificationPlatformBridge {
 
   base::android::ScopedJavaGlobalRef<jobject> java_object_;
 
+  // Factory for the creating refs in callbacks.
+  base::WeakPtrFactory<NotificationPlatformBridgeAndroid> weak_ptr_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(NotificationPlatformBridgeAndroid);
 };