Browse Source

feat: WebFrameMain no longer follows cross-origin navigations

Jeremy Rose 1 year ago
parent
commit
b824564dad

+ 0 - 17
shell/browser/api/electron_api_web_contents.cc

@@ -1753,23 +1753,6 @@ void WebContents::RenderFrameHostChanged(content::RenderFrameHost* old_host,
     if (new_host)
       new_host->GetRenderWidgetHost()->AddInputEventObserver(this);
   }
-
-  // During cross-origin navigation, a FrameTreeNode will swap out its RFH.
-  // If an instance of WebFrameMain exists, it will need to have its RFH
-  // swapped as well.
-  //
-  // |old_host| can be a nullptr so we use |new_host| for looking up the
-  // WebFrameMain instance.
-  auto* web_frame = WebFrameMain::FromRenderFrameHost(new_host);
-  if (web_frame) {
-    web_frame->UpdateRenderFrameHost(new_host);
-  }
-}
-
-void WebContents::FrameDeleted(int frame_tree_node_id) {
-  auto* web_frame = WebFrameMain::FromFrameTreeNodeId(frame_tree_node_id);
-  if (web_frame)
-    web_frame->Destroyed();
 }
 
 void WebContents::RenderViewDeleted(content::RenderViewHost* render_view_host) {

+ 0 - 1
shell/browser/api/electron_api_web_contents.h

@@ -628,7 +628,6 @@ class WebContents : public ExclusiveAccessContext,
   void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) override;
   void RenderFrameHostChanged(content::RenderFrameHost* old_host,
                               content::RenderFrameHost* new_host) override;
-  void FrameDeleted(int frame_tree_node_id) override;
   void RenderViewDeleted(content::RenderViewHost*) override;
   void PrimaryMainFrameRenderProcessGone(
       base::TerminationStatus status) override;

+ 12 - 21
shell/browser/api/electron_api_web_frame_main.cc

@@ -55,41 +55,32 @@ struct Converter<blink::mojom::PageVisibilityState> {
 
 namespace electron::api {
 
-typedef std::unordered_map<int, WebFrameMain*> WebFrameMainIdMap;
+typedef std::unordered_map<blink::LocalFrameToken,
+                           WebFrameMain*,
+                           blink::LocalFrameToken::Hasher>
+    WebFrameMainIdMap;
 
 WebFrameMainIdMap& GetWebFrameMainMap() {
   static base::NoDestructor<WebFrameMainIdMap> instance;
   return *instance;
 }
 
-// static
-WebFrameMain* WebFrameMain::FromFrameTreeNodeId(int frame_tree_node_id) {
-  WebFrameMainIdMap& frame_map = GetWebFrameMainMap();
-  auto iter = frame_map.find(frame_tree_node_id);
-  auto* web_frame = iter == frame_map.end() ? nullptr : iter->second;
-  return web_frame;
-}
-
 // static
 WebFrameMain* WebFrameMain::FromRenderFrameHost(content::RenderFrameHost* rfh) {
   if (!rfh)
     return nullptr;
 
-  // TODO(codebytere): remove after refactoring away from FrameTreeNodeId as map
-  // key.
-  auto* ftn =
-      static_cast<content::RenderFrameHostImpl*>(rfh)->frame_tree_node();
-  if (!ftn)
-    return nullptr;
-
-  return FromFrameTreeNodeId(rfh->GetFrameTreeNodeId());
+  WebFrameMainIdMap& frame_map = GetWebFrameMainMap();
+  auto iter = frame_map.find(rfh->GetGlobalFrameToken().frame_token);
+  auto* web_frame = iter == frame_map.end() ? nullptr : iter->second;
+  return web_frame;
 }
 
 gin::WrapperInfo WebFrameMain::kWrapperInfo = {gin::kEmbedderNativeGin};
 
 WebFrameMain::WebFrameMain(content::RenderFrameHost* rfh)
-    : frame_tree_node_id_(rfh->GetFrameTreeNodeId()), render_frame_(rfh) {
-  GetWebFrameMainMap().emplace(frame_tree_node_id_, this);
+    : rfh_token_(rfh->GetGlobalFrameToken()), render_frame_(rfh) {
+  GetWebFrameMainMap().emplace(rfh_token_.frame_token, this);
 }
 
 WebFrameMain::~WebFrameMain() {
@@ -98,7 +89,7 @@ WebFrameMain::~WebFrameMain() {
 
 void WebFrameMain::Destroyed() {
   MarkRenderFrameDisposed();
-  GetWebFrameMainMap().erase(frame_tree_node_id_);
+  GetWebFrameMainMap().erase(rfh_token_.frame_token);
   Unpin();
 }
 
@@ -270,7 +261,7 @@ void WebFrameMain::PostMessage(v8::Isolate* isolate,
 }
 
 int WebFrameMain::FrameTreeNodeID() const {
-  return frame_tree_node_id_;
+  return 0;
 }
 
 std::string WebFrameMain::Name() const {

+ 2 - 2
shell/browser/api/electron_api_web_frame_main.h

@@ -12,6 +12,7 @@
 #include "base/memory/raw_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/process/process.h"
+#include "content/public/browser/global_routing_id.h"
 #include "gin/handle.h"
 #include "gin/wrappable.h"
 #include "mojo/public/cpp/bindings/remote.h"
@@ -49,7 +50,6 @@ class WebFrameMain : public gin::Wrappable<WebFrameMain>,
   static gin::Handle<WebFrameMain> FromOrNull(
       v8::Isolate* isolate,
       content::RenderFrameHost* render_frame_host);
-  static WebFrameMain* FromFrameTreeNodeId(int frame_tree_node_id);
   static WebFrameMain* FromRenderFrameHost(
       content::RenderFrameHost* render_frame_host);
 
@@ -125,7 +125,7 @@ class WebFrameMain : public gin::Wrappable<WebFrameMain>,
   mojo::Remote<mojom::ElectronRenderer> renderer_api_;
   mojo::PendingReceiver<mojom::ElectronRenderer> pending_receiver_;
 
-  int frame_tree_node_id_;
+  content::GlobalRenderFrameHostToken rfh_token_;
 
   raw_ptr<content::RenderFrameHost> render_frame_ = nullptr;
 

+ 15 - 0
spec/api-ipc-spec.ts

@@ -198,6 +198,21 @@ describe('ipc module', () => {
       expect(received).to.have.lengthOf(1000);
       expect(received).to.deep.equal([...received].sort((a, b) => a - b));
     });
+
+    it('handles navigation', async () => {
+      const received = new Set<string>();
+      ipcMain.on('test', (e) => { received.add(e.senderFrame.url); });
+      const w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true, contextIsolation: false } });
+      // This is testing a race condition between navigation and IPC.
+      // It's not quite deterministic so repeat a few times to get good signal.
+      for (let i = 0; i < 10; i++) {
+        w.loadFile(path.resolve(fixturesPath, 'pages', 'ipc-after-navigate.html'));
+        await once(w.webContents, 'did-navigate');
+        const expectedUrl = w.getURL();
+        await once(w.webContents, 'did-stop-loading');
+        expect([...received.values()]).to.deep.equal([expectedUrl]);
+      }
+    });
   });
 
   describe('MessagePort', () => {

+ 9 - 0
spec/fixtures/pages/ipc-after-navigate.html

@@ -0,0 +1,9 @@
+<script>
+const { ipcRenderer } = require('electron')
+window.onload = () => {
+  setInterval(() => {
+    ipcRenderer.send('test')
+  }, 0);
+  window.location.replace("https://example.com");
+}
+</script>