Merge lp:~osomon/oxide/applicationstatechanged into lp:~oxide-developers/oxide/oxide.trunk

[Olivier Tilloy]

Monitor application state changes, and update PowerSaveBlocker state accordingly
(remove the screen dim lock when the app goes into the background, and restore it when it comes to the foreground).

[Chris Coulson (chrisccoulson)]

Thanks for this. I've added a couple of comments inline.

Also, I wonder whether it would be better to drop the state argument from the notification and add a BrowserPlatformIntegration::GetApplicationState() instead. There's always the possibility of a PowerSaveBlocker being created inbetween the notification firing and the application being stopped.

review: Needs Fixing

[lp:~osomon/oxide/applicationstatechanged]

863. By Olivier Tilloy on 2014-11-14

Remove useless explicit call to QObject constructor.

864. By Olivier Tilloy on 2014-11-14

Get rid of the Observe method and the non-default constructor,
and make the default constructor add itself to the BrowserPlatformIntegration singleton.

865. By Olivier Tilloy on 2014-11-14

Drop the state argument from the notification,
and add a BrowserPlatformIntegration::GetApplicationState() instead.

866. By Olivier Tilloy on 2014-11-14

Removed an unneeded slot parameter.

[Chris Coulson (chrisccoulson)]

review: Approve

=== modified file 'qt/core/browser/oxide_qt_browser_platform_integration.cc'
--- qt/core/browser/oxide_qt_browser_platform_integration.cc	2014-11-13 08:41:43 +0000
+++ qt/core/browser/oxide_qt_browser_platform_integration.cc	2014-11-14 14:36:23 +0000
@@ -47,10 +47,17 @@
 
 BrowserPlatformIntegration::BrowserPlatformIntegration(
     GLContextAdopted* gl_share_context)
-    : gl_share_context_(gl_share_context) {}
+    : gl_share_context_(gl_share_context) {
+  QObject::connect(qApp, SIGNAL(applicationStateChanged(Qt::ApplicationState)),
+                   this, SLOT(onApplicationStateChanged()));
+}
 
 BrowserPlatformIntegration::~BrowserPlatformIntegration() {}
 
+void BrowserPlatformIntegration::onApplicationStateChanged() {
+  NotifyApplicationStateChanged();
+}
+
 bool BrowserPlatformIntegration::LaunchURLExternally(const GURL& url) {
   return QDesktopServices::openUrl(QUrl(QString::fromStdString(url.spec())));
 }
@@ -109,6 +116,15 @@
   return new LocationProvider();
 }
 
+oxide::BrowserPlatformIntegration::ApplicationState
+BrowserPlatformIntegration::GetApplicationState() {
+  if (qApp->applicationState() == Qt::ApplicationActive) {
+    return APPLICATION_STATE_ACTIVE;
+  } else {
+    return APPLICATION_STATE_INACTIVE;
+  }
+}
+
 QThread* GetIOQThread() {
   return g_io_thread.Get();
 }

=== modified file 'qt/core/browser/oxide_qt_browser_platform_integration.h'
--- qt/core/browser/oxide_qt_browser_platform_integration.h	2014-11-12 14:50:11 +0000
+++ qt/core/browser/oxide_qt_browser_platform_integration.h	2014-11-14 14:36:23 +0000
@@ -18,6 +18,7 @@
 #ifndef _OXIDE_QT_CORE_BROWSER_PLATFORM_INTEGRATION_H_
 #define _OXIDE_QT_CORE_BROWSER_PLATFORM_INTEGRATION_H_
 
+#include <QObject>
 #include <QtGlobal>
 
 #include "base/macros.h"
@@ -35,11 +36,16 @@
 class GLContextAdopted;
 
 class BrowserPlatformIntegration final
-    : public oxide::BrowserPlatformIntegration {
+    : public QObject, public oxide::BrowserPlatformIntegration {
+  Q_OBJECT
+
  public:
   BrowserPlatformIntegration(GLContextAdopted* gl_share_context);
   ~BrowserPlatformIntegration();
 
+ private Q_SLOTS:
+  void onApplicationStateChanged();
+
  private:
   bool LaunchURLExternally(const GURL& url) final;
   bool IsTouchSupported() final;
@@ -49,6 +55,7 @@
   scoped_ptr<oxide::MessagePump> CreateUIMessagePump() final;
   void BrowserThreadInit(content::BrowserThread::ID id) final;
   content::LocationProvider* CreateLocationProvider() final;
+  ApplicationState GetApplicationState() final;
 
   scoped_refptr<GLContextAdopted> gl_share_context_;
 

=== modified file 'qt/core/core.gyp'
--- qt/core/core.gyp	2014-11-12 14:50:11 +0000
+++ qt/core/core.gyp	2014-11-14 14:36:23 +0000
@@ -67,6 +67,7 @@
         '<(DEPTH)'
       ],
       'sources': [
+        '<(INTERMEDIATE_DIR)/moc_oxide_qt_browser_platform_integration.cc',
         '<(INTERMEDIATE_DIR)/moc_oxide_qt_web_view.cc',
         'api/internal/oxideqwebpreferences_p.cc',
         'app/oxide_qt_main.cc',
@@ -128,6 +129,11 @@
           'includes': [ 'moc.gypi' ]
         },
         {
+          'action_name': 'moc_oxide_qt_browser_platform_integration.cc',
+          'moc_input': 'browser/oxide_qt_browser_platform_integration.h',
+          'includes': [ 'moc.gypi' ]
+        },
+        {
           'action_name': 'moc_oxide_qt_web_view.cc',
           'moc_input': 'browser/oxide_qt_web_view.h',
           'includes': [ 'moc.gypi' ]

=== modified file 'shared/browser/oxide_browser_platform_integration.cc'
--- shared/browser/oxide_browser_platform_integration.cc	2014-11-08 00:28:36 +0000
+++ shared/browser/oxide_browser_platform_integration.cc	2014-11-14 14:36:23 +0000
@@ -19,6 +19,8 @@
 
 #include "base/logging.h"
 
+#include "oxide_browser_platform_integration_observer.h"
+
 namespace oxide {
 
 namespace {
@@ -67,4 +69,25 @@
   return NULL;
 }
 
+BrowserPlatformIntegration::ApplicationState
+BrowserPlatformIntegration::GetApplicationState() {
+  return APPLICATION_STATE_ACTIVE;
+}
+
+void BrowserPlatformIntegration::AddObserver(
+    BrowserPlatformIntegrationObserver* observer) {
+  observers_.AddObserver(observer);
+}
+
+void BrowserPlatformIntegration::RemoveObserver(
+    BrowserPlatformIntegrationObserver* observer) {
+  observers_.RemoveObserver(observer);
+}
+
+void BrowserPlatformIntegration::NotifyApplicationStateChanged() {
+  FOR_EACH_OBSERVER(BrowserPlatformIntegrationObserver,
+                    observers_,
+                    ApplicationStateChanged());
+}
+
 } // namespace oxide

=== modified file 'shared/browser/oxide_browser_platform_integration.h'
--- shared/browser/oxide_browser_platform_integration.h	2014-11-08 00:28:36 +0000
+++ shared/browser/oxide_browser_platform_integration.h	2014-11-14 14:36:23 +0000
@@ -20,6 +20,7 @@
 
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/observer_list.h"
 #include "content/public/browser/browser_thread.h"
 #include "third_party/WebKit/public/platform/WebScreenInfo.h"
 
@@ -31,6 +32,7 @@
 
 namespace oxide {
 
+class BrowserPlatformIntegrationObserver;
 class GLContextAdopted;
 class MessagePump;
 
@@ -38,6 +40,11 @@
  public:
   virtual ~BrowserPlatformIntegration();
 
+  enum ApplicationState {
+    APPLICATION_STATE_INACTIVE,
+    APPLICATION_STATE_ACTIVE
+  };
+
   // Can be called on any thread. Destruction of this class
   // must only happen once all Chromium threads have been shut down
   static BrowserPlatformIntegration* GetInstance();
@@ -62,10 +69,21 @@
   // Called on the geolocation thread
   virtual content::LocationProvider* CreateLocationProvider();
 
+  virtual ApplicationState GetApplicationState();
+
  protected:
   BrowserPlatformIntegration();
 
+  void NotifyApplicationStateChanged();
+
  private:
+  friend class BrowserPlatformIntegrationObserver;
+
+  void AddObserver(BrowserPlatformIntegrationObserver* observer);
+  void RemoveObserver(BrowserPlatformIntegrationObserver* observer);
+
+  ObserverList<BrowserPlatformIntegrationObserver> observers_;
+
   DISALLOW_COPY_AND_ASSIGN(BrowserPlatformIntegration);
 };
 

=== added file 'shared/browser/oxide_browser_platform_integration_observer.cc'
--- shared/browser/oxide_browser_platform_integration_observer.cc	1970-01-01 00:00:00 +0000
+++ shared/browser/oxide_browser_platform_integration_observer.cc	2014-11-14 14:36:23 +0000
@@ -0,0 +1,30 @@
+// vim:expandtab:shiftwidth=2:tabstop=2:
+// Copyright (C) 2014 Canonical Ltd.
+
+// This library is free software; you can redistribute it and/or
+// modify it under the terms of the GNU Lesser General Public
+// License as published by the Free Software Foundation; either
+// version 2.1 of the License, or (at your option) any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+// Lesser General Public License for more details.
+
+// You should have received a copy of the GNU Lesser General Public
+// License along with this library; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+
+#include "oxide_browser_platform_integration_observer.h"
+
+namespace oxide {
+
+BrowserPlatformIntegrationObserver::BrowserPlatformIntegrationObserver() {
+  BrowserPlatformIntegration::GetInstance()->AddObserver(this);
+}
+
+BrowserPlatformIntegrationObserver::~BrowserPlatformIntegrationObserver() {
+  BrowserPlatformIntegration::GetInstance()->RemoveObserver(this);
+}
+
+} // namespace oxide

=== added file 'shared/browser/oxide_browser_platform_integration_observer.h'
--- shared/browser/oxide_browser_platform_integration_observer.h	1970-01-01 00:00:00 +0000
+++ shared/browser/oxide_browser_platform_integration_observer.h	2014-11-14 14:36:23 +0000
@@ -0,0 +1,37 @@
+// vim:expandtab:shiftwidth=2:tabstop=2:
+// Copyright (C) 2014 Canonical Ltd.
+
+// This library is free software; you can redistribute it and/or
+// modify it under the terms of the GNU Lesser General Public
+// License as published by the Free Software Foundation; either
+// version 2.1 of the License, or (at your option) any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+// Lesser General Public License for more details.
+
+// You should have received a copy of the GNU Lesser General Public
+// License along with this library; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+
+#ifndef _OXIDE_SHARED_BROWSER_PLATFORM_INTEGRATION_OBSERVER_H_
+#define _OXIDE_SHARED_BROWSER_PLATFORM_INTEGRATION_OBSERVER_H_
+
+#include "oxide_browser_platform_integration.h"
+
+namespace oxide {
+
+class BrowserPlatformIntegrationObserver {
+ public:
+  virtual ~BrowserPlatformIntegrationObserver();
+
+  virtual void ApplicationStateChanged() {}
+
+ protected:
+  BrowserPlatformIntegrationObserver();
+};
+
+} // namespace oxide
+
+#endif // _OXIDE_SHARED_BROWSER_PLATFORM_INTEGRATION_OBSERVER_H_

=== modified file 'shared/browser/oxide_power_save_blocker.cc'
--- shared/browser/oxide_power_save_blocker.cc	2014-10-27 16:32:42 +0000
+++ shared/browser/oxide_power_save_blocker.cc	2014-11-14 14:36:23 +0000
@@ -31,6 +31,7 @@
 
 #include "shared/port/content/browser/power_save_blocker_oxide.h"
 
+#include "oxide_browser_platform_integration_observer.h"
 #include "oxide_form_factor.h"
 
 namespace oxide {
@@ -43,7 +44,8 @@
 
 }
 
-class PowerSaveBlocker : public content::PowerSaveBlockerOxideDelegate {
+class PowerSaveBlocker : public content::PowerSaveBlockerOxideDelegate,
+                         public BrowserPlatformIntegrationObserver {
  public:
   PowerSaveBlocker();
 
@@ -56,16 +58,22 @@
   void ApplyBlock();
   void RemoveBlock();
 
+  // BrowserPlatformIntegrationObserver implementation
+  void ApplicationStateChanged() final;
+
   oxide::FormFactor form_factor_;
   scoped_refptr<dbus::Bus> bus_;
   int cookie_;
 };
 
 void PowerSaveBlocker::Init() {
-  content::BrowserThread::PostTask(
-      content::BrowserThread::FILE,
-      FROM_HERE,
-      base::Bind(&PowerSaveBlocker::ApplyBlock, this));
+  if (BrowserPlatformIntegration::GetInstance()->GetApplicationState() ==
+      BrowserPlatformIntegration::APPLICATION_STATE_ACTIVE) {
+    content::BrowserThread::PostTask(
+        content::BrowserThread::FILE,
+        FROM_HERE,
+        base::Bind(&PowerSaveBlocker::ApplyBlock, this));
+  }
 }
 
 void PowerSaveBlocker::CleanUp() {
@@ -116,9 +124,8 @@
 
   if (form_factor_ == oxide::FORM_FACTOR_PHONE ||
       form_factor_ == oxide::FORM_FACTOR_TABLET) {
-    DCHECK(bus_.get());
-
     if (cookie_ != 0) {
+      DCHECK(bus_.get());
       scoped_refptr<dbus::ObjectProxy> object_proxy = bus_->GetObjectProxy(
           kUnityScreenServiceName,
           dbus::ObjectPath(kUnityScreenPath));
@@ -132,16 +139,30 @@
       cookie_ = 0;
     }
 
-    bus_->ShutdownAndBlock();
-    bus_ = NULL;
+    if (bus_.get()) {
+      bus_->ShutdownAndBlock();
+      bus_ = NULL;
+    }
   } else {
     NOTIMPLEMENTED();
   }
 }
 
+void PowerSaveBlocker::ApplicationStateChanged() {
+  BrowserPlatformIntegration::ApplicationState state =
+      BrowserPlatformIntegration::GetInstance()->GetApplicationState();
+  if ((state == BrowserPlatformIntegration::APPLICATION_STATE_INACTIVE) &&
+      (cookie_ != 0)) {
+    CleanUp();
+  } else if ((state == BrowserPlatformIntegration::APPLICATION_STATE_ACTIVE) &&
+      (cookie_ == 0)) {
+    Init();
+  }
+}
+
 PowerSaveBlocker::PowerSaveBlocker()
-    : form_factor_(oxide::GetFormFactorHint()),
-      cookie_(0) {}
+    : form_factor_(oxide::GetFormFactorHint())
+    , cookie_(0) {}
 
 content::PowerSaveBlockerOxideDelegate* CreatePowerSaveBlocker(
     content::PowerSaveBlocker::PowerSaveBlockerType type,

=== modified file 'shared/shared.gyp'
--- shared/shared.gyp	2014-11-08 01:06:30 +0000
+++ shared/shared.gyp	2014-11-14 14:36:23 +0000
@@ -253,6 +253,8 @@
         'browser/oxide_browser_main_parts.h',
         'browser/oxide_browser_platform_integration.cc',
         'browser/oxide_browser_platform_integration.h',
+        'browser/oxide_browser_platform_integration_observer.cc',
+        'browser/oxide_browser_platform_integration_observer.h',
         'browser/oxide_browser_process_main.cc',
         'browser/oxide_browser_process_main.h',
         'browser/oxide_content_browser_client.cc',