mirrors/electron
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: cross-origin navigation disposing WebFrameMain instances (#30076)

Browse source
This commit is contained in:
Samuel Maddock 2021-08-18 14:23:41 -04:00 committed by GitHub
parent 90b5ba3bed
commit dd16d68e96
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 150 additions and 96 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

51
shell/browser/api/electron_api_web_contents.cc
View file

@ -1384,7 +1384,9 @@ void WebContents::HandleNewRenderFrame(
if (rwh_impl)
rwh_impl->disable_hidden_ = !background_throttling_;
WebFrameMain::RenderFrameCreated(render_frame_host);
auto* web_frame = WebFrameMain::FromRenderFrameHost(render_frame_host);
if (web_frame)
web_frame->Connect();
}
void WebContents::RenderFrameCreated(
@ -1392,6 +1394,46 @@ void WebContents::RenderFrameCreated(
HandleNewRenderFrame(render_frame_host);
}
void WebContents::RenderFrameDeleted(
content::RenderFrameHost* render_frame_host) {
// A RenderFrameHost can be deleted when:
// - A WebContents is removed and its containing frames are disposed.
// - An <iframe> is removed from the DOM.
// - Cross-origin navigation creates a new RFH in a separate process which
// is swapped by content::RenderFrameHostManager.
//
// WebFrameMain::FromRenderFrameHost(rfh) will use the RFH's FrameTreeNode ID
// to find an existing instance of WebFrameMain. During a cross-origin
// navigation, the deleted RFH will be the old host which was swapped out. In
// this special case, we need to also ensure that WebFrameMain's internal RFH
// matches before marking it as disposed.
auto* web_frame = WebFrameMain::FromRenderFrameHost(render_frame_host);
if (web_frame && web_frame->render_frame_host() == render_frame_host)
web_frame->MarkRenderFrameDisposed();
}
void WebContents::RenderFrameHostChanged(content::RenderFrameHost* old_host,
content::RenderFrameHost* new_host) {
// 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 in 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);
}
}
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) {
// This event is necessary for tracking any states with respect to
// intermediate render view hosts aka speculative render view hosts. Currently
@ -1631,13 +1673,6 @@ void WebContents::UpdateDraggableRegions(
observer.OnDraggableRegionsUpdated(regions);
}
void WebContents::RenderFrameDeleted(
content::RenderFrameHost* render_frame_host) {
// A WebFrameMain can outlive its RenderFrameHost so we need to mark it as
// disposed to prevent access to it.
WebFrameMain::RenderFrameDeleted(render_frame_host);
}
void WebContents::DidStartNavigation(
content::NavigationHandle* navigation_handle) {
EmitNavigationEvent("did-start-navigation", navigation_handle);

5
shell/browser/api/electron_api_web_contents.h
View file

@ -543,9 +543,12 @@ class WebContents : public gin::Wrappable<WebContents>,
void BeforeUnloadFired(bool proceed,
const base::TimeTicks& proceed_time) override;
void RenderFrameCreated(content::RenderFrameHost* render_frame_host) override;
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 RenderProcessGone(base::TerminationStatus status) override;
void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) override;
void DOMContentLoaded(content::RenderFrameHost* render_frame_host) override;
void DidFinishLoad(content::RenderFrameHost* render_frame_host,
const GURL& validated_url) override;

93
shell/browser/api/electron_api_web_frame_main.cc
View file

@ -9,8 +9,8 @@
#include <utility>
#include <vector>
#include "base/lazy_instance.h"
#include "base/logging.h"
#include "base/no_destructor.h"
#include "content/browser/renderer_host/frame_tree_node.h" // nogncheck
#include "content/public/browser/render_frame_host.h"
#include "electron/shell/common/api/api.mojom.h"
@ -56,36 +56,54 @@ namespace electron {
namespace api {
typedef std::unordered_map<content::RenderFrameHost*, WebFrameMain*>
RenderFrameMap;
base::LazyInstance<RenderFrameMap>::DestructorAtExit g_render_frame_map =
LAZY_INSTANCE_INITIALIZER;
typedef std::unordered_map<int, WebFrameMain*> WebFrameMainIdMap;
WebFrameMain* FromRenderFrameHost(content::RenderFrameHost* rfh) {
auto frame_map = g_render_frame_map.Get();
auto iter = frame_map.find(rfh);
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) {
return rfh ? FromFrameTreeNodeId(rfh->GetFrameTreeNodeId()) : nullptr;
}
gin::WrapperInfo WebFrameMain::kWrapperInfo = {gin::kEmbedderNativeGin};
WebFrameMain::WebFrameMain(content::RenderFrameHost* rfh) : render_frame_(rfh) {
g_render_frame_map.Get().emplace(rfh, this);
WebFrameMain::WebFrameMain(content::RenderFrameHost* rfh)
: frame_tree_node_id_(rfh->GetFrameTreeNodeId()), render_frame_(rfh) {
GetWebFrameMainMap().emplace(frame_tree_node_id_, this);
}
WebFrameMain::~WebFrameMain() {
Destroyed();
}
void WebFrameMain::Destroyed() {
MarkRenderFrameDisposed();
GetWebFrameMainMap().erase(frame_tree_node_id_);
Unpin();
}
void WebFrameMain::MarkRenderFrameDisposed() {
if (render_frame_disposed_)
return;
Unpin();
g_render_frame_map.Get().erase(render_frame_);
render_frame_ = nullptr;
render_frame_disposed_ = true;
}
void WebFrameMain::UpdateRenderFrameHost(content::RenderFrameHost* rfh) {
// Should only be called when swapping frames.
DCHECK(render_frame_);
render_frame_ = rfh;
}
bool WebFrameMain::CheckRenderFrame() const {
if (render_frame_disposed_) {
v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
@ -213,9 +231,7 @@ void WebFrameMain::PostMessage(v8::Isolate* isolate,
}
int WebFrameMain::FrameTreeNodeID() const {
if (!CheckRenderFrame())
return -1;
return render_frame_->GetFrameTreeNodeId();
return frame_tree_node_id_;
}
std::string WebFrameMain::Name() const {
@ -293,6 +309,13 @@ std::vector<content::RenderFrameHost*> WebFrameMain::FramesInSubtree() const {
return frame_hosts;
}
void WebFrameMain::Connect() {
if (pending_receiver_) {
render_frame_->GetRemoteInterfaces()->GetInterface(
std::move(pending_receiver_));
}
}
// static
gin::Handle<WebFrameMain> WebFrameMain::New(v8::Isolate* isolate) {
return gin::Handle<WebFrameMain>();
@ -315,35 +338,6 @@ gin::Handle<WebFrameMain> WebFrameMain::From(v8::Isolate* isolate,
return handle;
}
// static
gin::Handle<WebFrameMain> WebFrameMain::FromID(v8::Isolate* isolate,
int render_process_id,
int render_frame_id) {
auto* rfh =
content::RenderFrameHost::FromID(render_process_id, render_frame_id);
return From(isolate, rfh);
}
// static
void WebFrameMain::RenderFrameDeleted(content::RenderFrameHost* rfh) {
auto* web_frame = FromRenderFrameHost(rfh);
if (web_frame)
web_frame->MarkRenderFrameDisposed();
}
void WebFrameMain::RenderFrameCreated(content::RenderFrameHost* rfh) {
auto* web_frame = FromRenderFrameHost(rfh);
if (web_frame)
web_frame->Connect();
}
void WebFrameMain::Connect() {
if (pending_receiver_) {
render_frame_->GetRemoteInterfaces()->GetInterface(
std::move(pending_receiver_));
}
}
// static
v8::Local<v8::ObjectTemplate> WebFrameMain::FillObjectTemplate(
v8::Isolate* isolate,
@ -387,9 +381,10 @@ v8::Local<v8::Value> FromID(gin_helper::ErrorThrower thrower,
return v8::Null(thrower.isolate());
}
return WebFrameMain::FromID(thrower.isolate(), render_process_id,
render_frame_id)
.ToV8();
auto* rfh =
content::RenderFrameHost::FromID(render_process_id, render_frame_id);
return WebFrameMain::From(thrower.isolate(), rfh).ToV8();
}
void Initialize(v8::Local<v8::Object> exports,

38
shell/browser/api/electron_api_web_frame_main.h
View file

@ -31,6 +31,8 @@ namespace electron {
namespace api {
class WebContents;
// Bindings for accessing frames from the main process.
class WebFrameMain : public gin::Wrappable<WebFrameMain>,
public gin_helper::Pinnable<WebFrameMain>,
@ -39,23 +41,12 @@ class WebFrameMain : public gin::Wrappable<WebFrameMain>,
// Create a new WebFrameMain and return the V8 wrapper of it.
static gin::Handle<WebFrameMain> New(v8::Isolate* isolate);
static gin::Handle<WebFrameMain> FromID(v8::Isolate* isolate,
int render_process_id,
int render_frame_id);
static gin::Handle<WebFrameMain> From(
v8::Isolate* isolate,
content::RenderFrameHost* render_frame_host);
// Called to mark any RenderFrameHost as disposed by any WebFrameMain that
// may be holding a weak reference.
static void RenderFrameDeleted(content::RenderFrameHost* rfh);
static void RenderFrameCreated(content::RenderFrameHost* rfh);
// Mark RenderFrameHost as disposed and to no longer access it. This can
// occur upon frame navigation.
void MarkRenderFrameDisposed();
const mojo::Remote<mojom::ElectronRenderer>& GetRendererApi();
static WebFrameMain* FromFrameTreeNodeId(int frame_tree_node_id);
static WebFrameMain* FromRenderFrameHost(
content::RenderFrameHost* render_frame_host);
// gin::Wrappable
static gin::WrapperInfo kWrapperInfo;
@ -64,11 +55,28 @@ class WebFrameMain : public gin::Wrappable<WebFrameMain>,
v8::Local<v8::ObjectTemplate>);
const char* GetTypeName() override;
content::RenderFrameHost* render_frame_host() const { return render_frame_; }
protected:
explicit WebFrameMain(content::RenderFrameHost* render_frame);
~WebFrameMain() override;
private:
friend class WebContents;
// Called when FrameTreeNode is deleted.
void Destroyed();
// Mark RenderFrameHost as disposed and to no longer access it. This can
// happen when the WebFrameMain v8 handle is GC'd or when a FrameTreeNode
// is removed.
void MarkRenderFrameDisposed();
// Swap out the internal RFH when cross-origin navigation occurs.
void UpdateRenderFrameHost(content::RenderFrameHost* rfh);
const mojo::Remote<mojom::ElectronRenderer>& GetRendererApi();
// WebFrameMain can outlive its RenderFrameHost pointer so we need to check
// whether its been disposed of prior to accessing it.
bool CheckRenderFrame() const;
@ -104,6 +112,8 @@ class WebFrameMain : public gin::Wrappable<WebFrameMain>,
mojo::Remote<mojom::ElectronRenderer> renderer_api_;
mojo::PendingReceiver<mojom::ElectronRenderer> pending_receiver_;
int frame_tree_node_id_;
content::RenderFrameHost* render_frame_ = nullptr;
// Whether the RenderFrameHost has been removed and that it should no longer

59
spec-main/api-web-frame-main-spec.ts
View file

@ -14,6 +14,24 @@ describe('webFrameMain module', () => {
const fileUrl = (filename: string) => url.pathToFileURL(path.join(subframesPath, filename)).href;
type Server = { server: http.Server, url: string }
/** Creates an HTTP server whose handler embeds the given iframe src. */
const createServer = () => new Promise<Server>(resolve => {
const server = http.createServer((req, res) => {
const params = new URLSearchParams(url.parse(req.url || '').search || '');
if (params.has('frameSrc')) {
res.end(`<iframe src="${params.get('frameSrc')}"></iframe>`);
} else {
res.end('');
}
});
server.listen(0, '127.0.0.1', () => {
const url = `http://127.0.0.1:${(server.address() as AddressInfo).port}/`;
resolve({ server, url });
});
});
afterEach(closeAllWindows);
describe('WebFrame traversal APIs', () => {
@ -70,24 +88,6 @@ describe('webFrameMain module', () => {
});
describe('cross-origin', () => {
type Server = { server: http.Server, url: string }
/** Creates an HTTP server whose handler embeds the given iframe src. */
const createServer = () => new Promise<Server>(resolve => {
const server = http.createServer((req, res) => {
const params = new URLSearchParams(url.parse(req.url || '').search || '');
if (params.has('frameSrc')) {
res.end(`<iframe src="${params.get('frameSrc')}"></iframe>`);
} else {
res.end('');
}
});
server.listen(0, '127.0.0.1', () => {
const url = `http://127.0.0.1:${(server.address() as AddressInfo).port}/`;
resolve({ server, url });
});
});
let serverA = null as unknown as Server;
let serverB = null as unknown as Server;
@ -194,21 +194,32 @@ describe('webFrameMain module', () => {
});
});
describe('disposed WebFrames', () => {
describe('RenderFrame lifespan', () => {
let w: BrowserWindow;
let webFrame: WebFrameMain;
before(async () => {
beforeEach(async () => {
w = new BrowserWindow({ show: false, webPreferences: { contextIsolation: true } });
});
it('throws upon accessing properties when disposed', async () => {
await w.loadFile(path.join(subframesPath, 'frame-with-frame-container.html'));
webFrame = w.webContents.mainFrame;
const { mainFrame } = w.webContents;
w.destroy();
// Wait for WebContents, and thus RenderFrameHost, to be destroyed.
await new Promise(resolve => setTimeout(resolve, 0));
expect(() => mainFrame.url).to.throw();
});
it('throws upon accessing properties', () => {
expect(() => webFrame.url).to.throw();
it('persists through cross-origin navigation', async () => {
const server = await createServer();
// 'localhost' is treated as a separate origin.
const crossOriginUrl = server.url.replace('127.0.0.1', 'localhost');
await w.loadURL(server.url);
const { mainFrame } = w.webContents;
expect(mainFrame.url).to.equal(server.url);
await w.loadURL(crossOriginUrl);
expect(w.webContents.mainFrame).to.equal(mainFrame);
expect(mainFrame.url).to.equal(crossOriginUrl);
});
});