Make X11 power save suspension calls asynchronous.

content/browser/power_save_blocker_x11.cc

Index: content/browser/power_save_blocker_x11.cc
diff --git a/content/browser/power_save_blocker_x11.cc b/content/browser/power_save_blocker_x11.cc
index 9cd6113e3f06ddf427460b82a291c5bb085de971..c1be00b7675985ee70e9c210f54e85a3fc49c7d9 100644
--- a/content/browser/power_save_blocker_x11.cc
+++ b/content/browser/power_save_blocker_x11.cc
@@ -93,6 +93,11 @@ class PowerSaveBlockerImpl::Delegate
   void ApplyBlock(DBusAPI api);
   void RemoveBlock(DBusAPI api);
 
+  // Asynchronous callback functions for ApplyBlock and RemoveBlock.
+  // Functions do not receive ownership of |response|.
+  void ApplyBlockFinished(DBusAPI api, dbus::Response* response);
+  void RemoveBlockFinished(dbus::Response* response);
+
   // If DPMS (the power saving system in X11) is not enabled, then we don't want
   // to try to disable power saving, since on some desktop environments that may
   // enable DPMS with very poor default settings (e.g. turning off the display
@@ -115,6 +120,15 @@ class PowerSaveBlockerImpl::Delegate
   bool enqueue_apply_;
   base::Lock lock_;
 
+  // Indicates that a D-Bus power save blocking request is in flight.
+  bool block_inflight_;
+  // Used to detect erronous redundant calls to RemoveBlock().
+  bool unblock_inflight_;
+  // Indicates that RemoveBlock() is called before ApplyBlock() has finished.
+  // If it's true, then the RemoveBlock() call will be processed immediately
+  // after ApplyBlock() has finished.
+  bool unblock_enqueued_;
+
   scoped_refptr<dbus::Bus> bus_;
 
   // The cookie that identifies our inhibit request,
@@ -139,6 +153,9 @@ void PowerSaveBlockerImpl::Delegate::Init() {
   base::AutoLock lock(lock_);
   DCHECK(!enqueue_apply_);
   enqueue_apply_ = true;
+  block_inflight_ = false;
+  unblock_inflight_ = false;
+  unblock_enqueued_ = false;
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
                           base::Bind(&Delegate::InitOnUIThread, this));
 }
@@ -172,7 +189,8 @@ void PowerSaveBlockerImpl::Delegate::InitOnUIThread() {
 
 void PowerSaveBlockerImpl::Delegate::ApplyBlock(DBusAPI api) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
-  DCHECK(!bus_.get());  // ApplyBlock() should only be called once.
+  DCHECK(!bus_);  // ApplyBlock() should only be called once.
+  DCHECK(!block_inflight_);
 
   dbus::Bus::Options options;
   options.bus_type = dbus::Bus::SESSION;
@@ -233,26 +251,52 @@ void PowerSaveBlockerImpl::Delegate::ApplyBlock(DBusAPI api) {
       break;
   }
 
-  // We could do this method call asynchronously, but if we did, we'd need to
-  // handle the case where we want to cancel the block before we get a reply.
-  // We're on the FILE thread so it should be OK to block briefly here.
-  scoped_ptr<dbus::Response> response(object_proxy->CallMethodAndBlock(
-      method_call.get(), dbus::ObjectProxy::TIMEOUT_USE_DEFAULT));
+  block_inflight_ = true;
+  object_proxy->CallMethod(
+      method_call.get(), dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+      base::Bind(&PowerSaveBlockerImpl::Delegate::ApplyBlockFinished, this,
+                 api));
+}
+
+void PowerSaveBlockerImpl::Delegate::ApplyBlockFinished(
+    DBusAPI api,
+    dbus::Response* response) {
+  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+  DCHECK(bus_);
+  DCHECK(block_inflight_);
+  block_inflight_ = false;
+
   if (response) {
     // The method returns an inhibit_cookie, used to uniquely identify
     // this request. It should be used as an argument to Uninhibit()
     // in order to remove the request.
-    dbus::MessageReader message_reader(response.get());
+    dbus::MessageReader message_reader(response);
     if (!message_reader.PopUint32(&inhibit_cookie_))
       LOG(ERROR) << "Invalid Inhibit() response: " << response->ToString();
   } else {
     LOG(ERROR) << "No response to Inhibit() request!";
   }
+
+  if (unblock_enqueued_) {
+    unblock_enqueued_ = false;
+    // RemoveBlock() was called while the Inhibit operation was in flight,
+    // so go ahead and remove the block now.
+    BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+                            base::Bind(&Delegate::RemoveBlock, this, api_));
+  }
 }
 
 void PowerSaveBlockerImpl::Delegate::RemoveBlock(DBusAPI api) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
-  DCHECK(bus_.get());  // RemoveBlock() should only be called once.
+  DCHECK(bus_);  // RemoveBlock() should only be called once.
+  DCHECK(!unblock_inflight_);
+
+  if (block_inflight_) {
+    // Can't call RemoveBlock until ApplyBlock's asynchronous operation has
+    // finished. Enqueue it for execution once ApplyBlock is done.
+    unblock_enqueued_ = true;

[miu, 2015/05/15 18:43:31]

Should you DCHECK(!unblock_enqueued) before this line?

[Kevin M, 2015/05/15 18:55:25]

Good idea, done.

+    return;
+  }
 
   scoped_refptr<dbus::ObjectProxy> object_proxy;
   scoped_ptr<dbus::MethodCall> method_call;
@@ -279,13 +323,22 @@ void PowerSaveBlockerImpl::Delegate::RemoveBlock(DBusAPI api) {
 
   dbus::MessageWriter message_writer(method_call.get());
   message_writer.AppendUint32(inhibit_cookie_);
-  scoped_ptr<dbus::Response> response(object_proxy->CallMethodAndBlock(
-      method_call.get(), dbus::ObjectProxy::TIMEOUT_USE_DEFAULT));
+  unblock_inflight_ = true;
+  object_proxy->CallMethod(
+      method_call.get(), dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+      base::Bind(&PowerSaveBlockerImpl::Delegate::RemoveBlockFinished, this));
+}
+
+void PowerSaveBlockerImpl::Delegate::RemoveBlockFinished(
+    dbus::Response* response) {
+  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+  DCHECK(bus_);
+  unblock_inflight_ = false;
+
   if (!response)
     LOG(ERROR) << "No response to Uninhibit() request!";
   // We don't care about checking the result. We assume it works; we can't
   // really do anything about it anyway if it fails.
-  inhibit_cookie_ = 0;

[miu, 2015/05/15 18:43:31]

nit: Either leave this in, or adjust the header file comment for this member.

[Kevin M, 2015/05/15 18:55:25]

Done.

 
   bus_->ShutdownAndBlock();
   bus_ = NULL;