From 4d89174b4187067a1f91f44f48c4234720fd24fa Mon Sep 17 00:00:00 2001
From: Samuel Maddock <samuel.maddock@gmail.com>
Date: Wed, 1 Sep 2021 18:21:15 -0400
Subject: [PATCH] feat: add 'dom-ready' event to WebFrameMain (#29290)

---
 docs/api/web-contents.md                      | 22 ++++--
 docs/api/web-frame-main.md                    |  6 ++
 .../browser/api/electron_api_web_contents.cc  | 30 +++++++-
 .../api/electron_api_web_frame_main.cc        |  4 ++
 .../browser/api/electron_api_web_frame_main.h |  5 +-
 .../common/gin_converters/frame_converter.cc  | 68 +++++++++++++++++++
 shell/common/gin_converters/frame_converter.h | 11 +++
 shell/common/gin_helper/accessor.h            | 27 ++++++++
 shell/common/gin_helper/dictionary.h          | 31 +++++++++
 spec-main/api-web-frame-main-spec.ts          | 64 +++++++++++++++++
 .../fixtures/sub-frames/frame-container.html  |  2 +-
 .../fixtures/sub-frames/frame-with-frame.html |  2 +-
 12 files changed, 261 insertions(+), 11 deletions(-)
 create mode 100644 shell/common/gin_helper/accessor.h

diff --git a/docs/api/web-contents.md b/docs/api/web-contents.md
index c7db70e1ea7d4..040e5746e889d 100644
--- a/docs/api/web-contents.md
+++ b/docs/api/web-contents.md
@@ -134,7 +134,7 @@ Returns:
 
 * `event` Event
 
-Emitted when the document in the given frame is loaded.
+Emitted when the document in the top-level frame is loaded.
 
 #### Event: 'page-title-updated'
 
@@ -876,6 +876,16 @@ Emitted when the `WebContents` preferred size has changed.
 This event will only be emitted when `enablePreferredSizeMode` is set to `true`
 in `webPreferences`.
 
+#### Event: 'frame-created'
+
+Returns:
+
+* `event` Event
+* `details` Object
+  * `frame` WebFrameMain
+
+Emitted when the [mainFrame](web-contents.md#contentsmainframe-readonly), an `<iframe>`, or a nested `<iframe>` is loaded within the page.
+
 ### Instance Methods
 
 #### `contents.loadURL(url[, options])`
@@ -2012,11 +2022,6 @@ when the DevTools has been closed.
 
 A [`Debugger`](debugger.md) instance for this webContents.
 
-[keyboardevent]: https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent
-[event-emitter]: https://nodejs.org/api/events.html#events_class_eventemitter
-[SCA]: https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm
-[`postMessage`]: https://developer.mozilla.org/en-US/docs/Web/API/Window/postMessage
-
 #### `contents.backgroundThrottling`
 
 A `Boolean` property that determines whether or not this WebContents will throttle animations and timers
@@ -2025,3 +2030,8 @@ when the page becomes backgrounded. This also affects the Page Visibility API.
 #### `contents.mainFrame` _Readonly_
 
 A [`WebFrameMain`](web-frame-main.md) property that represents the top frame of the page's frame hierarchy.
+
+[keyboardevent]: https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent
+[event-emitter]: https://nodejs.org/api/events.html#events_class_eventemitter
+[SCA]: https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm
+[`postMessage`]: https://developer.mozilla.org/en-US/docs/Web/API/Window/postMessage
diff --git a/docs/api/web-frame-main.md b/docs/api/web-frame-main.md
index f4b974a649d30..aa12bcb3275fe 100644
--- a/docs/api/web-frame-main.md
+++ b/docs/api/web-frame-main.md
@@ -71,6 +71,12 @@ or `undefined` if there is no WebFrameMain associated with the given IDs.
 Process: [Main](../glossary.md#main-process)<br />
 _This class is not exported from the `'electron'` module. It is only available as a return value of other methods in the Electron API._
 
+### Instance Events
+
+#### Event: 'dom-ready'
+
+Emitted when the document is loaded.
+
 ### Instance Methods
 
 #### `frame.executeJavaScript(code[, userGesture])`
diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc
index f8cc6889e1e68..d78b6ff671ec2 100644
--- a/shell/browser/api/electron_api_web_contents.cc
+++ b/shell/browser/api/electron_api_web_contents.cc
@@ -1393,6 +1393,29 @@ void WebContents::HandleNewRenderFrame(
 void WebContents::RenderFrameCreated(
     content::RenderFrameHost* render_frame_host) {
   HandleNewRenderFrame(render_frame_host);
+
+  // RenderFrameCreated is called for speculative frames which may not be
+  // used in certain cross-origin navigations. Invoking
+  // RenderFrameHost::GetLifecycleState currently crashes when called for
+  // speculative frames so we need to filter it out for now. Check
+  // https://crbug.com/1183639 for details on when this can be removed.
+  auto* rfh_impl =
+      static_cast<content::RenderFrameHostImpl*>(render_frame_host);
+  if (rfh_impl->lifecycle_state() ==
+      content::RenderFrameHostImpl::LifecycleStateImpl::kSpeculative) {
+    return;
+  }
+
+  content::RenderFrameHost::LifecycleState lifecycle_state =
+      render_frame_host->GetLifecycleState();
+  if (lifecycle_state == content::RenderFrameHost::LifecycleState::kActive) {
+    v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
+    v8::HandleScope handle_scope(isolate);
+    gin_helper::Dictionary details =
+        gin_helper::Dictionary::CreateEmpty(isolate);
+    details.SetGetter("frame", render_frame_host);
+    Emit("frame-created", details);
+  }
 }
 
 void WebContents::RenderFrameDeleted(
@@ -1419,12 +1442,11 @@ void WebContents::RenderFrameHostChanged(content::RenderFrameHost* old_host,
   // If an instance of WebFrameMain exists, it will need to have its RFH
   // swapped as well.
   //
-  // |old_host| can be a nullptr in so we use |new_host| for looking up the
+  // |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());
   if (web_frame) {
-    CHECK_EQ(web_frame->render_frame_host(), old_host);
     web_frame->UpdateRenderFrameHost(new_host);
   }
 }
@@ -1504,6 +1526,10 @@ void WebContents::DidAcquireFullscreen(content::RenderFrameHost* rfh) {
 
 void WebContents::DOMContentLoaded(
     content::RenderFrameHost* render_frame_host) {
+  auto* web_frame = WebFrameMain::FromRenderFrameHost(render_frame_host);
+  if (web_frame)
+    web_frame->DOMContentLoaded();
+
   if (!render_frame_host->GetParent())
     Emit("dom-ready");
 }
diff --git a/shell/browser/api/electron_api_web_frame_main.cc b/shell/browser/api/electron_api_web_frame_main.cc
index d5d9f856cb0a6..a6415ef83903c 100644
--- a/shell/browser/api/electron_api_web_frame_main.cc
+++ b/shell/browser/api/electron_api_web_frame_main.cc
@@ -317,6 +317,10 @@ void WebFrameMain::Connect() {
   }
 }
 
+void WebFrameMain::DOMContentLoaded() {
+  Emit("dom-ready");
+}
+
 // static
 gin::Handle<WebFrameMain> WebFrameMain::New(v8::Isolate* isolate) {
   return gin::Handle<WebFrameMain>();
diff --git a/shell/browser/api/electron_api_web_frame_main.h b/shell/browser/api/electron_api_web_frame_main.h
index fbea08085870d..114a53d40bf31 100644
--- a/shell/browser/api/electron_api_web_frame_main.h
+++ b/shell/browser/api/electron_api_web_frame_main.h
@@ -13,6 +13,7 @@
 #include "gin/handle.h"
 #include "gin/wrappable.h"
 #include "mojo/public/cpp/bindings/remote.h"
+#include "shell/browser/event_emitter_mixin.h"
 #include "shell/common/gin_helper/constructible.h"
 #include "shell/common/gin_helper/pinnable.h"
 #include "third_party/blink/public/mojom/page/page_visibility_state.mojom-forward.h"
@@ -35,6 +36,7 @@ class WebContents;
 
 // Bindings for accessing frames from the main process.
 class WebFrameMain : public gin::Wrappable<WebFrameMain>,
+                     public gin_helper::EventEmitterMixin<WebFrameMain>,
                      public gin_helper::Pinnable<WebFrameMain>,
                      public gin_helper::Constructible<WebFrameMain> {
  public:
@@ -80,7 +82,6 @@ class WebFrameMain : public gin::Wrappable<WebFrameMain>,
   // WebFrameMain can outlive its RenderFrameHost pointer so we need to check
   // whether its been disposed of prior to accessing it.
   bool CheckRenderFrame() const;
-  void Connect();
 
   v8::Local<v8::Promise> ExecuteJavaScript(gin::Arguments* args,
                                            const std::u16string& code);
@@ -108,6 +109,8 @@ class WebFrameMain : public gin::Wrappable<WebFrameMain>,
   std::vector<content::RenderFrameHost*> FramesInSubtree() const;
 
   void OnRendererConnectionError();
+  void Connect();
+  void DOMContentLoaded();
 
   mojo::Remote<mojom::ElectronRenderer> renderer_api_;
   mojo::PendingReceiver<mojom::ElectronRenderer> pending_receiver_;
diff --git a/shell/common/gin_converters/frame_converter.cc b/shell/common/gin_converters/frame_converter.cc
index 943a6214651a6..6be7d3b681c65 100644
--- a/shell/common/gin_converters/frame_converter.cc
+++ b/shell/common/gin_converters/frame_converter.cc
@@ -5,10 +5,18 @@
 #include "shell/common/gin_converters/frame_converter.h"
 
 #include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"
 #include "shell/browser/api/electron_api_web_frame_main.h"
+#include "shell/common/gin_helper/accessor.h"
 
 namespace gin {
 
+namespace {
+
+v8::Persistent<v8::ObjectTemplate> rfh_templ;
+
+}  // namespace
+
 // static
 v8::Local<v8::Value> Converter<content::RenderFrameHost*>::ToV8(
     v8::Isolate* isolate,
@@ -18,4 +26,64 @@ v8::Local<v8::Value> Converter<content::RenderFrameHost*>::ToV8(
   return electron::api::WebFrameMain::From(isolate, val).ToV8();
 }
 
+// static
+v8::Local<v8::Value>
+Converter<gin_helper::AccessorValue<content::RenderFrameHost*>>::ToV8(
+    v8::Isolate* isolate,
+    gin_helper::AccessorValue<content::RenderFrameHost*> val) {
+  content::RenderFrameHost* rfh = val.Value;
+  if (!rfh)
+    return v8::Null(isolate);
+
+  const int process_id = rfh->GetProcess()->GetID();
+  const int routing_id = rfh->GetRoutingID();
+
+  if (rfh_templ.IsEmpty()) {
+    v8::EscapableHandleScope inner(isolate);
+    v8::Local<v8::ObjectTemplate> local = v8::ObjectTemplate::New(isolate);
+    local->SetInternalFieldCount(2);
+    rfh_templ.Reset(isolate, inner.Escape(local));
+  }
+
+  v8::Local<v8::Object> rfh_obj =
+      v8::Local<v8::ObjectTemplate>::New(isolate, rfh_templ)
+          ->NewInstance(isolate->GetCurrentContext())
+          .ToLocalChecked();
+
+  rfh_obj->SetInternalField(0, v8::Number::New(isolate, process_id));
+  rfh_obj->SetInternalField(1, v8::Number::New(isolate, routing_id));
+
+  return rfh_obj;
+}
+
+// static
+bool Converter<gin_helper::AccessorValue<content::RenderFrameHost*>>::FromV8(
+    v8::Isolate* isolate,
+    v8::Local<v8::Value> val,
+    gin_helper::AccessorValue<content::RenderFrameHost*>* out) {
+  v8::Local<v8::Object> rfh_obj;
+  if (!ConvertFromV8(isolate, val, &rfh_obj))
+    return false;
+
+  if (rfh_obj->InternalFieldCount() != 2)
+    return false;
+
+  v8::Local<v8::Value> process_id_wrapper = rfh_obj->GetInternalField(0);
+  v8::Local<v8::Value> routing_id_wrapper = rfh_obj->GetInternalField(1);
+
+  if (process_id_wrapper.IsEmpty() || !process_id_wrapper->IsNumber() ||
+      routing_id_wrapper.IsEmpty() || !routing_id_wrapper->IsNumber())
+    return false;
+
+  const int process_id = process_id_wrapper.As<v8::Number>()->Value();
+  const int routing_id = routing_id_wrapper.As<v8::Number>()->Value();
+
+  auto* rfh = content::RenderFrameHost::FromID(process_id, routing_id);
+  if (!rfh)
+    return false;
+
+  out->Value = rfh;
+  return true;
+}
+
 }  // namespace gin
diff --git a/shell/common/gin_converters/frame_converter.h b/shell/common/gin_converters/frame_converter.h
index 4916054175e25..a2579ed6019b4 100644
--- a/shell/common/gin_converters/frame_converter.h
+++ b/shell/common/gin_converters/frame_converter.h
@@ -6,6 +6,7 @@
 #define SHELL_COMMON_GIN_CONVERTERS_FRAME_CONVERTER_H_
 
 #include "gin/converter.h"
+#include "shell/common/gin_helper/accessor.h"
 
 namespace content {
 class RenderFrameHost;
@@ -19,6 +20,16 @@ struct Converter<content::RenderFrameHost*> {
                                    content::RenderFrameHost* val);
 };
 
+template <>
+struct Converter<gin_helper::AccessorValue<content::RenderFrameHost*>> {
+  static v8::Local<v8::Value> ToV8(
+      v8::Isolate* isolate,
+      gin_helper::AccessorValue<content::RenderFrameHost*> val);
+  static bool FromV8(v8::Isolate* isolate,
+                     v8::Local<v8::Value> val,
+                     gin_helper::AccessorValue<content::RenderFrameHost*>* out);
+};
+
 }  // namespace gin
 
 #endif  // SHELL_COMMON_GIN_CONVERTERS_FRAME_CONVERTER_H_
diff --git a/shell/common/gin_helper/accessor.h b/shell/common/gin_helper/accessor.h
new file mode 100644
index 0000000000000..6d258a56d674e
--- /dev/null
+++ b/shell/common/gin_helper/accessor.h
@@ -0,0 +1,27 @@
+// Copyright (c) 2021 Samuel Maddock <sam@samuelmaddock.com>.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef SHELL_COMMON_GIN_HELPER_ACCESSOR_H_
+#define SHELL_COMMON_GIN_HELPER_ACCESSOR_H_
+
+namespace gin_helper {
+
+// Wrapper for a generic value to be used as an accessor in a
+// gin_helper::Dictionary.
+template <typename T>
+struct AccessorValue {
+  T Value;
+};
+template <typename T>
+struct AccessorValue<const T&> {
+  T Value;
+};
+template <typename T>
+struct AccessorValue<const T*> {
+  T* Value;
+};
+
+}  // namespace gin_helper
+
+#endif  // SHELL_COMMON_GIN_HELPER_ACCESSOR_H_
diff --git a/shell/common/gin_helper/dictionary.h b/shell/common/gin_helper/dictionary.h
index c5c3acc735157..5e55c418f2582 100644
--- a/shell/common/gin_helper/dictionary.h
+++ b/shell/common/gin_helper/dictionary.h
@@ -10,6 +10,7 @@
 
 #include "gin/dictionary.h"
 #include "shell/common/gin_converters/std_converter.h"
+#include "shell/common/gin_helper/accessor.h"
 #include "shell/common/gin_helper/function_template.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
 
@@ -109,6 +110,36 @@ class Dictionary : public gin::Dictionary {
         .ToChecked();
   }
 
+  template <typename K, typename V>
+  bool SetGetter(const K& key, const V& val) {
+    AccessorValue<V> acc_value;
+    acc_value.Value = val;
+
+    v8::Local<v8::Value> v8_value_accessor;
+    if (!gin::TryConvertToV8(isolate(), acc_value, &v8_value_accessor))
+      return false;
+
+    auto context = isolate()->GetCurrentContext();
+
+    return GetHandle()
+        ->SetAccessor(
+            context, gin::StringToV8(isolate(), key),
+            [](v8::Local<v8::Name> property_name,
+               const v8::PropertyCallbackInfo<v8::Value>& info) {
+              AccessorValue<V> acc_value;
+              if (!gin::ConvertFromV8(info.GetIsolate(), info.Data(),
+                                      &acc_value))
+                return;
+
+              V val = acc_value.Value;
+              v8::Local<v8::Value> v8_value;
+              if (gin::TryConvertToV8(info.GetIsolate(), val, &v8_value))
+                info.GetReturnValue().Set(v8_value);
+            },
+            NULL, v8_value_accessor)
+        .ToChecked();
+  }
+
   template <typename T>
   bool SetReadOnly(base::StringPiece key, const T& val) {
     v8::Local<v8::Value> v8_value;
diff --git a/spec-main/api-web-frame-main-spec.ts b/spec-main/api-web-frame-main-spec.ts
index 32885838423f7..94f3bd59827e3 100644
--- a/spec-main/api-web-frame-main-spec.ts
+++ b/spec-main/api-web-frame-main-spec.ts
@@ -244,4 +244,68 @@ describe('webFrameMain module', () => {
       }
     });
   });
+
+  describe('"frame-created" event', () => {
+    it('emits when the main frame is created', async () => {
+      const w = new BrowserWindow({ show: false });
+      const promise = emittedOnce(w.webContents, 'frame-created');
+      w.webContents.loadFile(path.join(subframesPath, 'frame.html'));
+      const [, details] = await promise;
+      expect(details.frame).to.equal(w.webContents.mainFrame);
+    });
+
+    it('emits when nested frames are created', async () => {
+      const w = new BrowserWindow({ show: false });
+      const promise = emittedNTimes(w.webContents, 'frame-created', 2);
+      w.webContents.loadFile(path.join(subframesPath, 'frame-container.html'));
+      const [[, mainDetails], [, nestedDetails]] = await promise;
+      expect(mainDetails.frame).to.equal(w.webContents.mainFrame);
+      expect(nestedDetails.frame).to.equal(w.webContents.mainFrame.frames[0]);
+    });
+
+    it('is not emitted upon cross-origin navigation', async () => {
+      const server = await createServer();
+
+      // HACK: Use 'localhost' instead of '127.0.0.1' so Chromium treats it as
+      // a separate origin because differing ports aren't enough 🤔
+      const secondUrl = `http://localhost:${new URL(server.url).port}`;
+
+      const w = new BrowserWindow({ show: false });
+      await w.webContents.loadURL(server.url);
+
+      let frameCreatedEmitted = false;
+
+      w.webContents.once('frame-created', () => {
+        frameCreatedEmitted = true;
+      });
+
+      await w.webContents.loadURL(secondUrl);
+
+      expect(frameCreatedEmitted).to.be.false();
+    });
+  });
+
+  describe('"dom-ready" event', () => {
+    it('emits for top-level frame', async () => {
+      const w = new BrowserWindow({ show: false });
+      const promise = emittedOnce(w.webContents.mainFrame, 'dom-ready');
+      w.webContents.loadURL('about:blank');
+      await promise;
+    });
+
+    it('emits for sub frame', async () => {
+      const w = new BrowserWindow({ show: false });
+      const promise = new Promise<void>(resolve => {
+        w.webContents.on('frame-created', (e, { frame }) => {
+          frame.on('dom-ready', () => {
+            if (frame.name === 'frameA') {
+              resolve();
+            }
+          });
+        });
+      });
+      w.webContents.loadFile(path.join(subframesPath, 'frame-with-frame.html'));
+      await promise;
+    });
+  });
 });
diff --git a/spec-main/fixtures/sub-frames/frame-container.html b/spec-main/fixtures/sub-frames/frame-container.html
index f731555a5ddaf..48e1e12150be4 100644
--- a/spec-main/fixtures/sub-frames/frame-container.html
+++ b/spec-main/fixtures/sub-frames/frame-container.html
@@ -8,6 +8,6 @@
 </head>
 <body>
   This is the root page
-  <iframe src="./frame.html"></iframe>
+  <iframe src="./frame.html" name="frameA"></iframe>
 </body>
 </html>
\ No newline at end of file
diff --git a/spec-main/fixtures/sub-frames/frame-with-frame.html b/spec-main/fixtures/sub-frames/frame-with-frame.html
index 9d99fef71b332..3f46a8adab9d3 100644
--- a/spec-main/fixtures/sub-frames/frame-with-frame.html
+++ b/spec-main/fixtures/sub-frames/frame-with-frame.html
@@ -8,6 +8,6 @@
 </head>
 <body>
   This is a frame, is has one child
-  <iframe src="./frame.html"></iframe>
+  <iframe src="./frame.html" name="frameA"></iframe>
 </body>
 </html>
\ No newline at end of file