Browse Source

Merge remote-tracking branch 'origin/main' into replace-browserview

Jeremy Rose 2 years ago
parent
commit
f78fc0518d

+ 1 - 1
docs/api/clipboard.md

@@ -226,7 +226,7 @@ clipboard.writeBuffer('public/utf8-plain-text', buffer)
 
 const ret = clipboard.readBuffer('public/utf8-plain-text')
 
-console.log(buffer.equals(out))
+console.log(buffer.equals(ret))
 // true
 ```
 

+ 4 - 0
docs/api/session.md

@@ -916,6 +916,10 @@ session.fromPartition('some-partition').setPermissionCheckHandler((webContents,
         Specifying a loopback device will capture system audio, and is
         currently only supported on Windows. If a WebFrameMain is specified,
         will capture audio from that frame.
+      * `enableLocalEcho` Boolean (optional) - If `audio` is a [WebFrameMain](web-frame-main.md)
+         and this is set to `true`, then local playback of audio will not be muted (e.g. using `MediaRecorder`
+         to record `WebFrameMain` with this flag set to `true` will allow audio to pass through to the speakers
+         while recording). Default is `false`.
 
 This handler will be called when web content requests access to display media
 via the `navigator.mediaDevices.getDisplayMedia` API. Use the

+ 70 - 23
docs/api/web-contents.md

@@ -207,8 +207,23 @@ See [`window.open()`](window-open.md) for more details and how to use this in co
 
 Returns:
 
-* `event` Event
-* `url` string
+* `details` Event<>
+  * `url` string - The URL the frame is navigating to.
+  * `isSameDocument` boolean - Whether the navigation happened without changing
+    document. Examples of same document navigations are reference fragment
+    navigations, pushState/replaceState, and same page history navigation.
+  * `isMainFrame` boolean - True if the navigation is taking place in a main frame.
+  * `frame` WebFrameMain - The frame to be navigated.
+  * `initiator` WebFrameMain (optional) - The frame which initiated the
+    navigation, which can be a parent frame (e.g. via `window.open` with a
+    frame's name), or null if the navigation was not initiated by a frame. This
+    can also be null if the initiating frame was deleted before the event was
+    emitted.
+* `url` string _Deprecated_
+* `isInPlace` boolean _Deprecated_
+* `isMainFrame` boolean _Deprecated_
+* `frameProcessId` Integer _Deprecated_
+* `frameRoutingId` Integer _Deprecated_
 
 Emitted when a user or the page wants to start navigation. It can happen when
 the `window.location` object is changed or a user clicks a link in the page.
@@ -226,26 +241,47 @@ Calling `event.preventDefault()` will prevent the navigation.
 
 Returns:
 
-* `event` Event
-* `url` string
-* `isInPlace` boolean
-* `isMainFrame` boolean
-* `frameProcessId` Integer
-* `frameRoutingId` Integer
-
-Emitted when any frame (including main) starts navigating. `isInPlace` will be
-`true` for in-page navigations.
+* `details` Event<>
+  * `url` string - The URL the frame is navigating to.
+  * `isSameDocument` boolean - Whether the navigation happened without changing
+    document. Examples of same document navigations are reference fragment
+    navigations, pushState/replaceState, and same page history navigation.
+  * `isMainFrame` boolean - True if the navigation is taking place in a main frame.
+  * `frame` WebFrameMain - The frame to be navigated.
+  * `initiator` WebFrameMain (optional) - The frame which initiated the
+    navigation, which can be a parent frame (e.g. via `window.open` with a
+    frame's name), or null if the navigation was not initiated by a frame. This
+    can also be null if the initiating frame was deleted before the event was
+    emitted.
+* `url` string _Deprecated_
+* `isInPlace` boolean _Deprecated_
+* `isMainFrame` boolean _Deprecated_
+* `frameProcessId` Integer _Deprecated_
+* `frameRoutingId` Integer _Deprecated_
+
+Emitted when any frame (including main) starts navigating.
 
 #### Event: 'will-redirect'
 
 Returns:
 
-* `event` Event
-* `url` string
-* `isInPlace` boolean
-* `isMainFrame` boolean
-* `frameProcessId` Integer
-* `frameRoutingId` Integer
+* `details` Event<>
+  * `url` string - The URL the frame is navigating to.
+  * `isSameDocument` boolean - Whether the navigation happened without changing
+    document. Examples of same document navigations are reference fragment
+    navigations, pushState/replaceState, and same page history navigation.
+  * `isMainFrame` boolean - True if the navigation is taking place in a main frame.
+  * `frame` WebFrameMain - The frame to be navigated.
+  * `initiator` WebFrameMain (optional) - The frame which initiated the
+    navigation, which can be a parent frame (e.g. via `window.open` with a
+    frame's name), or null if the navigation was not initiated by a frame. This
+    can also be null if the initiating frame was deleted before the event was
+    emitted.
+* `url` string _Deprecated_
+* `isInPlace` boolean _Deprecated_
+* `isMainFrame` boolean _Deprecated_
+* `frameProcessId` Integer _Deprecated_
+* `frameRoutingId` Integer _Deprecated_
 
 Emitted when a server side redirect occurs during navigation.  For example a 302
 redirect.
@@ -260,12 +296,23 @@ redirect).
 
 Returns:
 
-* `event` Event
-* `url` string
-* `isInPlace` boolean
-* `isMainFrame` boolean
-* `frameProcessId` Integer
-* `frameRoutingId` Integer
+* `details` Event<>
+  * `url` string - The URL the frame is navigating to.
+  * `isSameDocument` boolean - Whether the navigation happened without changing
+    document. Examples of same document navigations are reference fragment
+    navigations, pushState/replaceState, and same page history navigation.
+  * `isMainFrame` boolean - True if the navigation is taking place in a main frame.
+  * `frame` WebFrameMain - The frame to be navigated.
+  * `initiator` WebFrameMain (optional) - The frame which initiated the
+    navigation, which can be a parent frame (e.g. via `window.open` with a
+    frame's name), or null if the navigation was not initiated by a frame. This
+    can also be null if the initiating frame was deleted before the event was
+    emitted.
+* `url` string _Deprecated_
+* `isInPlace` boolean _Deprecated_
+* `isMainFrame` boolean _Deprecated_
+* `frameProcessId` Integer _Deprecated_
+* `frameRoutingId` Integer _Deprecated_
 
 Emitted after a server side redirect occurs during navigation.  For example a 302
 redirect.

+ 1 - 0
patches/node/.patches

@@ -35,3 +35,4 @@ enable_crashpad_linux_node_processes.patch
 allow_embedder_to_control_codegenerationfromstringscallback.patch
 src_allow_optional_isolation_termination_in_node.patch
 test_mark_cpu_prof_tests_as_flaky_in_electron.patch
+lib_fix_broadcastchannel_initialization_location.patch

+ 44 - 0
patches/node/lib_fix_broadcastchannel_initialization_location.patch

@@ -0,0 +1,44 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Shelley Vohr <[email protected]>
+Date: Mon, 27 Feb 2023 12:56:15 +0100
+Subject: lib: fix BroadcastChannel initialization location
+
+Refs https://github.com/nodejs/node/pull/40532.
+
+Fixes a bug in the above, wherein BroadcastChannel should have been
+initialized in bootstrap/browser instead of bootstrap/node. That
+inadvertently made it such that there was incorrect handling of the
+DOM vs Node.js implementations of BroadcastChannel.
+
+This will be upstreamed.
+
+diff --git a/lib/internal/bootstrap/browser.js b/lib/internal/bootstrap/browser.js
+index d0c01ca2a512be549b0fea8a829c05eabbec799a..210a1bb7e929021725b04786bc11d9b3ce09ad04 100644
+--- a/lib/internal/bootstrap/browser.js
++++ b/lib/internal/bootstrap/browser.js
+@@ -12,6 +12,10 @@ const {
+ } = require('internal/util');
+ const config = internalBinding('config');
+ 
++// Non-standard extensions:
++const { BroadcastChannel } = require('internal/worker/io');
++exposeInterface(globalThis, 'BroadcastChannel', BroadcastChannel);
++
+ // https://console.spec.whatwg.org/#console-namespace
+ exposeNamespace(globalThis, 'console',
+                 createGlobalConsole());
+diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
+index 7dd89d5f134b09da2678dd54fa9139466fea179c..f235545b6433c0f1dd335e0bb97cd3dc77616c0f 100644
+--- a/lib/internal/bootstrap/node.js
++++ b/lib/internal/bootstrap/node.js
+@@ -237,10 +237,6 @@ const {
+   queueMicrotask
+ } = require('internal/process/task_queues');
+ 
+-// Non-standard extensions:
+-const { BroadcastChannel } = require('internal/worker/io');
+-exposeInterface(globalThis, 'BroadcastChannel', BroadcastChannel);
+-
+ defineOperation(globalThis, 'queueMicrotask', queueMicrotask);
+ 
+ const timers = require('timers');

+ 59 - 22
shell/browser/api/electron_api_web_contents.cc

@@ -112,6 +112,7 @@
 #include "shell/common/gin_converters/gurl_converter.h"
 #include "shell/common/gin_converters/image_converter.h"
 #include "shell/common/gin_converters/net_converter.h"
+#include "shell/common/gin_converters/optional_converter.h"
 #include "shell/common/gin_converters/value_converter.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/gin_helper/object_template_builder.h"
@@ -1597,8 +1598,7 @@ void WebContents::RenderFrameHostChanged(content::RenderFrameHost* old_host,
   //
   // |old_host| can be a nullptr so we use |new_host| for looking up the
   // WebFrameMain instance.
-  auto* web_frame =
-      WebFrameMain::FromFrameTreeNodeId(new_host->GetFrameTreeNodeId());
+  auto* web_frame = WebFrameMain::FromRenderFrameHost(new_host);
   if (web_frame) {
     web_frame->UpdateRenderFrameHost(new_host);
   }
@@ -1737,7 +1737,7 @@ void WebContents::DidStopLoading() {
 }
 
 bool WebContents::EmitNavigationEvent(
-    const std::string& event,
+    const std::string& event_name,
     content::NavigationHandle* navigation_handle) {
   bool is_main_frame = navigation_handle->IsInMainFrame();
   int frame_tree_node_id = navigation_handle->GetFrameTreeNodeId();
@@ -1758,8 +1758,30 @@ bool WebContents::EmitNavigationEvent(
   }
   bool is_same_document = navigation_handle->IsSameDocument();
   auto url = navigation_handle->GetURL();
-  return Emit(event, url, is_same_document, is_main_frame, frame_process_id,
-              frame_routing_id);
+  content::RenderFrameHost* initiator_frame_host =
+      navigation_handle->GetInitiatorFrameToken().has_value()
+          ? content::RenderFrameHost::FromFrameToken(
+                navigation_handle->GetInitiatorProcessID(),
+                navigation_handle->GetInitiatorFrameToken().value())
+          : nullptr;
+
+  v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
+  v8::HandleScope handle_scope(isolate);
+
+  gin::Handle<gin_helper::internal::Event> event =
+      gin_helper::internal::Event::New(isolate);
+  v8::Local<v8::Object> event_object = event.ToV8().As<v8::Object>();
+
+  gin_helper::Dictionary dict(isolate, event_object);
+  dict.Set("url", url);
+  dict.Set("isSameDocument", is_same_document);
+  dict.Set("isMainFrame", is_main_frame);
+  dict.Set("frame", frame_host);
+  dict.SetGetter("initiator", initiator_frame_host);
+
+  EmitWithoutEvent(event_name, event, url, is_same_document, is_main_frame,
+                   frame_process_id, frame_routing_id);
+  return event->GetDefaultPrevented();
 }
 
 void WebContents::Message(bool internal,
@@ -1813,22 +1835,23 @@ class ReplyChannel : public gin::Wrappable<ReplyChannel> {
   }
   const char* GetTypeName() override { return "ReplyChannel"; }
 
+  void SendError(const std::string& msg) {
+    v8::Isolate* isolate = electron::JavascriptEnvironment::GetIsolate();
+    // If there's no current context, it means we're shutting down, so we
+    // don't need to send an event.
+    if (!isolate->GetCurrentContext().IsEmpty()) {
+      v8::HandleScope scope(isolate);
+      auto message = gin::DataObjectBuilder(isolate).Set("error", msg).Build();
+      SendReply(isolate, message);
+    }
+  }
+
  private:
   explicit ReplyChannel(InvokeCallback callback)
       : callback_(std::move(callback)) {}
   ~ReplyChannel() override {
-    if (callback_) {
-      v8::Isolate* isolate = electron::JavascriptEnvironment::GetIsolate();
-      // If there's no current context, it means we're shutting down, so we
-      // don't need to send an event.
-      if (!isolate->GetCurrentContext().IsEmpty()) {
-        v8::HandleScope scope(isolate);
-        auto message = gin::DataObjectBuilder(isolate)
-                           .Set("error", "reply was never sent")
-                           .Build();
-        SendReply(isolate, message);
-      }
-    }
+    if (callback_)
+      SendError("reply was never sent");
   }
 
   bool SendReply(v8::Isolate* isolate, v8::Local<v8::Value> arg) {
@@ -1853,8 +1876,14 @@ gin::Handle<gin_helper::internal::Event> WebContents::MakeEventWithSender(
     content::RenderFrameHost* frame,
     electron::mojom::ElectronApiIPC::InvokeCallback callback) {
   v8::Local<v8::Object> wrapper;
-  if (!GetWrapper(isolate).ToLocal(&wrapper))
+  if (!GetWrapper(isolate).ToLocal(&wrapper)) {
+    if (callback) {
+      // We must always invoke the callback if present.
+      ReplyChannel::Create(isolate, std::move(callback))
+          ->SendError("WebContents was destroyed");
+    }
     return gin::Handle<gin_helper::internal::Event>();
+  }
   gin::Handle<gin_helper::internal::Event> event =
       gin_helper::internal::Event::New(isolate);
   gin_helper::Dictionary dict(isolate, event.ToV8().As<v8::Object>());
@@ -3586,18 +3615,26 @@ v8::Local<v8::Promise> WebContents::TakeHeapSnapshot(
   flags = base::File::AddFlagsForPassingToUntrustedProcess(flags);
   base::File file(file_path, flags);
   if (!file.IsValid()) {
-    promise.RejectWithErrorMessage("takeHeapSnapshot failed");
+    promise.RejectWithErrorMessage(
+        "Failed to take heap snapshot with invalid file path " +
+#if BUILDFLAG(IS_WIN)
+        base::WideToUTF8(file_path.value()));
+#else
+        file_path.value());
+#endif
     return handle;
   }
 
   auto* frame_host = web_contents()->GetPrimaryMainFrame();
   if (!frame_host) {
-    promise.RejectWithErrorMessage("takeHeapSnapshot failed");
+    promise.RejectWithErrorMessage(
+        "Failed to take heap snapshot with invalid webContents main frame");
     return handle;
   }
 
   if (!frame_host->IsRenderFrameLive()) {
-    promise.RejectWithErrorMessage("takeHeapSnapshot failed");
+    promise.RejectWithErrorMessage(
+        "Failed to take heap snapshot with nonexistent render frame");
     return handle;
   }
 
@@ -3617,7 +3654,7 @@ v8::Local<v8::Promise> WebContents::TakeHeapSnapshot(
             if (success) {
               promise.Resolve();
             } else {
-              promise.RejectWithErrorMessage("takeHeapSnapshot failed");
+              promise.RejectWithErrorMessage("Failed to take heap snapshot");
             }
           },
           base::Owned(std::move(electron_renderer)), std::move(promise)));

+ 1 - 1
shell/browser/browser.h

@@ -363,7 +363,7 @@ class Browser : public WindowListObserver {
   base::Time last_dock_show_;
 #endif
 
-  base::Value about_panel_options_;
+  base::Value::Dict about_panel_options_;
 
 #if BUILDFLAG(IS_WIN)
   void UpdateBadgeContents(HWND hwnd,

+ 9 - 15
shell/browser/browser_linux.cc

@@ -162,31 +162,25 @@ bool Browser::IsEmojiPanelSupported() {
 void Browser::ShowAboutPanel() {
   const auto& opts = about_panel_options_;
 
-  if (!opts.is_dict()) {
-    LOG(WARNING) << "Called showAboutPanel(), but didn't use "
-                    "setAboutPanelSettings() first";
-    return;
-  }
-
   GtkWidget* dialogWidget = gtk_about_dialog_new();
   GtkAboutDialog* dialog = GTK_ABOUT_DIALOG(dialogWidget);
 
   const std::string* str;
-  const base::Value* val;
+  const base::Value::List* list;
 
-  if ((str = opts.FindStringKey("applicationName"))) {
+  if ((str = opts.FindString("applicationName"))) {
     gtk_about_dialog_set_program_name(dialog, str->c_str());
   }
-  if ((str = opts.FindStringKey("applicationVersion"))) {
+  if ((str = opts.FindString("applicationVersion"))) {
     gtk_about_dialog_set_version(dialog, str->c_str());
   }
-  if ((str = opts.FindStringKey("copyright"))) {
+  if ((str = opts.FindString("copyright"))) {
     gtk_about_dialog_set_copyright(dialog, str->c_str());
   }
-  if ((str = opts.FindStringKey("website"))) {
+  if ((str = opts.FindString("website"))) {
     gtk_about_dialog_set_website(dialog, str->c_str());
   }
-  if ((str = opts.FindStringKey("iconPath"))) {
+  if ((str = opts.FindString("iconPath"))) {
     GError* error = nullptr;
     constexpr int width = 64;   // width of about panel icon in pixels
     constexpr int height = 64;  // height of about panel icon in pixels
@@ -203,9 +197,9 @@ void Browser::ShowAboutPanel() {
     }
   }
 
-  if ((val = opts.FindListKey("authors"))) {
+  if ((list = opts.FindList("authors"))) {
     std::vector<const char*> cstrs;
-    for (const auto& authorVal : val->GetList()) {
+    for (const auto& authorVal : *list) {
       if (authorVal.is_string()) {
         cstrs.push_back(authorVal.GetString().c_str());
       }
@@ -223,7 +217,7 @@ void Browser::ShowAboutPanel() {
 }
 
 void Browser::SetAboutPanelOptions(base::Value::Dict options) {
-  about_panel_options_ = base::Value(std::move(options));
+  about_panel_options_ = std::move(options);
 }
 
 }  // namespace electron

+ 3 - 4
shell/browser/browser_mac.mm

@@ -513,8 +513,7 @@ void Browser::DockSetIcon(v8::Isolate* isolate, v8::Local<v8::Value> icon) {
 }
 
 void Browser::ShowAboutPanel() {
-  NSDictionary* options =
-      DictionaryValueToNSDictionary(about_panel_options_.GetDict());
+  NSDictionary* options = DictionaryValueToNSDictionary(about_panel_options_);
 
   // Credits must be a NSAttributedString instead of NSString
   NSString* credits = (NSString*)options[@"Credits"];
@@ -537,13 +536,13 @@ void Browser::ShowAboutPanel() {
 }
 
 void Browser::SetAboutPanelOptions(base::Value::Dict options) {
-  about_panel_options_.GetDict().clear();
+  about_panel_options_.clear();
 
   for (const auto pair : options) {
     std::string key = pair.first;
     if (!key.empty() && pair.second.is_string()) {
       key[0] = base::ToUpperASCII(key[0]);
-      about_panel_options_.GetDict().Set(key, pair.second.Clone());
+      about_panel_options_.Set(key, pair.second.Clone());
     }
   }
 }

+ 8 - 9
shell/browser/browser_win.cc

@@ -734,30 +734,29 @@ void Browser::ShowEmojiPanel() {
 }
 
 void Browser::ShowAboutPanel() {
-  base::Value dict(base::Value::Type::DICTIONARY);
+  base::Value::Dict dict;
   std::string aboutMessage = "";
   gfx::ImageSkia image;
 
   // grab defaults from Windows .EXE file
   std::unique_ptr<FileVersionInfo> exe_info = FetchFileVersionInfo();
-  dict.SetStringKey("applicationName", exe_info->file_description());
-  dict.SetStringKey("applicationVersion", exe_info->product_version());
+  dict.Set("applicationName", exe_info->file_description());
+  dict.Set("applicationVersion", exe_info->product_version());
 
-  if (about_panel_options_.is_dict()) {
-    dict.MergeDictionary(&about_panel_options_);
-  }
+  // Merge user-provided options, overwriting any of the above
+  dict.Merge(about_panel_options_.Clone());
 
   std::vector<std::string> stringOptions = {
       "applicationName", "applicationVersion", "copyright", "credits"};
 
   const std::string* str;
   for (std::string opt : stringOptions) {
-    if ((str = dict.FindStringKey(opt))) {
+    if ((str = dict.FindString(opt))) {
       aboutMessage.append(*str).append("\r\n");
     }
   }
 
-  if ((str = dict.FindStringKey("iconPath"))) {
+  if ((str = dict.FindString("iconPath"))) {
     base::FilePath path = base::FilePath::FromUTF8Unsafe(*str);
     electron::util::PopulateImageSkiaRepsFromPath(&image, path);
   }
@@ -770,7 +769,7 @@ void Browser::ShowAboutPanel() {
 }
 
 void Browser::SetAboutPanelOptions(base::Value::Dict options) {
-  about_panel_options_ = base::Value(std::move(options));
+  about_panel_options_ = std::move(options);
 }
 
 }  // namespace electron

+ 10 - 7
shell/browser/electron_browser_context.cc

@@ -501,13 +501,16 @@ void ElectronBrowserContext::DisplayMediaDeviceChosen(
       devices.audio_device =
           blink::MediaStreamDevice(request.audio_type, id, name);
     } else if (result_dict.Get("audio", &rfh)) {
-      devices.audio_device = blink::MediaStreamDevice(
-          request.audio_type,
-          content::WebContentsMediaCaptureId(rfh->GetProcess()->GetID(),
-                                             rfh->GetRoutingID(),
-                                             /* disable_local_echo= */ true)
-              .ToString(),
-          "Tab audio");
+      bool enable_local_echo = false;
+      result_dict.Get("enableLocalEcho", &enable_local_echo);
+      bool disable_local_echo = !enable_local_echo;
+      devices.audio_device =
+          blink::MediaStreamDevice(request.audio_type,
+                                   content::WebContentsMediaCaptureId(
+                                       rfh->GetProcess()->GetID(),
+                                       rfh->GetRoutingID(), disable_local_echo)
+                                       .ToString(),
+                                   "Tab audio");
     } else if (result_dict.Get("audio", &id)) {
       devices.audio_device =
           blink::MediaStreamDevice(request.audio_type, id, "System audio");

+ 10 - 8
shell/browser/notifications/mac/cocoa_notification.mm

@@ -58,7 +58,11 @@ void CocoaNotification::Show(const NotificationOptions& options) {
     [notification_ setSoundName:base::SysUTF16ToNSString(options.sound)];
   }
 
-  [notification_ setHasActionButton:false];
+  if (options.has_reply) {
+    [notification_ setHasReplyButton:true];
+    [notification_ setResponsePlaceholder:base::SysUTF16ToNSString(
+                                              options.reply_placeholder)];
+  }
 
   int i = 0;
   action_index_ = UINT_MAX;
@@ -66,7 +70,10 @@ void CocoaNotification::Show(const NotificationOptions& options) {
       [[[NSMutableArray alloc] init] autorelease];
   for (const auto& action : options.actions) {
     if (action.type == u"button") {
-      if (action_index_ == UINT_MAX) {
+      // If the notification has both a reply and actions,
+      // the reply takes precedence and the actions all
+      // become additional actions.
+      if (!options.has_reply && action_index_ == UINT_MAX) {
         // First button observed is the displayed action
         [notification_ setHasActionButton:true];
         [notification_
@@ -86,16 +93,11 @@ void CocoaNotification::Show(const NotificationOptions& options) {
     }
     i++;
   }
+
   if ([additionalActions count] > 0) {
     [notification_ setAdditionalActions:additionalActions];
   }
 
-  if (options.has_reply) {
-    [notification_ setResponsePlaceholder:base::SysUTF16ToNSString(
-                                              options.reply_placeholder)];
-    [notification_ setHasReplyButton:true];
-  }
-
   if (!options.close_button_text.empty()) {
     [notification_ setOtherButtonTitle:base::SysUTF16ToNSString(
                                            options.close_button_text)];

+ 91 - 0
shell/browser/ui/cocoa/electron_ns_window.mm

@@ -11,6 +11,9 @@
 #include "shell/browser/ui/cocoa/root_view_mac.h"
 #include "ui/base/cocoa/window_size_constants.h"
 
+#import <objc/message.h>
+#import <objc/runtime.h>
+
 namespace electron {
 
 int ScopedDisableResize::disable_resize_ = 0;
@@ -19,8 +22,65 @@ int ScopedDisableResize::disable_resize_ = 0;
 
 @interface NSWindow (PrivateAPI)
 - (NSImage*)_cornerMask;
+- (int64_t)_resizeDirectionForMouseLocation:(CGPoint)location;
+@end
+
+#if IS_MAS_BUILD()
+// See components/remote_cocoa/app_shim/native_widget_mac_nswindow.mm
+@interface NSView (CRFrameViewAdditions)
+- (void)cr_mouseDownOnFrameView:(NSEvent*)event;
+@end
+
+typedef void (*MouseDownImpl)(id, SEL, NSEvent*);
+
+namespace {
+MouseDownImpl g_nsthemeframe_mousedown;
+MouseDownImpl g_nsnextstepframe_mousedown;
+}  // namespace
+
+// This class is never instantiated, it's just a container for our swizzled
+// mouseDown method.
+@interface SwizzledMouseDownHolderClass : NSView
+@end
+
+@implementation SwizzledMouseDownHolderClass
+- (void)swiz_nsthemeframe_mouseDown:(NSEvent*)event {
+  if ([self.window respondsToSelector:@selector(shell)]) {
+    electron::NativeWindowMac* shell =
+        (electron::NativeWindowMac*)[(id)self.window shell];
+    if (shell && !shell->has_frame())
+      [self cr_mouseDownOnFrameView:event];
+    g_nsthemeframe_mousedown(self, @selector(mouseDown:), event);
+  }
+}
+
+- (void)swiz_nsnextstepframe_mouseDown:(NSEvent*)event {
+  if ([self.window respondsToSelector:@selector(shell)]) {
+    electron::NativeWindowMac* shell =
+        (electron::NativeWindowMac*)[(id)self.window shell];
+    if (shell && !shell->has_frame()) {
+      [self cr_mouseDownOnFrameView:event];
+    }
+    g_nsnextstepframe_mousedown(self, @selector(mouseDown:), event);
+  }
+}
 @end
 
+namespace {
+void SwizzleMouseDown(NSView* frame_view,
+                      SEL swiz_selector,
+                      MouseDownImpl* orig_impl) {
+  Method original_mousedown =
+      class_getInstanceMethod([frame_view class], @selector(mouseDown:));
+  *orig_impl = (MouseDownImpl)method_getImplementation(original_mousedown);
+  Method new_mousedown = class_getInstanceMethod(
+      [SwizzledMouseDownHolderClass class], swiz_selector);
+  method_setImplementation(original_mousedown,
+                           method_getImplementation(new_mousedown));
+}
+}  // namespace
+#endif  // IS_MAS_BUILD
+
 @implementation ElectronNSWindow
 
 @synthesize acceptsFirstMouse;
@@ -36,6 +96,37 @@ int ScopedDisableResize::disable_resize_ = 0;
                                styleMask:styleMask
                                  backing:NSBackingStoreBuffered
                                    defer:NO])) {
+#if IS_MAS_BUILD()
+    // The first time we create a frameless window, we swizzle the
+    // implementation of -[NSNextStepFrame mouseDown:], replacing it with our
+    // own.
+    // This is only necessary on MAS where we can't directly refer to
+    // NSNextStepFrame or NSThemeFrame, as they are private APIs.
+    // See components/remote_cocoa/app_shim/native_widget_mac_nswindow.mm for
+    // the non-MAS-compatible way of doing this.
+    if (styleMask & NSWindowStyleMaskTitled) {
+      if (!g_nsthemeframe_mousedown) {
+        NSView* theme_frame = [[self contentView] superview];
+        DCHECK(strcmp(class_getName([theme_frame class]), "NSThemeFrame") == 0)
+            << "Expected NSThemeFrame but was "
+            << class_getName([theme_frame class]);
+        SwizzleMouseDown(theme_frame, @selector(swiz_nsthemeframe_mouseDown:),
+                         &g_nsthemeframe_mousedown);
+      }
+    } else {
+      if (!g_nsnextstepframe_mousedown) {
+        NSView* nextstep_frame = [[self contentView] superview];
+        DCHECK(strcmp(class_getName([nextstep_frame class]),
+                      "NSNextStepFrame") == 0)
+            << "Expected NSNextStepFrame but was "
+            << class_getName([nextstep_frame class]);
+        SwizzleMouseDown(nextstep_frame,
+                         @selector(swiz_nsnextstepframe_mouseDown:),
+                         &g_nsnextstepframe_mousedown);
+      }
+    }
+#endif  // IS_MAS_BUILD
+
     shell_ = shell;
   }
   return self;

+ 38 - 3
shell/browser/ui/cocoa/electron_ns_window_delegate.mm

@@ -128,16 +128,17 @@ using FullScreenTransitionState =
 - (NSSize)windowWillResize:(NSWindow*)sender toSize:(NSSize)frameSize {
   NSSize newSize = frameSize;
   double aspectRatio = shell_->GetAspectRatio();
+  NSWindow* window = shell_->GetNativeWindow().GetNativeNSWindow();
 
   if (aspectRatio > 0.0) {
     gfx::Size windowSize = shell_->GetSize();
     gfx::Size contentSize = shell_->GetContentSize();
     gfx::Size extraSize = shell_->GetAspectRatioExtraSize();
 
+    double titleBarHeight = windowSize.height() - contentSize.height();
     double extraWidthPlusFrame =
         windowSize.width() - contentSize.width() + extraSize.width();
-    double extraHeightPlusFrame =
-        windowSize.height() - contentSize.height() + extraSize.height();
+    double extraHeightPlusFrame = titleBarHeight + extraSize.height();
 
     newSize.width =
         roundf((frameSize.height - extraHeightPlusFrame) * aspectRatio +
@@ -145,10 +146,44 @@ using FullScreenTransitionState =
     newSize.height =
         roundf((newSize.width - extraWidthPlusFrame) / aspectRatio +
                extraHeightPlusFrame);
+
+    // Clamp to minimum width/height while ensuring aspect ratio remains.
+    NSSize minSize = [window minSize];
+    NSSize zeroSize =
+        shell_->has_frame() ? NSMakeSize(0, titleBarHeight) : NSZeroSize;
+    if (!NSEqualSizes(minSize, zeroSize)) {
+      double minWidthForAspectRatio =
+          (minSize.height - titleBarHeight) * aspectRatio;
+      bool atMinHeight =
+          minSize.height > zeroSize.height && newSize.height <= minSize.height;
+      newSize.width = atMinHeight ? minWidthForAspectRatio
+                                  : std::max(newSize.width, minSize.width);
+
+      double minHeightForAspectRatio = minSize.width / aspectRatio;
+      bool atMinWidth =
+          minSize.width > zeroSize.width && newSize.width <= minSize.width;
+      newSize.height = atMinWidth ? minHeightForAspectRatio
+                                  : std::max(newSize.height, minSize.height);
+    }
+
+    // Clamp to maximum width/height while ensuring aspect ratio remains.
+    NSSize maxSize = [window maxSize];
+    if (!NSEqualSizes(maxSize, NSMakeSize(FLT_MAX, FLT_MAX))) {
+      double maxWidthForAspectRatio = maxSize.height * aspectRatio;
+      bool atMaxHeight =
+          maxSize.height < FLT_MAX && newSize.height >= maxSize.height;
+      newSize.width = atMaxHeight ? maxWidthForAspectRatio
+                                  : std::min(newSize.width, maxSize.width);
+
+      double maxHeightForAspectRatio = maxSize.width / aspectRatio;
+      bool atMaxWidth =
+          maxSize.width < FLT_MAX && newSize.width >= maxSize.width;
+      newSize.height = atMaxWidth ? maxHeightForAspectRatio
+                                  : std::min(newSize.height, maxSize.height);
+    }
   }
 
   if (!resizingHorizontally_) {
-    NSWindow* window = shell_->GetNativeWindow().GetNativeNSWindow();
     const auto widthDelta = frameSize.width - [window frame].size.width;
     const auto heightDelta = frameSize.height - [window frame].size.height;
     resizingHorizontally_ = std::abs(widthDelta) > std::abs(heightDelta);

+ 9 - 0
spec/api-app-spec.ts

@@ -1859,6 +1859,15 @@ describe('app module', () => {
       })).to.eventually.be.rejectedWith(/ERR_NAME_NOT_RESOLVED/);
     });
   });
+
+  describe('about panel', () => {
+    it('app.setAboutPanelOptions() does not crash', () => {
+      app.setAboutPanelOptions({
+        applicationName: 'electron!!',
+        version: '1.2.3'
+      });
+    });
+  });
 });
 
 describe('default behavior', () => {

+ 18 - 0
spec/api-browser-view-spec.ts

@@ -503,6 +503,24 @@ describe('BrowserView module', () => {
       const [code] = await once(rc.process, 'exit');
       expect(code).to.equal(0);
     });
+
+    it('emits the destroyed event when webContents.close() is called', async () => {
+      view = new BrowserView();
+      w.setBrowserView(view);
+      await view.webContents.loadFile(path.join(fixtures, 'pages', 'a.html'));
+
+      view.webContents.close();
+      await once(view.webContents, 'destroyed');
+    });
+
+    it('emits the destroyed event when window.close() is called', async () => {
+      view = new BrowserView();
+      w.setBrowserView(view);
+      await view.webContents.loadFile(path.join(fixtures, 'pages', 'a.html'));
+
+      view.webContents.executeJavaScript('window.close()');
+      await once(view.webContents, 'destroyed');
+    });
   });
 
   describe('window.open()', () => {

+ 11 - 28
spec/api-browser-window-spec.ts

@@ -5,7 +5,8 @@ import * as fs from 'fs';
 import * as qs from 'querystring';
 import * as http from 'http';
 import * as os from 'os';
-import { app, BrowserWindow, BrowserView, dialog, ipcMain, OnBeforeSendHeadersListenerDetails, protocol, screen, webContents, session, WebContents } from 'electron/main';
+import { AddressInfo } from 'net';
+import { app, BrowserWindow, BrowserView, dialog, ipcMain, OnBeforeSendHeadersListenerDetails, protocol, screen, webContents, session, WebContents, WebFrameMain } from 'electron/main';
 
 import { emittedUntil, emittedNTimes } from './lib/events-helpers';
 import { ifit, ifdescribe, defer, listen } from './lib/spec-helpers';
@@ -553,35 +554,17 @@ describe('BrowserWindow module', () => {
         });
 
         it('is triggered when a cross-origin iframe navigates _top', async () => {
-          await w.loadURL(`data:text/html,<iframe src="${url}/navigate-top"></iframe>`);
-          await setTimeout(1000);
-          w.webContents.debugger.attach('1.1');
-          const targets = await w.webContents.debugger.sendCommand('Target.getTargets');
-          const iframeTarget = targets.targetInfos.find((t: any) => t.type === 'iframe');
-          const { sessionId } = await w.webContents.debugger.sendCommand('Target.attachToTarget', {
-            targetId: iframeTarget.targetId,
-            flatten: true
-          });
-          let willNavigateEmitted = false;
-          w.webContents.on('will-navigate', () => {
-            willNavigateEmitted = true;
+          w.loadURL(`data:text/html,<iframe src="http://127.0.0.1:${(server.address() as AddressInfo).port}/navigate-top"></iframe>`);
+          await emittedUntil(w.webContents, 'did-frame-finish-load', (e: any, isMainFrame: boolean) => !isMainFrame);
+          let initiator: WebFrameMain | undefined;
+          w.webContents.on('will-navigate', (e) => {
+            initiator = e.initiator;
           });
-          await w.webContents.debugger.sendCommand('Input.dispatchMouseEvent', {
-            type: 'mousePressed',
-            x: 10,
-            y: 10,
-            clickCount: 1,
-            button: 'left'
-          }, sessionId);
-          await w.webContents.debugger.sendCommand('Input.dispatchMouseEvent', {
-            type: 'mouseReleased',
-            x: 10,
-            y: 10,
-            clickCount: 1,
-            button: 'left'
-          }, sessionId);
+          const subframe = w.webContents.mainFrame.frames[0];
+          subframe.executeJavaScript('document.getElementsByTagName("a")[0].click()', true);
           await once(w.webContents, 'did-navigate');
-          expect(willNavigateEmitted).to.be.true();
+          expect(initiator).not.to.be.undefined();
+          expect(initiator).to.equal(subframe);
         });
       });
 

+ 18 - 2
spec/api-web-contents-spec.ts

@@ -1803,8 +1803,24 @@ describe('webContents module', () => {
 
       await w.loadURL('about:blank');
 
-      const promise = w.webContents.takeHeapSnapshot('');
-      return expect(promise).to.be.eventually.rejectedWith(Error, 'takeHeapSnapshot failed');
+      const badPath = path.join('i', 'am', 'a', 'super', 'bad', 'path');
+      const promise = w.webContents.takeHeapSnapshot(badPath);
+      return expect(promise).to.be.eventually.rejectedWith(Error, `Failed to take heap snapshot with invalid file path ${badPath}`);
+    });
+
+    it('fails with invalid render process', async () => {
+      const w = new BrowserWindow({
+        show: false,
+        webPreferences: {
+          sandbox: true
+        }
+      });
+
+      const filePath = path.join(app.getPath('temp'), 'test.heapsnapshot');
+
+      w.webContents.destroy();
+      const promise = w.webContents.takeHeapSnapshot(filePath);
+      return expect(promise).to.be.eventually.rejectedWith(Error, 'Failed to take heap snapshot with nonexistent render frame');
     });
   });
 

+ 3 - 0
spec/disabled-tests.json

@@ -0,0 +1,3 @@
+[
+  "// NOTE: this file is used to disable tests in our test suite by their full title."
+]

+ 13 - 0
spec/index.js

@@ -1,3 +1,4 @@
+const fs = require('fs');
 const path = require('path');
 const v8 = require('v8');
 
@@ -65,6 +66,18 @@ app.whenReady().then(async () => {
   }
   const mocha = new Mocha(mochaOptions);
 
+  // Add a root hook on mocha to skip any tests that are disabled
+  const disabledTests = new Set(
+    JSON.parse(
+      fs.readFileSync(path.join(__dirname, 'disabled-tests.json'), 'utf8')
+    )
+  );
+  mocha.suite.beforeEach(function () {
+    if (disabledTests.has(this.currentTest?.fullTitle())) {
+      this.skip();
+    }
+  });
+
   // The cleanup method is registered this way rather than through an
   // `afterEach` at the top level so that it can run before other `afterEach`
   // methods.