Browse Source

refactor: ginify session.netLog (#22732)

Jeremy Apthorp 5 years ago
parent
commit
c4a7eade28

+ 2 - 6
docs/api/net-log.md

@@ -40,7 +40,7 @@ Starts recording network events to `path`.
 
 ### `netLog.stopLogging()`
 
-Returns `Promise<String>` - resolves with a file path to which network logs were recorded.
+Returns `Promise<void>` - resolves when the net log has been flushed to disk.
 
 Stops recording network events. If not called, net logging will automatically end when app quits.
 
@@ -48,8 +48,4 @@ Stops recording network events. If not called, net logging will automatically en
 
 ### `netLog.currentlyLogging` _Readonly_
 
-A `Boolean` property that indicates whether network logs are recorded.
-
-### `netLog.currentlyLoggingPath` _Readonly_ _Deprecated_
-
-A `String` property that returns the path to the current log file.
+A `Boolean` property that indicates whether network logs are currently being recorded.

+ 1 - 30
lib/browser/api/session.js

@@ -2,7 +2,7 @@
 
 const { EventEmitter } = require('events')
 const { app, deprecate } = require('electron')
-const { fromPartition, Session, Cookies, NetLog, Protocol, ServiceWorkerContext } = process.electronBinding('session')
+const { fromPartition, Session, Cookies, Protocol, ServiceWorkerContext } = process.electronBinding('session')
 
 // Public API.
 Object.defineProperties(exports, {
@@ -23,32 +23,3 @@ Object.setPrototypeOf(Session.prototype, EventEmitter.prototype)
 Session.prototype._init = function () {
   app.emit('session-created', this)
 }
-
-const _originalStartLogging = NetLog.prototype.startLogging
-NetLog.prototype.startLogging = function (path, ...args) {
-  this._currentlyLoggingPath = path
-  try {
-    return _originalStartLogging.call(this, path, ...args)
-  } catch (e) {
-    this._currentlyLoggingPath = null
-    throw e
-  }
-}
-
-const _originalStopLogging = NetLog.prototype.stopLogging
-NetLog.prototype.stopLogging = function () {
-  const logPath = this._currentlyLoggingPath
-  this._currentlyLoggingPath = null
-  return _originalStopLogging.call(this).then(() => logPath)
-}
-
-const currentlyLoggingPathDeprecated = deprecate.warnOnce('currentlyLoggingPath')
-Object.defineProperties(NetLog.prototype, {
-  currentlyLoggingPath: {
-    enumerable: true,
-    get () {
-      currentlyLoggingPathDeprecated()
-      return this._currentlyLoggingPath == null ? '' : this._currentlyLoggingPath
-    }
-  }
-})

+ 23 - 20
shell/browser/api/electron_api_net_log.cc

@@ -12,11 +12,11 @@
 #include "components/net_log/chrome_net_log.h"
 #include "content/public/browser/storage_partition.h"
 #include "electron/electron_version.h"
+#include "gin/object_template_builder.h"
 #include "shell/browser/electron_browser_context.h"
 #include "shell/browser/net/system_network_context_manager.h"
 #include "shell/common/gin_converters/file_path_converter.h"
 #include "shell/common/gin_helper/dictionary.h"
-#include "shell/common/gin_helper/object_template_builder.h"
 #include "shell/common/node_includes.h"
 
 namespace gin {
@@ -76,18 +76,19 @@ void ResolvePromiseWithNetError(gin_helper::Promise<void> promise,
 
 namespace api {
 
+gin::WrapperInfo NetLog::kWrapperInfo = {gin::kEmbedderNativeGin};
+
 NetLog::NetLog(v8::Isolate* isolate, ElectronBrowserContext* browser_context)
     : browser_context_(browser_context), weak_ptr_factory_(this) {
-  Init(isolate);
   file_task_runner_ = CreateFileTaskRunner();
 }
 
 NetLog::~NetLog() = default;
 
 v8::Local<v8::Promise> NetLog::StartLogging(base::FilePath log_path,
-                                            gin_helper::Arguments* args) {
+                                            gin::Arguments* args) {
   if (log_path.empty()) {
-    args->ThrowError("The first parameter must be a valid string");
+    args->ThrowTypeError("The first parameter must be a valid string");
     return v8::Local<v8::Promise>();
   }
 
@@ -100,7 +101,7 @@ v8::Local<v8::Promise> NetLog::StartLogging(base::FilePath log_path,
     if (dict.Get("captureMode", &capture_mode_v8)) {
       if (!gin::ConvertFromV8(args->isolate(), capture_mode_v8,
                               &capture_mode)) {
-        args->ThrowError("Invalid value for captureMode");
+        args->ThrowTypeError("Invalid value for captureMode");
         return v8::Local<v8::Promise>();
       }
     }
@@ -108,19 +109,19 @@ v8::Local<v8::Promise> NetLog::StartLogging(base::FilePath log_path,
     if (dict.Get("maxFileSize", &max_file_size_v8)) {
       if (!gin::ConvertFromV8(args->isolate(), max_file_size_v8,
                               &max_file_size)) {
-        args->ThrowError("Invalid value for maxFileSize");
+        args->ThrowTypeError("Invalid value for maxFileSize");
         return v8::Local<v8::Promise>();
       }
     }
   }
 
   if (net_log_exporter_) {
-    args->ThrowError("There is already a net log running");
+    args->ThrowTypeError("There is already a net log running");
     return v8::Local<v8::Promise>();
   }
 
   pending_start_promise_ =
-      base::make_optional<gin_helper::Promise<void>>(isolate());
+      base::make_optional<gin_helper::Promise<void>>(args->isolate());
   v8::Local<v8::Promise> handle = pending_start_promise_->GetHandle();
 
   auto command_line_string =
@@ -189,8 +190,8 @@ bool NetLog::IsCurrentlyLogging() const {
   return !!net_log_exporter_;
 }
 
-v8::Local<v8::Promise> NetLog::StopLogging(gin_helper::Arguments* args) {
-  gin_helper::Promise<void> promise(isolate());
+v8::Local<v8::Promise> NetLog::StopLogging(gin::Arguments* args) {
+  gin_helper::Promise<void> promise(args->isolate());
   v8::Local<v8::Promise> handle = promise.GetHandle();
 
   if (net_log_exporter_) {
@@ -212,22 +213,24 @@ v8::Local<v8::Promise> NetLog::StopLogging(gin_helper::Arguments* args) {
   return handle;
 }
 
+gin::ObjectTemplateBuilder NetLog::GetObjectTemplateBuilder(
+    v8::Isolate* isolate) {
+  return gin::Wrappable<NetLog>::GetObjectTemplateBuilder(isolate)
+      .SetProperty("currentlyLogging", &NetLog::IsCurrentlyLogging)
+      .SetMethod("startLogging", &NetLog::StartLogging)
+      .SetMethod("stopLogging", &NetLog::StopLogging);
+}
+
+const char* NetLog::GetTypeName() {
+  return "NetLog";
+}
+
 // static
 gin::Handle<NetLog> NetLog::Create(v8::Isolate* isolate,
                                    ElectronBrowserContext* browser_context) {
   return gin::CreateHandle(isolate, new NetLog(isolate, browser_context));
 }
 
-// static
-void NetLog::BuildPrototype(v8::Isolate* isolate,
-                            v8::Local<v8::FunctionTemplate> prototype) {
-  prototype->SetClassName(gin::StringToV8(isolate, "NetLog"));
-  gin_helper::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate())
-      .SetProperty("currentlyLogging", &NetLog::IsCurrentlyLogging)
-      .SetMethod("startLogging", &NetLog::StartLogging)
-      .SetMethod("stopLogging", &NetLog::StopLogging);
-}
-
 }  // namespace api
 
 }  // namespace electron

+ 14 - 7
shell/browser/api/electron_api_net_log.h

@@ -13,9 +13,13 @@
 #include "base/optional.h"
 #include "base/values.h"
 #include "gin/handle.h"
+#include "gin/wrappable.h"
 #include "services/network/public/mojom/net_log.mojom.h"
 #include "shell/common/gin_helper/promise.h"
-#include "shell/common/gin_helper/trackable_object.h"
+
+namespace gin {
+class Arguments;
+}
 
 namespace electron {
 
@@ -23,19 +27,22 @@ class ElectronBrowserContext;
 
 namespace api {
 
-class NetLog : public gin_helper::TrackableObject<NetLog> {
+class NetLog : public gin::Wrappable<NetLog> {
  public:
   static gin::Handle<NetLog> Create(v8::Isolate* isolate,
                                     ElectronBrowserContext* browser_context);
 
-  static void BuildPrototype(v8::Isolate* isolate,
-                             v8::Local<v8::FunctionTemplate> prototype);
-
   v8::Local<v8::Promise> StartLogging(base::FilePath log_path,
-                                      gin_helper::Arguments* args);
-  v8::Local<v8::Promise> StopLogging(gin_helper::Arguments* args);
+                                      gin::Arguments* args);
+  v8::Local<v8::Promise> StopLogging(gin::Arguments* args);
   bool IsCurrentlyLogging() const;
 
+  // gin::Wrappable
+  static gin::WrapperInfo kWrapperInfo;
+  gin::ObjectTemplateBuilder GetObjectTemplateBuilder(
+      v8::Isolate* isolate) override;
+  const char* GetTypeName() override;
+
  protected:
   explicit NetLog(v8::Isolate* isolate,
                   ElectronBrowserContext* browser_context);

+ 0 - 5
shell/browser/api/electron_api_session.cc

@@ -307,7 +307,6 @@ Session::~Session() {
   // Refs https://github.com/electron/electron/pull/12305.
   DestroyGlobalHandle(isolate(), cookies_);
   DestroyGlobalHandle(isolate(), protocol_);
-  DestroyGlobalHandle(isolate(), net_log_);
   g_sessions.erase(weak_map_id());
 }
 
@@ -1029,7 +1028,6 @@ void Session::BuildPrototype(v8::Isolate* isolate,
 namespace {
 
 using electron::api::Cookies;
-using electron::api::NetLog;
 using electron::api::Protocol;
 using electron::api::ServiceWorkerContext;
 using electron::api::Session;
@@ -1058,9 +1056,6 @@ void Initialize(v8::Local<v8::Object> exports,
   dict.Set(
       "Cookies",
       Cookies::GetConstructor(isolate)->GetFunction(context).ToLocalChecked());
-  dict.Set(
-      "NetLog",
-      NetLog::GetConstructor(isolate)->GetFunction(context).ToLocalChecked());
   dict.Set(
       "Protocol",
       Protocol::GetConstructor(isolate)->GetFunction(context).ToLocalChecked());

+ 1 - 4
spec-main/api-net-log-spec.ts

@@ -69,10 +69,7 @@ describe('netLog module', () => {
 
     expect(testNetLog().currentlyLogging).to.be.true('currently logging')
 
-    expect(testNetLog().currentlyLoggingPath).to.equal(dumpFileDynamic)
-
-    const path = await testNetLog().stopLogging()
-    expect(path).to.equal(dumpFileDynamic)
+    await testNetLog().stopLogging()
 
     expect(fs.existsSync(dumpFileDynamic)).to.be.true('currently logging')
   })