New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 795258 link

Starred by 2 users

Issue metadata

Status: Started
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 1
Type: Bug



Sign in to add a comment

Make it easy for clients of RF::GetRemoteInterfaces to correctly manage Mojo connection lifetimes

Project Member Reported by engedy@chromium.org, Dec 15 2017

Issue description

Some clients of RenderFrame::GetRemoteInterfaces() are unaware that correct operation of mojom::InterfacePtrs they request directly or indirectly through GetRemoteInterfaces are only guaranteed for the active document. That is, if the request end is not bound to the implementation on the browser side before the next cross-document navigation, it is dropped, and the client end becomes a dud).

To avoid replicating logic to re-bind InterfacePtrs on DidCommitProvisionalLoad (if !is_same_document_navigation and !reused_global_object) in all clients who just want to reliably send a message, we should introduce some helper classes to make this easy.
 
Labels: OS-Android OS-Chrome OS-Fuchsia OS-Linux OS-Mac OS-Windows
Is this really P2? Most users of RenderFrame::GetRemoteInterfaces() will suffer random, rare, failures now. If this goes to stable, it may cause teams to spend a lot of time debugging it (and it's also bad for the end users of course).

Comment 2 by engedy@chromium.org, Mar 21 2018

Labels: -Pri-2 M-67 Pri-1
Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 28 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/07512adc7fab8fa6b6b85dfc0d1f008a81097fa1

commit 07512adc7fab8fa6b6b85dfc0d1f008a81097fa1
Author: Balazs Engedy <engedy@chromium.org>
Date: Wed Mar 28 11:26:29 2018

Add RenderFrameHostImpl.DroppedInterfaceRequests.

This CL adds RenderFrameHostImpl.DroppedInterfaceRequests that records,
for each load in a frame, the number of interface requests to
RenderFrame::GetRemoteInterfaces that arrived to the RenderFrameHostImpl
after the RFHI had already committed the next cross-document navigation.

This means that |document_scoped_interface_provider_binding_| was
already unbound at the time from the interface connection that had been
used to service RenderFrame::GetRemoteInterface for the previous load,
so those interface requests are dropped.

This scenario could not be reproduced during local testing at all,
although it is suspected to occasionally happen in practice. This
histogram will give how often this happens, which will at least serve
as an upper bound for potentially problematic cases.

Bug: 795258
Change-Id: I3e6d61b62f97121ab0824a63243b84c94f8e6161
Reviewed-on: https://chromium-review.googlesource.com/974263
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546455}
[modify] https://crrev.com/07512adc7fab8fa6b6b85dfc0d1f008a81097fa1/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/07512adc7fab8fa6b6b85dfc0d1f008a81097fa1/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/07512adc7fab8fa6b6b85dfc0d1f008a81097fa1/content/browser/frame_host/render_frame_host_impl_browsertest.cc
[modify] https://crrev.com/07512adc7fab8fa6b6b85dfc0d1f008a81097fa1/tools/metrics/histograms/histograms.xml

Project Member

Comment 4 by bugdroid1@chromium.org, Apr 10 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fa01f1ef035dc107d3796bda7102f06d52caea6b

commit fa01f1ef035dc107d3796bda7102f06d52caea6b
Author: Balazs Engedy <engedy@chromium.org>
Date: Tue Apr 10 13:35:33 2018

Add RenderFrameHostImpl.DroppedInterfaceRequestName.

Interface requests to RenderFrame::GetRemoteInterfaces are dropped when
they arrive to the RenderFrameHostImpl after the RenderFrameHostImpl had
already committed the next cross-document navigation.

In follow-up to crrev.com/c/974263, this CL adds a histogram to record,
for each load, and for each dropped Mojo interface request in a frame, a
sample with a value corresponding to the hash of the dropped interface's
name, calculated as the lower 31 bits of the base::HashMetricName(name).

Bug: 795258
Change-Id: I0a4a3aa552c4bc2eb543b4aad3a3a35ecae9f831
Reviewed-on: https://chromium-review.googlesource.com/993059
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549505}
[modify] https://crrev.com/fa01f1ef035dc107d3796bda7102f06d52caea6b/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/fa01f1ef035dc107d3796bda7102f06d52caea6b/content/browser/frame_host/render_frame_host_impl_browsertest.cc
[modify] https://crrev.com/fa01f1ef035dc107d3796bda7102f06d52caea6b/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/fa01f1ef035dc107d3796bda7102f06d52caea6b/tools/metrics/histograms/histograms.xml

Cc: -roc...@chromium.org rockot@google.com

Sign in to add a comment