Skip to content

Commit

Permalink
Revert "WebView: Release its reference when RenderFrameHost is destro…
Browse files Browse the repository at this point in the history
…yed"

This reverts commit bf9fb13.

Reason for revert: broke the Google account flow (crbug.com/1493848)

Original change's description:
> WebView: Release its reference when RenderFrameHost is destroyed
>
> The iframes within the page can be dynamically removed, triggering the
> destruction of the RenderFrameHost(RFH). However, the 'host_' and
> 'hosts_is_in_primary_main_frame_' within GinJavaBridgeMessageFilter
> still reference the released RFH. Frequent iframe operations may result
> in a continuous increase in the size of 'host_' and
> 'hosts_is_in_primary_main_frame_'
>
> R=torne@chromium.org
>
> Low-Coverage-Reason: HARD_TO_TEST manually tested on crbug.com/1481603
> Bug: 1481603,1481037
> Change-Id: I517ab2e0245d05db83ed6f45a6f221714028cbde
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4861403
> Commit-Queue: Richard (Torne) Coles <torne@chromium.org>
> Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
> Reviewed-by: Richard (Torne) Coles <torne@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1208297}

(cherry picked from commit e1aedc8)

Bug: 1481603,1481037
Change-Id: If674fb53bd864ecef118d6fe240a1486a6d79d39
Fixed: 1493848
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4954035
Owners-Override: Harry Souders <harrysouders@google.com>
Auto-Submit: Nate Fischer <ntfschr@chromium.org>
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Commit-Queue: Harry Souders <harrysouders@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Original-Commit-Position: refs/heads/main@{#1211883}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4953510
Cr-Commit-Position: refs/branch-heads/6076@{#3}
Cr-Branched-From: 65ba1ef-refs/heads/main@{#1211682}
  • Loading branch information
ntfschr-chromium authored and Chromium LUCI CQ committed Oct 19, 2023
1 parent 200f527 commit 96f01f9
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 46 deletions.
1 change: 0 additions & 1 deletion AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,6 @@ Giovanni Panaro <tsrwebgl@gmail.com>
Girish Kumar M <mck.giri@samsung.com>
Gitanshu Mehndiratta <g.mehndiratt@samsung.com>
Giuseppe Iuculano <giuseppe@iuculano.it>
Gloam <gaoqingguang@kuaishou.com>
Gnanasekar Somanathan <gnanasekar.s@samsung.com>
Gordana Cmiljanovic <gordana.cmiljanovic@imgtec.com>
Goutham Jagannatha <wrm364@motorola.com>
Expand Down
87 changes: 44 additions & 43 deletions content/browser/android/java/gin_java_bridge_dispatcher_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,33 +55,28 @@ void GinJavaBridgeDispatcherHost::InstallFilterAndRegisterAllRoutingIds() {
return;
}

web_contents()->GetPrimaryMainFrame()->ForEachRenderFrameHost(
[this](RenderFrameHostImpl* frame) {
if (frame->IsRenderFrameLive()) {
InstallFilterAndRegisterRoutingId(frame);
}
});
}

void GinJavaBridgeDispatcherHost::InstallFilterAndRegisterRoutingId(
RenderFrameHost* render_frame_host) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
AgentSchedulingGroupHost& agent_scheduling_group =
static_cast<RenderFrameHostImpl*>(render_frame_host)
->GetAgentSchedulingGroup();
scoped_refptr<GinJavaBridgeMessageFilter> per_asg_filter =
GinJavaBridgeMessageFilter::FromHost(agent_scheduling_group,
/*create_if_not_exists=*/true);
if (base::FeatureList::IsEnabled(features::kMBIMode)) {
scoped_refptr<GinJavaBridgeObjectDeletionMessageFilter>
process_global_filter =
GinJavaBridgeObjectDeletionMessageFilter::FromHost(
agent_scheduling_group.GetProcess(),
/*create_if_not_exists=*/true);
process_global_filter->AddRoutingIdForHost(this, render_frame_host);
}

per_asg_filter->AddRoutingIdForHost(this, render_frame_host);
web_contents()
->GetPrimaryMainFrame()
->ForEachRenderFrameHost(
[this](RenderFrameHostImpl* frame) {
AgentSchedulingGroupHost& agent_scheduling_group =
frame->GetAgentSchedulingGroup();

scoped_refptr<GinJavaBridgeMessageFilter> per_asg_filter =
GinJavaBridgeMessageFilter::FromHost(
agent_scheduling_group,
/*create_if_not_exists=*/true);
if (base::FeatureList::IsEnabled(features::kMBIMode)) {
scoped_refptr<GinJavaBridgeObjectDeletionMessageFilter>
process_global_filter =
GinJavaBridgeObjectDeletionMessageFilter::FromHost(
agent_scheduling_group.GetProcess(),
/*create_if_not_exists=*/true);
process_global_filter->AddRoutingIdForHost(this, frame);
}

per_asg_filter->AddRoutingIdForHost(this, frame);
});
}

WebContentsImpl* GinJavaBridgeDispatcherHost::web_contents() const {
Expand All @@ -91,11 +86,16 @@ WebContentsImpl* GinJavaBridgeDispatcherHost::web_contents() const {
void GinJavaBridgeDispatcherHost::RenderFrameCreated(
RenderFrameHost* render_frame_host) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
if (named_objects_.empty()) {
return;
AgentSchedulingGroupHost& agent_scheduling_group =
static_cast<RenderFrameHostImpl*>(render_frame_host)
->GetAgentSchedulingGroup();
if (scoped_refptr<GinJavaBridgeMessageFilter> filter =
GinJavaBridgeMessageFilter::FromHost(
agent_scheduling_group, /*create_if_not_exists=*/false)) {
filter->AddRoutingIdForHost(this, render_frame_host);
} else {
InstallFilterAndRegisterAllRoutingIds();
}

InstallFilterAndRegisterRoutingId(render_frame_host);
for (NamedObjectMap::const_iterator iter = named_objects_.begin();
iter != named_objects_.end();
++iter) {
Expand All @@ -104,18 +104,19 @@ void GinJavaBridgeDispatcherHost::RenderFrameCreated(
}
}

void GinJavaBridgeDispatcherHost::RenderFrameDeleted(
RenderFrameHost* render_frame_host) {
AgentSchedulingGroupHost& agent_scheduling_group =
static_cast<RenderFrameHostImpl*>(render_frame_host)
->GetAgentSchedulingGroup();
scoped_refptr<GinJavaBridgeMessageFilter> filter =
GinJavaBridgeMessageFilter::FromHost(agent_scheduling_group,
/*create_if_not_exists=*/false);

if (filter) {
filter->RemoveHost(this);
}
void GinJavaBridgeDispatcherHost::WebContentsDestroyed() {
// Unretained() is safe because ForEachRenderFrameHost() is synchronous.
web_contents()->GetPrimaryMainFrame()->ForEachRenderFrameHost(
[this](RenderFrameHostImpl* frame) {
AgentSchedulingGroupHost& agent_scheduling_group =
frame->GetAgentSchedulingGroup();
scoped_refptr<GinJavaBridgeMessageFilter> filter =
GinJavaBridgeMessageFilter::FromHost(
agent_scheduling_group, /*create_if_not_exists=*/false);

if (filter)
filter->RemoveHost(this);
});
}

void GinJavaBridgeDispatcherHost::PrimaryPageChanged(Page& page) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ class GinJavaBridgeDispatcherHost

// WebContentsObserver
void RenderFrameCreated(RenderFrameHost* render_frame_host) override;
void RenderFrameDeleted(RenderFrameHost* render_frame_host) override;
void PrimaryMainDocumentElementAvailable() override;
void WebContentsDestroyed() override;
void PrimaryPageChanged(Page& page) override;

// GinJavaMethodInvocationHelper::DispatcherDelegate
Expand Down Expand Up @@ -84,7 +84,6 @@ class GinJavaBridgeDispatcherHost

// Run on the UI thread.
void InstallFilterAndRegisterAllRoutingIds();
void InstallFilterAndRegisterRoutingId(RenderFrameHost* render_frame_host);
WebContentsImpl* web_contents() const;

// Run on any thread.
Expand Down

0 comments on commit 96f01f9

Please sign in to comment.