Refactor NotificationPlatformBridgeLinux

chrome/browser/notifications/notification_platform_bridge_linux.h

Index: chrome/browser/notifications/notification_platform_bridge_linux.h
diff --git a/chrome/browser/notifications/notification_platform_bridge_linux.h b/chrome/browser/notifications/notification_platform_bridge_linux.h
index d86935c95a75f55430d1b4f42c1eda86d8522c56..840d133d8d931a9f875414791a951e6752bbbef7 100644
--- a/chrome/browser/notifications/notification_platform_bridge_linux.h
+++ b/chrome/browser/notifications/notification_platform_bridge_linux.h
@@ -5,22 +5,24 @@
 #ifndef CHROME_BROWSER_NOTIFICATIONS_NOTIFICATION_PLATFORM_BRIDGE_LINUX_H_
 #define CHROME_BROWSER_NOTIFICATIONS_NOTIFICATION_PLATFORM_BRIDGE_LINUX_H_
 
-#include <gio/gio.h>
-
 #include <unordered_map>
 
 #include "base/macros.h"
-#include "base/memory/weak_ptr.h"
+#include "base/optional.h"
 #include "chrome/browser/notifications/notification_platform_bridge.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
-#include "ui/base/glib/glib_signal.h"
-#include "ui/base/glib/scoped_gobject.h"
+
+namespace dbus {
+class Bus;
+class ObjectProxy;
+class Signal;
+}
 
 class NotificationPlatformBridgeLinux : public NotificationPlatformBridge,
                                         public content::NotificationObserver {
  public:
-  explicit NotificationPlatformBridgeLinux(GDBusProxy* notification_proxy);
+  NotificationPlatformBridgeLinux();
 
   ~NotificationPlatformBridgeLinux() override;
 
@@ -37,48 +39,38 @@ class NotificationPlatformBridgeLinux : public NotificationPlatformBridge,
       bool incognito,
       const GetDisplayedNotificationsCallback& callback) const override;
 
-  // Called from NotifyCompleteReceiver().
-  void NotifyCompleteInternal(gpointer user_data, GVariant* value);
+  void IsConnected(base::OnceCallback<void(bool)> callback);

[Lei Zhang, 2017/04/21 22:45:18]

Maybe rename to CheckConnection? IsConnected sounds like it ought to return a
bool.

[Tom (Use chromium acct), 2017/04/24 20:46:15]

Done.

 
  private:
+  struct ResourceFile;

[Lei Zhang, 2017/04/21 22:45:18]

Leave as it was, AKA sorted.

[Tom (Use chromium acct), 2017/04/24 20:46:15]

Done.

   struct NotificationData;
-  struct ResourceFiles;
 
   // content::NotificationObserver:
   void Observe(int type,
                const content::NotificationSource& source,
                const content::NotificationDetails& details) override;
 
-  // Sets up a task to call NotifyNow() after an IO thread writes the
-  // necessary resource files.
-  void Notify(const Notification& notification,
-              NotificationData* data,
-              GAsyncReadyCallback callback,
-              gpointer user_data);
+  // Sets up the D-Bus connection.
+  void Init();
+
+  // Closes the D-Bus connection and deletes notification resources.
+  void CleanUp();
+  void CleanUpInternal();

[Lei Zhang, 2017/04/21 22:45:18]

CleanUpOnFileThread?

[Tom (Use chromium acct), 2017/04/24 20:46:15]

Done.

 
   // Makes the "Notify" call to D-Bus.
-  void NotifyNow(const Notification& notification,
-                 base::WeakPtr<NotificationData> data,
-                 GAsyncReadyCallback callback,
-                 gpointer user_data,
-                 std::unique_ptr<ResourceFiles> resource_files);
+  void DisplayNow(NotificationCommon::Type notification_type,
+                  const std::string& notification_id,
+                  const std::string& profile_id,
+                  bool is_incognito,
+                  std::unique_ptr<Notification> notification);
 
   // Makes the "CloseNotification" call to D-Bus.
-  void CloseNow(uint32_t dbus_id);
+  void CloseNow(const std::string& profile_id,
+                const std::string& notification_id);
 
-  void ForwardNotificationOperation(uint32_t dbus_id,
-                                    NotificationCommon::Operation operation,
-                                    int action_index);
-
-  // GSignalReceiver: The function that GLib calls into for
-  // ActionInvoked and NotificationClosed signals.
-  CHROMEG_CALLBACK_3(NotificationPlatformBridgeLinux,
-                     void,
-                     GSignalReceiver,
-                     GDBusProxy*,
-                     const char*,
-                     const char*,
-                     GVariant*);
+  void GetDisplayedNow(const std::string& profile_id,
+                       bool incognito,
+                       const GetDisplayedNotificationsCallback& callback) const;
 
   NotificationData* FindNotificationData(const std::string& notification_id,
                                          const std::string& profile_id,
@@ -86,10 +78,26 @@ class NotificationPlatformBridgeLinux : public NotificationPlatformBridge,
 
   NotificationData* FindNotificationData(uint32_t dbus_id);
 
-  ScopedGObject<GDBusProxy> notification_proxy_;
+  void ForwardNotificationOperation(NotificationData* data,
+                                    NotificationCommon::Operation operation,
+                                    int action_index);
 
-  // Used to disconnect from "g-signal" during destruction.
-  gulong proxy_signal_handler_ = 0;
+  void OnActionInvoked(dbus::Signal* signal);
+
+  void OnNotificationClosed(dbus::Signal* signal);
+
+  // Called once the connection has been set up (or not).  |success|
+  // indicates the connection is ready to use.
+  void OnConnected(bool success);
+
+  // State necessary for OnConnected() and IsConnected().
+  base::Optional<bool> connected_;
+  base::Lock connected_lock_;

[Lei Zhang, 2017/04/21 22:45:18]

Does this only protect |connected_| or |on_connected_callbacks_| as well? Not
clear from just reading what's here.

[Tom (Use chromium acct), 2017/04/24 20:46:15]

It protects both.  Added a comment to clarify

+  std::vector<base::OnceCallback<void(bool)>> on_connected_callbacks_;
+
+  scoped_refptr<dbus::Bus> bus_;
+
+  dbus::ObjectProxy* notification_proxy_ = nullptr;
 
   // A std::set<std::unique_ptr<T>> doesn't work well because
   // eg. std::set::erase(T) would require a std::unique_ptr<T>
@@ -101,8 +109,6 @@ class NotificationPlatformBridgeLinux : public NotificationPlatformBridge,
 
   content::NotificationRegistrar registrar_;
 
-  base::WeakPtrFactory<NotificationPlatformBridgeLinux> weak_factory_;
-
   DISALLOW_COPY_AND_ASSIGN(NotificationPlatformBridgeLinux);
 };