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

feat: add WebFrameMain detached property (#43473)

Browse source 
* feat: add WebFrameMain detached property

fix: throw instead of returning null senderFrame

test: detached frames

fix: ensure IPCs of pending deletion RFHs are dispatched

fix: lookup WFM by FTN ID to dispatch IPCs

feat: add frame.isDestroyed()

return null

fix: return undefined

docs: add null to all frame properties

refactor: option c, return null and emit warning

refactor: add routingId & processId to navigation events

test: null frame property

docs: clarify warning message

better wording

clarify null frame

fix: browserwindow spec

* maybe fix 🤷

* fix: use updated util #43722

* docs: add notice for frame change of behavior

* docs: clarify why frame properties may be null

* lint

* wip

* fix: content::FrameTreeNodeId lookup and converter

* refactor: avoid holey array deoptimization

---------

Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>
This commit is contained in:
Sam Maddock 2024-10-11 18:33:53 -04:00 • committed by GitHub
parent 527efc01a4
commit 8b3d70a2a3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
20 changed files with 410 additions and 126 deletions
Download patch file Download diff file Expand all files Collapse all files

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

@ -1689,7 +1689,8 @@ 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::FromRenderFrameHost(new_host);
auto* web_frame =
WebFrameMain::FromFrameTreeNodeId(new_host->GetFrameTreeNodeId());
if (web_frame) {
web_frame->UpdateRenderFrameHost(new_host);
}
@ -1861,6 +1862,8 @@ bool WebContents::EmitNavigationEvent(
dict.Set("url", url);
dict.Set("isSameDocument", is_same_document);
dict.Set("isMainFrame", is_main_frame);
dict.Set("processId", frame_process_id);
dict.Set("routingId", frame_routing_id);
dict.SetGetter("frame", frame_host);
dict.SetGetter("initiator", initiator_frame_host);
@ -1980,8 +1983,10 @@ gin::Handle<gin_helper::internal::Event> WebContents::MakeEventWithSender(
dict.Set("_replyChannel",
ReplyChannel::Create(isolate, std::move(callback)));
if (frame) {
dict.SetGetter("senderFrame", frame);
dict.Set("frameId", frame->GetRoutingID());
dict.Set("processId", frame->GetProcess()->GetID());
dict.Set("frameTreeNodeId", frame->GetFrameTreeNodeId());
}
return event;
}

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

@ -33,6 +33,37 @@
#include "shell/common/node_includes.h"
#include "shell/common/v8_value_serializer.h"
namespace {
// RenderFrameHost (RFH) exists as a child of a FrameTreeNode. When a
// cross-origin navigation occurs, the FrameTreeNode swaps RFHs. After
// swapping, the old RFH will be marked for deletion and run any unload
// listeners. If an IPC is sent during an unload/beforeunload listener,
// it's possible that it arrives after the RFH swap and has been
// detached from the FrameTreeNode.
bool IsDetachedFrameHost(content::RenderFrameHost* rfh) {
if (!rfh)
return true;
// 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*>(rfh);
// During cross-origin navigation, a RFH may be swapped out of its
// FrameTreeNode with a new RFH. In these cases, it's marked for
// deletion. As this pending deletion RFH won't be following future
// swaps, we need to indicate that its been pinned.
return (rfh_impl->lifecycle_state() !=
content::RenderFrameHostImpl::LifecycleStateImpl::kSpeculative &&
rfh->GetLifecycleState() ==
content::RenderFrameHost::LifecycleState::kPendingDeletion);
}
} // namespace
namespace gin {
template <>
@ -57,45 +88,64 @@ struct Converter<blink::mojom::PageVisibilityState> {
namespace electron::api {
// FrameTreeNodeId -> WebFrameMain*
// Using FrameTreeNode allows us to track frame across navigations. This
// is most similar to how <iframe> works.
typedef std::unordered_map<content::FrameTreeNodeId,
WebFrameMain*,
content::FrameTreeNodeId::Hasher>
WebFrameMainIdMap;
FrameTreeNodeIdMap;
WebFrameMainIdMap& GetWebFrameMainMap() {
static base::NoDestructor<WebFrameMainIdMap> instance;
// Token -> WebFrameMain*
// Maps exact RFH to a WebFrameMain instance.
typedef std::map<content::GlobalRenderFrameHostToken, WebFrameMain*>
FrameTokenMap;
FrameTreeNodeIdMap& GetFrameTreeNodeIdMap() {
static base::NoDestructor<FrameTreeNodeIdMap> instance;
return *instance;
}
FrameTokenMap& GetFrameTokenMap() {
static base::NoDestructor<FrameTokenMap> instance;
return *instance;
}
// static
WebFrameMain* WebFrameMain::FromFrameTreeNodeId(
content::FrameTreeNodeId frame_tree_node_id) {
WebFrameMainIdMap& frame_map = GetWebFrameMainMap();
// Pinned frames aren't tracked across navigations so only non-pinned
// frames will be retrieved.
FrameTreeNodeIdMap& frame_map = GetFrameTreeNodeIdMap();
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::FromFrameToken(
content::GlobalRenderFrameHostToken frame_token) {
FrameTokenMap& frame_map = GetFrameTokenMap();
auto iter = frame_map.find(frame_token);
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());
return FromFrameToken(rfh->GetGlobalFrameToken());
}
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);
: frame_tree_node_id_(rfh->GetFrameTreeNodeId()),
frame_token_(rfh->GetGlobalFrameToken()),
render_frame_(rfh),
render_frame_detached_(IsDetachedFrameHost(rfh)) {
GetFrameTreeNodeIdMap().emplace(frame_tree_node_id_, this);
GetFrameTokenMap().emplace(frame_token_, this);
}
WebFrameMain::~WebFrameMain() {
@ -104,18 +154,24 @@ WebFrameMain::~WebFrameMain() {
void WebFrameMain::Destroyed() {
MarkRenderFrameDisposed();
GetWebFrameMainMap().erase(frame_tree_node_id_);
GetFrameTreeNodeIdMap().erase(frame_tree_node_id_);
GetFrameTokenMap().erase(frame_token_);
Unpin();
}
void WebFrameMain::MarkRenderFrameDisposed() {
render_frame_ = nullptr;
render_frame_detached_ = true;
render_frame_disposed_ = true;
TeardownMojoConnection();
}
// Should only be called when swapping frames.
void WebFrameMain::UpdateRenderFrameHost(content::RenderFrameHost* rfh) {
// Should only be called when swapping frames.
GetFrameTokenMap().erase(frame_token_);
frame_token_ = rfh->GetGlobalFrameToken();
GetFrameTokenMap().emplace(frame_token_, this);
render_frame_disposed_ = false;
render_frame_ = rfh;
TeardownMojoConnection();
@ -186,6 +242,10 @@ bool WebFrameMain::Reload() {
return render_frame_->Reload();
}
bool WebFrameMain::IsDestroyed() const {
return render_frame_disposed_;
}
void WebFrameMain::Send(v8::Isolate* isolate,
bool internal,
const std::string& channel,
@ -275,8 +335,12 @@ void WebFrameMain::PostMessage(v8::Isolate* isolate,
std::move(transferable_message));
}
int WebFrameMain::FrameTreeNodeIDAsInt() const {
return frame_tree_node_id_.value();
bool WebFrameMain::Detached() const {
return render_frame_detached_;
}
content::FrameTreeNodeId WebFrameMain::FrameTreeNodeID() const {
return frame_tree_node_id_;
}
std::string WebFrameMain::Name() const {
@ -389,29 +453,17 @@ gin::Handle<WebFrameMain> WebFrameMain::From(v8::Isolate* isolate,
return handle;
}
// static
gin::Handle<WebFrameMain> WebFrameMain::FromOrNull(
v8::Isolate* isolate,
content::RenderFrameHost* rfh) {
if (!rfh)
return gin::Handle<WebFrameMain>();
auto* web_frame = FromRenderFrameHost(rfh);
if (!web_frame)
return gin::Handle<WebFrameMain>();
return gin::CreateHandle(isolate, web_frame);
}
// static
void WebFrameMain::FillObjectTemplate(v8::Isolate* isolate,
v8::Local<v8::ObjectTemplate> templ) {
gin_helper::ObjectTemplateBuilder(isolate, templ)
.SetMethod("executeJavaScript", &WebFrameMain::ExecuteJavaScript)
.SetMethod("reload", &WebFrameMain::Reload)
.SetMethod("isDestroyed", &WebFrameMain::IsDestroyed)
.SetMethod("_send", &WebFrameMain::Send)
.SetMethod("_postMessage", &WebFrameMain::PostMessage)
.SetProperty("frameTreeNodeId", &WebFrameMain::FrameTreeNodeIDAsInt)
.SetProperty("detached", &WebFrameMain::Detached)
.SetProperty("frameTreeNodeId", &WebFrameMain::FrameTreeNodeID)
.SetProperty("name", &WebFrameMain::Name)
.SetProperty("osProcessId", &WebFrameMain::OSProcessID)
.SetProperty("processId", &WebFrameMain::ProcessID)
@ -446,22 +498,38 @@ v8::Local<v8::Value> FromID(gin_helper::ErrorThrower thrower,
auto* rfh =
content::RenderFrameHost::FromID(render_process_id, render_frame_id);
if (!rfh)
return v8::Undefined(thrower.isolate());
return WebFrameMain::From(thrower.isolate(), rfh).ToV8();
}
v8::Local<v8::Value> FromIDOrNull(gin_helper::ErrorThrower thrower,
int render_process_id,
int render_frame_id) {
v8::Local<v8::Value> FromIdIfExists(gin_helper::ErrorThrower thrower,
int render_process_id,
int render_frame_id) {
if (!electron::Browser::Get()->is_ready()) {
thrower.ThrowError("WebFrameMain is available only after app ready");
return v8::Null(thrower.isolate());
}
auto* rfh =
content::RenderFrameHost* rfh =
content::RenderFrameHost::FromID(render_process_id, render_frame_id);
WebFrameMain* web_frame = WebFrameMain::FromRenderFrameHost(rfh);
if (!web_frame)
return v8::Null(thrower.isolate());
return gin::CreateHandle(thrower.isolate(), web_frame).ToV8();
}
return WebFrameMain::FromOrNull(thrower.isolate(), rfh).ToV8();
v8::Local<v8::Value> FromFtnIdIfExists(gin_helper::ErrorThrower thrower,
int frame_tree_node_id) {
if (!electron::Browser::Get()->is_ready()) {
thrower.ThrowError("WebFrameMain is available only after app ready");
return v8::Null(thrower.isolate());
}
WebFrameMain* web_frame = WebFrameMain::FromFrameTreeNodeId(
content::FrameTreeNodeId(frame_tree_node_id));
if (!web_frame)
return v8::Null(thrower.isolate());
return gin::CreateHandle(thrower.isolate(), web_frame).ToV8();
}
void Initialize(v8::Local<v8::Object> exports,
@ -472,7 +540,8 @@ void Initialize(v8::Local<v8::Object> exports,
gin_helper::Dictionary dict(isolate, exports);
dict.Set("WebFrameMain", WebFrameMain::GetConstructor(context));
dict.SetMethod("fromId", &FromID);
dict.SetMethod("fromIdOrNull", &FromIDOrNull);
dict.SetMethod("_fromIdIfExists", &FromIdIfExists);
dict.SetMethod("_fromFtnIdIfExists", &FromFtnIdIfExists);
}
} // namespace

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

@ -13,6 +13,7 @@
#include "base/memory/weak_ptr.h"
#include "base/process/process.h"
#include "content/public/browser/frame_tree_node_id.h"
#include "content/public/browser/global_routing_id.h"
#include "gin/wrappable.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/remote.h"
@ -51,11 +52,10 @@ class WebFrameMain final : public gin::Wrappable<WebFrameMain>,
static gin::Handle<WebFrameMain> From(
v8::Isolate* isolate,
content::RenderFrameHost* render_frame_host);
static gin::Handle<WebFrameMain> FromOrNull(
v8::Isolate* isolate,
content::RenderFrameHost* render_frame_host);
static WebFrameMain* FromFrameTreeNodeId(
content::FrameTreeNodeId frame_tree_node_id);
static WebFrameMain* FromFrameToken(
content::GlobalRenderFrameHostToken frame_token);
static WebFrameMain* FromRenderFrameHost(
content::RenderFrameHost* render_frame_host);
@ -103,6 +103,7 @@ class WebFrameMain final : public gin::Wrappable<WebFrameMain>,
v8::Local<v8::Promise> ExecuteJavaScript(gin::Arguments* args,
const std::u16string& code);
bool Reload();
bool IsDestroyed() const;
void Send(v8::Isolate* isolate,
bool internal,
const std::string& channel,
@ -112,7 +113,8 @@ class WebFrameMain final : public gin::Wrappable<WebFrameMain>,
v8::Local<v8::Value> message_value,
std::optional<v8::Local<v8::Value>> transfer);
int FrameTreeNodeIDAsInt() const;
bool Detached() const;
content::FrameTreeNodeId FrameTreeNodeID() const;
std::string Name() const;
base::ProcessId OSProcessID() const;
int ProcessID() const;
@ -132,6 +134,7 @@ class WebFrameMain final : public gin::Wrappable<WebFrameMain>,
mojo::PendingReceiver<mojom::ElectronRenderer> pending_receiver_;
content::FrameTreeNodeId frame_tree_node_id_;
content::GlobalRenderFrameHostToken frame_token_;
raw_ptr<content::RenderFrameHost> render_frame_ = nullptr;
@ -139,6 +142,10 @@ class WebFrameMain final : public gin::Wrappable<WebFrameMain>,
// be accessed.
bool render_frame_disposed_ = false;
// Whether the content::RenderFrameHost is detached from the frame
// tree. This can occur while it's running unload handlers.
bool render_frame_detached_;
base::WeakPtrFactory<WebFrameMain> weak_factory_{this};
};

21
shell/common/gin_converters/frame_converter.cc
View file

@ -8,6 +8,7 @@
#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"
#include "shell/common/node_util.h"
namespace gin {
@ -17,6 +18,13 @@ v8::Persistent<v8::ObjectTemplate> rfh_templ;
} // namespace
// static
v8::Local<v8::Value> Converter<content::FrameTreeNodeId>::ToV8(
v8::Isolate* isolate,
const content::FrameTreeNodeId& val) {
return v8::Number::New(isolate, val.value());
}
// static
v8::Local<v8::Value> Converter<content::RenderFrameHost*>::ToV8(
v8::Isolate* isolate,
@ -94,8 +102,17 @@ bool Converter<gin_helper::AccessorValue<content::RenderFrameHost*>>::FromV8(
const int routing_id = routing_id_wrapper.As<v8::Number>()->Value();
auto* rfh = content::RenderFrameHost::FromID(process_id, routing_id);
if (!rfh)
return false;
if (!rfh) {
// Lazily evaluted property accessed after RFH has been destroyed.
// Continue to return nullptr, but emit warning to inform developers
// what occurred.
electron::util::EmitWarning(
isolate,
"Frame property was accessed after it navigated or was destroyed. "
"Avoid asynchronous tasks prior to indexing.",
"electron");
}
out->Value = rfh;
return true;

8
shell/common/gin_converters/frame_converter.h
View file

@ -5,15 +5,23 @@
#ifndef ELECTRON_SHELL_COMMON_GIN_CONVERTERS_FRAME_CONVERTER_H_
#define ELECTRON_SHELL_COMMON_GIN_CONVERTERS_FRAME_CONVERTER_H_
#include "content/public/browser/frame_tree_node_id.h"
#include "gin/converter.h"
#include "shell/common/gin_helper/accessor.h"
namespace content {
class RenderFrameHost;
}
namespace gin {
template <>
struct Converter<content::FrameTreeNodeId> {
static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
const content::FrameTreeNodeId& val);
};
template <>
struct Converter<content::RenderFrameHost*> {
static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,