8000 fix: cross-origin navigation disposing WebFrameMain instances by samuelmaddock · Pull Request #30076 · electron/electron · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
8000

fix: cross-origin navigation disposing WebFrameMain instances #30076

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 18, 2021
Merged

fix: cross-origin navigation disposing WebFrameMain instances #30076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b528222
refactor: call methods directly from electron:WebContents
samuelmaddock Jun 16, 2021
8c522db
fix: cross-origin navigation disposing WebFrameMain instances
samuelmaddock Jul 11, 2021
3ffcbb7
fix: move call to WFM::Connect
samuelmaddock Aug 11, 2021
76331bb
refactor: use base::NoDestructor<T>
samuelmaddock Aug 11, 2021
323c07b
Update shell/browser/api/electron_api_web_frame_main.cc
samuelmaddock Aug 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 43 additions & 8 deletions shell/browser/api/electron_api_web_contents.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1384,14 +1384,56 @@ 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(
content::RenderFrameHost* render_frame_host) {
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
Expand Down Expand Up @@ -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< 10000 /td>
// disposed to prevent access to it.
WebFrameMain::RenderFrameDeleted(render_frame_host);
}

void WebContents::DidStartNavigation(
content::NavigationHandle* navigation_handle) {
EmitNavigationEvent("did-start-navigation", navigation_handle);
Expand Down
5 changes: 4 additions & 1 deletion shell/browser/api/electron_api_web_contents.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
93 changes: 44 additions & 49 deletions shell/browser/api/electron_api_web_frame_main.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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>();
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
38 changes: 24 additions & 14 deletions shell/browser/api/electron_api_web_frame_main.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down
Loading
0