Refactor WebApkServiceConnectionManager.

chrome/browser/notifications/notification_platform_bridge_android.h

 // Copyright 2014 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_NOTIFICATIONS_NOTIFICATION_PLATFORM_BRIDGE_ANDROID_H_
 #define CHROME_BROWSER_NOTIFICATIONS_NOTIFICATION_PLATFORM_BRIDGE_ANDROID_H_
 
 #include <jni.h>
 #include <stdint.h>
 #include <map>
 #include <set>
 #include <string>
 
 #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"
 
 namespace user_prefs {
 class PrefRegistrySyncable;
 }
 
 // Implementation of the NotificationPlatformBridge for Android, which defers to
 // the Android framework for displaying notifications.
@@ skipping 29 matching lines @@
   void OnNotificationClosed(
       JNIEnv* env,
       const base::android::JavaParamRef<jobject>& java_object,
       const base::android::JavaParamRef<jstring>& java_notification_id,
       const base::android::JavaParamRef<jstring>& java_origin,
       const base::android::JavaParamRef<jstring>& java_profile_id,
       jboolean incognito,
       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,
                const std::string& profile_id,
                bool incognito,
                const Notification& notification) override;
   void Close(const std::string& profile_id,
              const std::string& notification_id) override;
   void GetDisplayed(
       const std::string& profile_id,
@@ skipping 15 matching lines @@
     RegeneratedNotificationInfo(const std::string& origin,
                                 const std::string& tag,
                                 const std::string& webapk_package);
     ~RegeneratedNotificationInfo();
 
     std::string origin;
     std::string tag;
     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.
   std::map<std::string, RegeneratedNotificationInfo>
       regenerated_notification_infos_;
 
   base::android::ScopedJavaGlobalRef<jobject> java_object_;
 
+  // Factory for the creating refs in callbacks.
+  base::WeakPtrFactory<NotificationPlatformBridgeAndroid> weak_ptr_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(NotificationPlatformBridgeAndroid);
 };
 
 #endif  // CHROME_BROWSER_NOTIFICATIONS_NOTIFICATION_PLATFORM_BRIDGE_ANDROID_H_