Browse Source

refactor: ginify powerMonitor (#22751)

Jeremy Apthorp 5 years ago
parent
commit
07cd70a37e

+ 0 - 16
docs/api/power-monitor.md

@@ -4,22 +4,6 @@
 
 Process: [Main](../glossary.md#main-process)
 
-
-This module cannot be used until the `ready` event of the `app`
-module is emitted.
-
-For example:
-
-```javascript
-const { app, powerMonitor } = require('electron')
-
-app.whenReady().then(() => {
-  powerMonitor.on('suspend', () => {
-    console.log('The system is going to sleep')
-  })
-})
-```
-
 ## Events
 
 The `powerMonitor` module emits the following events:

+ 1 - 1
filenames.gni

@@ -187,7 +187,6 @@ filenames = {
     "shell/browser/javascript_environment.h",
     "shell/browser/lib/bluetooth_chooser.cc",
     "shell/browser/lib/bluetooth_chooser.h",
-    "shell/browser/lib/power_observer.h",
     "shell/browser/lib/power_observer_linux.cc",
     "shell/browser/lib/power_observer_linux.h",
     "shell/browser/linux/unity_service.cc",
@@ -526,6 +525,7 @@ filenames = {
     "shell/common/gin_helper/object_template_builder.h",
     "shell/common/gin_helper/persistent_dictionary.cc",
     "shell/common/gin_helper/persistent_dictionary.h",
+    "shell/common/gin_helper/pinnable.h",
     "shell/common/gin_helper/promise.cc",
     "shell/common/gin_helper/promise.h",
     "shell/common/gin_helper/trackable_object.cc",

+ 39 - 31
lib/browser/api/power-monitor.ts

@@ -1,38 +1,46 @@
-'use strict';
+import { EventEmitter } from 'events';
+import { app } from 'electron';
 
-import { createLazyInstance } from '../utils';
+const { createPowerMonitor, getSystemIdleState, getSystemIdleTime } = process.electronBinding('power_monitor');
 
-const { EventEmitter } = require('events');
-const { createPowerMonitor, PowerMonitor } = process.electronBinding('power_monitor');
+class PowerMonitor extends EventEmitter {
+  constructor () {
+    super();
+    // Don't start the event source until both a) the app is ready and b)
+    // there's a listener registered for a powerMonitor event.
+    this.once('newListener', () => {
+      app.whenReady().then(() => {
+        const pm = createPowerMonitor();
+        pm.emit = this.emit.bind(this);
 
-// PowerMonitor is an EventEmitter.
-Object.setPrototypeOf(PowerMonitor.prototype, EventEmitter.prototype);
+        if (process.platform === 'linux') {
+          // On Linux, we inhibit shutdown in order to give the app a chance to
+          // decide whether or not it wants to prevent the shutdown. We don't
+          // inhibit the shutdown event unless there's a listener for it. This
+          // keeps the C++ code informed about whether there are any listeners.
+          pm.setListeningForShutdown(this.listenerCount('shutdown') > 0);
+          this.on('newListener', (event) => {
+            if (event === 'shutdown') {
+              pm.setListeningForShutdown(this.listenerCount('shutdown') + 1 > 0);
+            }
+          });
+          this.on('removeListener', (event) => {
+            if (event === 'shutdown') {
+              pm.setListeningForShutdown(this.listenerCount('shutdown') > 0);
+            }
+          });
+        }
+      });
+    });
+  }
 
-const powerMonitor = createLazyInstance(createPowerMonitor, PowerMonitor, true);
+  getSystemIdleState (idleThreshold: number) {
+    return getSystemIdleState(idleThreshold);
+  }
 
-if (process.platform === 'linux') {
-  // In order to delay system shutdown when e.preventDefault() is invoked
-  // on a powerMonitor 'shutdown' event, we need an org.freedesktop.login1
-  // shutdown delay lock. For more details see the "Taking Delay Locks"
-  // section of https://www.freedesktop.org/wiki/Software/systemd/inhibit/
-  //
-  // So here we watch for 'shutdown' listeners to be added or removed and
-  // set or unset our shutdown delay lock accordingly.
-  const { app } = require('electron');
-  app.whenReady().then(() => {
-    powerMonitor.on('newListener', (event: string) => {
-      // whenever the listener count is incremented to one...
-      if (event === 'shutdown' && powerMonitor.listenerCount('shutdown') === 0) {
-        powerMonitor.blockShutdown();
-      }
-    });
-    powerMonitor.on('removeListener', (event: string) => {
-      // whenever the listener count is decremented to zero...
-      if (event === 'shutdown' && powerMonitor.listenerCount('shutdown') === 0) {
-        powerMonitor.unblockShutdown();
-      }
-    });
-  });
+  getSystemIdleTime () {
+    return getSystemIdleTime();
+  }
 }
 
-module.exports = powerMonitor;
+module.exports = new PowerMonitor();

+ 51 - 51
shell/browser/api/electron_api_power_monitor.cc

@@ -6,10 +6,10 @@
 
 #include "base/power_monitor/power_monitor.h"
 #include "base/power_monitor/power_monitor_device_source.h"
-#include "gin/dictionary.h"
 #include "gin/handle.h"
 #include "shell/browser/browser.h"
 #include "shell/common/gin_converters/callback_converter.h"
+#include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/gin_helper/object_template_builder.h"
 #include "shell/common/node_includes.h"
 
@@ -39,16 +39,16 @@ namespace electron {
 
 namespace api {
 
+gin::WrapperInfo PowerMonitor::kWrapperInfo = {gin::kEmbedderNativeGin};
+
 PowerMonitor::PowerMonitor(v8::Isolate* isolate) {
-#if defined(OS_LINUX)
-  SetShutdownHandler(base::BindRepeating(&PowerMonitor::ShouldShutdown,
-                                         base::Unretained(this)));
-#elif defined(OS_MACOSX)
+#if defined(OS_MACOSX)
   Browser::Get()->SetShutdownHandler(base::BindRepeating(
       &PowerMonitor::ShouldShutdown, base::Unretained(this)));
 #endif
+
   base::PowerMonitor::AddObserver(this);
-  Init(isolate);
+
 #if defined(OS_MACOSX) || defined(OS_WIN)
   InitPlatformSpecificMonitors();
 #endif
@@ -62,16 +62,6 @@ bool PowerMonitor::ShouldShutdown() {
   return !Emit("shutdown");
 }
 
-#if defined(OS_LINUX)
-void PowerMonitor::BlockShutdown() {
-  PowerObserverLinux::BlockShutdown();
-}
-
-void PowerMonitor::UnblockShutdown() {
-  PowerObserverLinux::UnblockShutdown();
-}
-#endif
-
 void PowerMonitor::OnPowerStateChange(bool on_battery_power) {
   if (on_battery_power)
     Emit("on-battery");
@@ -87,46 +77,41 @@ void PowerMonitor::OnResume() {
   Emit("resume");
 }
 
-ui::IdleState PowerMonitor::GetSystemIdleState(v8::Isolate* isolate,
-                                               int idle_threshold) {
-  if (idle_threshold > 0) {
-    return ui::CalculateIdleState(idle_threshold);
+#if defined(OS_LINUX)
+void PowerMonitor::SetListeningForShutdown(bool is_listening) {
+  if (is_listening) {
+    // unretained is OK because we own power_observer_linux_
+    power_observer_linux_.SetShutdownHandler(base::BindRepeating(
+        &PowerMonitor::ShouldShutdown, base::Unretained(this)));
   } else {
-    isolate->ThrowException(v8::Exception::TypeError(gin::StringToV8(
-        isolate, "Invalid idle threshold, must be greater than 0")));
-    return ui::IDLE_STATE_UNKNOWN;
+    power_observer_linux_.SetShutdownHandler(base::RepeatingCallback<bool()>());
   }
 }
-
-int PowerMonitor::GetSystemIdleTime() {
-  return ui::CalculateIdleTime();
-}
+#endif
 
 // static
 v8::Local<v8::Value> PowerMonitor::Create(v8::Isolate* isolate) {
-  if (!Browser::Get()->is_ready()) {
-    isolate->ThrowException(v8::Exception::Error(
-        gin::StringToV8(isolate,
-                        "The 'powerMonitor' module can't be used before the "
-                        "app 'ready' event")));
-    return v8::Null(isolate);
-  }
-
-  return gin::CreateHandle(isolate, new PowerMonitor(isolate)).ToV8();
+  CHECK(Browser::Get()->is_ready());
+  auto* pm = new PowerMonitor(isolate);
+  auto handle = gin::CreateHandle(isolate, pm).ToV8();
+  pm->Pin(isolate);
+  return handle;
 }
 
-// static
-void PowerMonitor::BuildPrototype(v8::Isolate* isolate,
-                                  v8::Local<v8::FunctionTemplate> prototype) {
-  prototype->SetClassName(gin::StringToV8(isolate, "PowerMonitor"));
-  gin_helper::Destroyable::MakeDestroyable(isolate, prototype);
-  gin_helper::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate())
+gin::ObjectTemplateBuilder PowerMonitor::GetObjectTemplateBuilder(
+    v8::Isolate* isolate) {
+  auto builder =
+      gin_helper::EventEmitterMixin<PowerMonitor>::GetObjectTemplateBuilder(
+          isolate);
 #if defined(OS_LINUX)
-      .SetMethod("blockShutdown", &PowerMonitor::BlockShutdown)
-      .SetMethod("unblockShutdown", &PowerMonitor::UnblockShutdown)
+  builder.SetMethod("setListeningForShutdown",
+                    &PowerMonitor::SetListeningForShutdown);
 #endif
-      .SetMethod("getSystemIdleState", &PowerMonitor::GetSystemIdleState)
-      .SetMethod("getSystemIdleTime", &PowerMonitor::GetSystemIdleTime);
+  return builder;
+}
+
+const char* PowerMonitor::GetTypeName() {
+  return "PowerMonitor";
 }
 
 }  // namespace api
@@ -137,16 +122,31 @@ namespace {
 
 using electron::api::PowerMonitor;
 
+ui::IdleState GetSystemIdleState(v8::Isolate* isolate, int idle_threshold) {
+  if (idle_threshold > 0) {
+    return ui::CalculateIdleState(idle_threshold);
+  } else {
+    isolate->ThrowException(v8::Exception::TypeError(gin::StringToV8(
+        isolate, "Invalid idle threshold, must be greater than 0")));
+    return ui::IDLE_STATE_UNKNOWN;
+  }
+}
+
+int GetSystemIdleTime() {
+  return ui::CalculateIdleTime();
+}
+
 void Initialize(v8::Local<v8::Object> exports,
                 v8::Local<v8::Value> unused,
                 v8::Local<v8::Context> context,
                 void* priv) {
   v8::Isolate* isolate = context->GetIsolate();
-  gin::Dictionary dict(isolate, exports);
-  dict.Set("createPowerMonitor", base::BindRepeating(&PowerMonitor::Create));
-  dict.Set("PowerMonitor", PowerMonitor::GetConstructor(isolate)
-                               ->GetFunction(context)
-                               .ToLocalChecked());
+  gin_helper::Dictionary dict(isolate, exports);
+  dict.SetMethod("createPowerMonitor",
+                 base::BindRepeating(&PowerMonitor::Create));
+  dict.SetMethod("getSystemIdleState",
+                 base::BindRepeating(&GetSystemIdleState));
+  dict.SetMethod("getSystemIdleTime", base::BindRepeating(&GetSystemIdleTime));
 }
 
 }  // namespace

+ 28 - 18
shell/browser/api/electron_api_power_monitor.h

@@ -5,36 +5,44 @@
 #ifndef SHELL_BROWSER_API_ELECTRON_API_POWER_MONITOR_H_
 #define SHELL_BROWSER_API_ELECTRON_API_POWER_MONITOR_H_
 
-#include "base/compiler_specific.h"
-#include "shell/browser/lib/power_observer.h"
-#include "shell/common/gin_helper/trackable_object.h"
+#include "base/power_monitor/power_observer.h"
+#include "gin/wrappable.h"
+#include "shell/browser/event_emitter_mixin.h"
+#include "shell/common/gin_helper/pinnable.h"
 #include "ui/base/idle/idle.h"
 
+#if defined(OS_LINUX)
+#include "shell/browser/lib/power_observer_linux.h"
+#endif
+
 namespace electron {
 
 namespace api {
 
-class PowerMonitor : public gin_helper::TrackableObject<PowerMonitor>,
-                     public PowerObserver {
+class PowerMonitor : public gin::Wrappable<PowerMonitor>,
+                     public gin_helper::EventEmitterMixin<PowerMonitor>,
+                     public gin_helper::Pinnable<PowerMonitor>,
+                     public base::PowerObserver {
  public:
   static v8::Local<v8::Value> Create(v8::Isolate* isolate);
 
-  static void BuildPrototype(v8::Isolate* isolate,
-                             v8::Local<v8::FunctionTemplate> prototype);
+  // gin::Wrappable
+  static gin::WrapperInfo kWrapperInfo;
+  gin::ObjectTemplateBuilder GetObjectTemplateBuilder(
+      v8::Isolate* isolate) override;
+  const char* GetTypeName() override;
 
- protected:
+ private:
   explicit PowerMonitor(v8::Isolate* isolate);
   ~PowerMonitor() override;
 
-  // Called by native calles.
-  bool ShouldShutdown();
-
 #if defined(OS_LINUX)
-  // Private JS APIs.
-  void BlockShutdown();
-  void UnblockShutdown();
+  void SetListeningForShutdown(bool);
 #endif
 
+  // Called by native calles.
+  bool ShouldShutdown();
+
 #if defined(OS_MACOSX) || defined(OS_WIN)
   void InitPlatformSpecificMonitors();
 #endif
@@ -44,10 +52,6 @@ class PowerMonitor : public gin_helper::TrackableObject<PowerMonitor>,
   void OnSuspend() override;
   void OnResume() override;
 
- private:
-  ui::IdleState GetSystemIdleState(v8::Isolate* isolate, int idle_threshold);
-  int GetSystemIdleTime();
-
 #if defined(OS_WIN)
   // Static callback invoked when a message comes in to our messaging window.
   static LRESULT CALLBACK WndProcStatic(HWND hwnd,
@@ -70,6 +74,12 @@ class PowerMonitor : public gin_helper::TrackableObject<PowerMonitor>,
   HWND window_;
 #endif
 
+#if defined(OS_LINUX)
+  PowerObserverLinux power_observer_linux_{this};
+#endif
+
+  v8::Global<v8::Value> pinned_;
+
   DISALLOW_COPY_AND_ASSIGN(PowerMonitor);
 };
 

+ 0 - 26
shell/browser/lib/power_observer.h

@@ -1,26 +0,0 @@
-// Copyright (c) 2017 GitHub, Inc.
-// Use of this source code is governed by the MIT license that can be
-// found in the LICENSE file.
-
-#ifndef SHELL_BROWSER_LIB_POWER_OBSERVER_H_
-#define SHELL_BROWSER_LIB_POWER_OBSERVER_H_
-
-#include "base/macros.h"
-
-#if defined(OS_LINUX)
-#include "shell/browser/lib/power_observer_linux.h"
-#else
-#include "base/power_monitor/power_observer.h"
-#endif  // defined(OS_LINUX)
-
-namespace electron {
-
-#if defined(OS_LINUX)
-typedef PowerObserverLinux PowerObserver;
-#else
-typedef base::PowerObserver PowerObserver;
-#endif  // defined(OS_LINUX)
-
-}  // namespace electron
-
-#endif  // SHELL_BROWSER_LIB_POWER_OBSERVER_H_

+ 22 - 12
shell/browser/lib/power_observer_linux.cc

@@ -9,6 +9,8 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/command_line.h"
+#include "base/files/file_path.h"
 #include "device/bluetooth/dbus/bluez_dbus_thread_manager.h"
 
 namespace {
@@ -17,22 +19,19 @@ const char kLogindServiceName[] = "org.freedesktop.login1";
 const char kLogindObjectPath[] = "/org/freedesktop/login1";
 const char kLogindManagerInterface[] = "org.freedesktop.login1.Manager";
 
-std::string get_executable_basename() {
-  char buf[4096];
-  size_t buf_size = sizeof(buf);
-  std::string rv("electron");
-  if (!uv_exepath(buf, &buf_size)) {
-    rv = strrchr(static_cast<const char*>(buf), '/') + 1;
-  }
-  return rv;
+base::FilePath::StringType GetExecutableBaseName() {
+  return base::CommandLine::ForCurrentProcess()
+      ->GetProgram()
+      .BaseName()
+      .value();
 }
 
 }  // namespace
 
 namespace electron {
 
-PowerObserverLinux::PowerObserverLinux()
-    : lock_owner_name_(get_executable_basename()), weak_ptr_factory_(this) {
+PowerObserverLinux::PowerObserverLinux(base::PowerObserver* observer)
+    : observer_(observer), lock_owner_name_(GetExecutableBaseName()) {
   auto* bus = bluez::BluezDBusThreadManager::Get()->GetSystemBus();
   if (!bus) {
     LOG(WARNING) << "Failed to get system bus connection";
@@ -114,6 +113,15 @@ void PowerObserverLinux::UnblockShutdown() {
 }
 
 void PowerObserverLinux::SetShutdownHandler(base::Callback<bool()> handler) {
+  // In order to delay system shutdown when e.preventDefault() is invoked
+  // on a powerMonitor 'shutdown' event, we need an org.freedesktop.login1
+  // shutdown delay lock. For more details see the "Taking Delay Locks"
+  // section of https://www.freedesktop.org/wiki/Software/systemd/inhibit/
+  if (handler && !should_shutdown_) {
+    BlockShutdown();
+  } else if (!handler && should_shutdown_) {
+    UnblockShutdown();
+  }
   should_shutdown_ = std::move(handler);
 }
 
@@ -133,11 +141,13 @@ void PowerObserverLinux::OnPrepareForSleep(dbus::Signal* signal) {
     return;
   }
   if (suspending) {
-    OnSuspend();
+    observer_->OnSuspend();
+
     UnblockSleep();
   } else {
     BlockSleep();
-    OnResume();
+
+    observer_->OnResume();
   }
 }
 

+ 10 - 9
shell/browser/lib/power_observer_linux.h

@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/power_monitor/power_observer.h"
@@ -16,20 +17,19 @@
 
 namespace electron {
 
-class PowerObserverLinux : public base::PowerObserver {
+class PowerObserverLinux {
  public:
-  PowerObserverLinux();
-  ~PowerObserverLinux() override;
+  explicit PowerObserverLinux(base::PowerObserver* observer);
+  ~PowerObserverLinux();
 
- protected:
+  void SetShutdownHandler(base::Callback<bool()> should_shutdown);
+
+ private:
   void BlockSleep();
   void UnblockSleep();
   void BlockShutdown();
   void UnblockShutdown();
 
-  void SetShutdownHandler(base::Callback<bool()> should_shutdown);
-
- private:
   void OnLoginServiceAvailable(bool available);
   void OnInhibitResponse(base::ScopedFD* scoped_fd, dbus::Response* response);
   void OnPrepareForSleep(dbus::Signal* signal);
@@ -38,13 +38,14 @@ class PowerObserverLinux : public base::PowerObserver {
                          const std::string& signal,
                          bool success);
 
-  base::Callback<bool()> should_shutdown_;
+  base::RepeatingCallback<bool()> should_shutdown_;
+  base::PowerObserver* observer_;
 
   scoped_refptr<dbus::ObjectProxy> logind_;
   std::string lock_owner_name_;
   base::ScopedFD sleep_lock_;
   base::ScopedFD shutdown_lock_;
-  base::WeakPtrFactory<PowerObserverLinux> weak_ptr_factory_;
+  base::WeakPtrFactory<PowerObserverLinux> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(PowerObserverLinux);
 };

+ 34 - 0
shell/common/gin_helper/pinnable.h

@@ -0,0 +1,34 @@
+// Copyright (c) 2020 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef SHELL_COMMON_GIN_HELPER_PINNABLE_H_
+#define SHELL_COMMON_GIN_HELPER_PINNABLE_H_
+
+#include "v8/include/v8.h"
+
+namespace gin_helper {
+
+template <typename T>
+class Pinnable {
+ protected:
+  // Prevent the object from being garbage collected until Unpin() is called.
+  void Pin(v8::Isolate* isolate) {
+    v8::Locker locker(isolate);
+    v8::HandleScope scope(isolate);
+    v8::Local<v8::Value> wrapper;
+    if (static_cast<T*>(this)->GetWrapper(isolate).ToLocal(&wrapper)) {
+      pinned_.Reset(isolate, wrapper);
+    }
+  }
+
+  // Allow the object to be garbage collected.
+  void Unpin() { pinned_.Reset(); }
+
+ private:
+  v8::Global<v8::Value> pinned_;
+};
+
+}  // namespace gin_helper
+
+#endif  // SHELL_COMMON_GIN_HELPER_PINNABLE_H_

+ 30 - 11
spec-main/api-power-monitor-spec.ts

@@ -44,17 +44,36 @@ describe('powerMonitor', () => {
       dbusMockPowerMonitor = require('electron').powerMonitor;
     });
 
-    it('should call Inhibit to delay suspend', async () => {
-      const calls = await getCalls();
-      expect(calls).to.be.an('array').that.has.lengthOf(1);
-      expect(calls[0].slice(1)).to.deep.equal([
-        'Inhibit', [
-          [[{ type: 's', child: [] }], ['sleep']],
-          [[{ type: 's', child: [] }], ['electron']],
-          [[{ type: 's', child: [] }], ['Application cleanup before suspend']],
-          [[{ type: 's', child: [] }], ['delay']]
-        ]
-      ]);
+    it('should call Inhibit to delay suspend once a listener is added', async () => {
+      // No calls to dbus until a listener is added
+      {
+        const calls = await getCalls();
+        expect(calls).to.be.an('array').that.has.lengthOf(0);
+      }
+      // Add a dummy listener to engage the monitors
+      dbusMockPowerMonitor.on('dummy-event', () => {});
+      try {
+        let retriesRemaining = 3;
+        // There doesn't seem to be a way to get a notification when a call
+        // happens, so poll `getCalls` a few times to reduce flake.
+        let calls: any[] = [];
+        while (retriesRemaining-- > 0) {
+          calls = await getCalls();
+          if (calls.length > 0) break;
+          await new Promise(resolve => setTimeout(resolve, 1000));
+        }
+        expect(calls).to.be.an('array').that.has.lengthOf(1);
+        expect(calls[0].slice(1)).to.deep.equal([
+          'Inhibit', [
+            [[{ type: 's', child: [] }], ['sleep']],
+            [[{ type: 's', child: [] }], ['electron']],
+            [[{ type: 's', child: [] }], ['Application cleanup before suspend']],
+            [[{ type: 's', child: [] }], ['delay']]
+          ]
+        ]);
+      } finally {
+        dbusMockPowerMonitor.removeAllListeners('dummy-event');
+      }
     });
 
     describe('when PrepareForSleep(true) signal is sent by logind', () => {