fix: account for potentially swapped FrameTreeNodeId in WebFrameMain

[codebytere]

Description of Change

Refs https://issues.chromium.org/issues/40169570
Refs #30076

Fixes a crash in WebFrameMain::FromRenderFrameHost that could occur when a given RenderFrameHost's FrameTreeNode is invalid. In #30076 we addressed cross-origin navigation disposing WebFrameMain instances by mapping FrameTreeNode ids to WebFrameMains. At the time, a RenderFrameHost's FrameTreeNode was constant for the lifetime of the RenderFrameHost - that's no longer the case. With prerendering and bfcache, a RenderFrameHost can travel from one FrameTreeNode to another one during prerender activation.

We should migrate our mapping logic away from 1:1 FrameTreeNode -> RenderFrameHost but this addresses it crashing in the short term.

Details

Electron Framework
content::RenderFrameHostImpl::GetFrameTreeNodeId() const frame_tree_node.h:141
Electron Framework
electron::api::WebFrameMain::FromRenderFrameHost(content::RenderFrameHost*) electron_api_web_frame_main.cc:80
Electron Framework
electron::api::WebFrameMain::From(v8::Isolate*, content::RenderFrameHost*) electron_api_web_frame_main.cc:434
Electron Framework
electron::api::WebContents::FindReply(content::WebContents*, int, int, gfx::Rect const&, int, bool, content::RenderFrameHost*) electron_api_web_contents.cc:1512
Electron Framework
content::WebContentsImpl::NotifyFindReply(int, int, gfx::Rect const&, int, bool, content::RenderFrameHost*) web_contents_impl.cc:8965
Electron Framework
content::FindRequestManager::FinalUpdateReceived(int, content::RenderFrameHost*) find_request_manager.cc:914
Electron Framework
content::FindRequestManager::HandleFinalUpdateForFrame(content::RenderFrameHostImpl*, int) find_request_manager.cc:451
Electron Framework
blink::mojom::FindInPageClientStubDispatch::Accept(blink::mojom::FindInPageClient*, mojo::Message*) find_in_page.mojom.cc:825
Electron Framework
mojo::InterfaceEndpointClient::HandleValidatedMessage(mojo::Message*) interface_endpoint_client.cc:1016
Electron Framework
mojo::MessageDispatcher::Accept(mojo::Message*) message_dispatcher.cc:43
Electron Framework
mojo::InterfaceEndpointClient::HandleIncomingMessage(mojo::Message*) interface_endpoint_client.cc:701
Electron Framework
mojo::internal::MultiplexRouter::Accept(mojo::Message*) multiplex_router.cc:1096
Electron Framework
mojo::MessageDispatcher::Accept(mojo::Message*) message_dispatcher.cc:43
Electron Framework
mojo::Connector::DispatchMessage(mojo::ScopedHandleBase<mojo::MessageHandle>) connector.cc:561
Electron Framework
base::internal::Invoker<base::internal::BindState<void (mojo::Connector::*)(char const*, unsigned int), base::internal::UnretainedWrapper<mojo::Connector, base::unretained_traits::MayNotDangle, (base::RawPtrTraits)0>, base::internal::UnretainedWrapper<char const, base::unretained_traits::MayNotDangle, (base::RawPtrTraits)0>>, void (unsigned int)>::Run(base::internal::BindStateBase*, unsigned int) connector.cc:618
Electron Framework
base::internal::Invoker<base::internal::BindState<void (*)(base::RepeatingCallback<void (unsigned int)> const&, unsigned int, mojo::HandleSignalsState const&), base::RepeatingCallback<void (unsigned int)>>, void (unsigned int, mojo::HandleSignalsState const&)>::Run(base::internal::BindStateBase*, unsigned int, mojo::HandleSignalsState const&) callback.h:333
Electron Framework
base::internal::Invoker<base::internal::BindState<void (mojo::SimpleWatcher::*)(int, unsigned int, mojo::HandleSignalsState const&), base::WeakPtr<mojo::SimpleWatcher>, int, unsigned int, mojo::HandleSignalsState>, void ()>::RunOnce(base::internal::BindStateBase*) callback.h:333
Electron Framework
base::TaskAnnotator::RunTaskImpl(base::PendingTask&) callback.h:152
Electron Framework
base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl(base::LazyNow*) task_annotator.h:89
Electron Framework
non-virtual thunk to base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork() thread_controller_with_message_pump_impl.cc:345
Electron Framework
invocation function for block in base::MessagePumpCFRunLoopBase::RunWorkSource(void*) message_pump_apple.mm:416
Electron Framework
invocation function for block in base::MessagePumpCFRunLoopBase::RunWorkSource(void*) message_pump_apple.mm:416
Electron Framework
base::apple::CallWithEHFrame(void () block_pointer)
CoreFoundation
0x8da8f9d8 + 514520

Checklist

• PR description included and stakeholders cc'd
• npm test passes
• PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

Notes: none

[deepak1556]

Does the current map get updated when ftn gets swapped for the pre-rendering case ? What is the alternative here, should we change to use GlobalRenderFrameHostToken as the map identifier ?

[codebytere]

No - at the moment it doesn't since it was written for the assumption that wouldn't happen. I do think we can use GlobalRenderFrameHostToken instead; i can refactor it in this PR but i wasn't sure how much time it'd take to look into it more so I thought i'd mitigate a hard crash before doing so. Do you have a preference?

[deepak1556]

Lets do it as follow-up, we might want to change to track ftn directly rather than the underlying rfh so this needs further discussion.

[samuelmaddock]

Looking at the call stack, I'm not seeing where a frame is being created in WebContents::FindReply. Is this part of a fork?

Electron Framework
content::RenderFrameHostImpl::GetFrameTreeNodeId() const frame_tree_node.h:141
Electron Framework
electron::api::WebFrameMain::FromRenderFrameHost(content::RenderFrameHost*) electron_api_web_frame_main.cc:80
Electron Framework
electron::api::WebFrameMain::From(v8::Isolate*, content::RenderFrameHost*) electron_api_web_frame_main.cc:434
Electron Framework
electron::api::WebContents::FindReply(content::WebContents*, int, int, gfx::Rect const&, int, bool, content::RenderFrameHost*) electron_api_web_contents.cc:1512
Electron Framework
content::WebContentsImpl::NotifyFindReply(int, int, gfx::Rect const&, int, bool, content::RenderFrameHost*) web_contents_impl.cc:8965
Electron Framework

The intent I had with designing WebFrameMain was to essentially track a corresponding <iframe> in the renderer process. With that in mind, I think it should still be aware of FrameTreeNode/FTN (which more closely represents frames over the transient RFH) in some capacity. The unfortunate part of this design is that FTNs are not a public API. Open to discussing more beyond this PR.

That being said, I think merging this to protect against the hard crash is fine for now.

[deepak1556]

Yeah its from a fork which has the find API based on #28274

[deepak1556]

LGTM

[codebytere]

Failures are known flakes.

[release-clerk]

No Release Notes

[trop]

I have automatically backported this PR to "28-x-y", please check out #41592

[trop]

I have automatically backported this PR to "29-x-y", please check out #41593

[trop]

I have automatically backported this PR to "30-x-y", please check out #41594