Commit 59487759 authored by arthursonzogni's avatar arthursonzogni Committed by Commit Bot

[COOP] access reporting: Clear old CoopAccessMonitor.

Clear CoopAccessMonitor when their reporter are gone. This avoids
accumulating them.
Similarly, because we don't have RenderDocument, the accessing
LocalFrame might host new documents. A new CoopAccessMonitor will be
sent after the commit. The new one must
replace the old one. This will allow the new set or reported URLs to be
reported.

This is a positive change, but still not perfect at all. Ideally, the
CoopAccessMonitor(s) passed into process of the new Document should be
passed along the CommitNavigation IPC.

Bug: 1090273
Change-Id: Ib444e760175207702867f47012fa05bd5e051576
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2408752Reviewed-by: default avatarCamille Lamy <clamy@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#806625}
parent 9341ceb5
......@@ -446,10 +446,27 @@ void DOMWindow::InstallCoopAccessMonitor(
monitor.report_type = report_type;
monitor.accessing_main_frame = accessing_frame->GetFrameToken();
// TODO(arthursonzogni): Clean coop_access_monitor_ when a reporter is gone.
// Use mojo::Remote::set_disconnect_handler.
monitor.reporter.Bind(std::move(pending_reporter));
// CoopAccessMonitor are cleared when their reporter are gone. This avoids
// accumulation. However it would have been interesting continuing reporting
// accesses past this point, at least for the ReportingObserver and Devtool.
// TODO(arthursonzogni): Consider observing |accessing_main_frame| deletion
// instead.
monitor.reporter.set_disconnect_handler(
WTF::Bind(&DOMWindow::DisconnectCoopAccessMonitor,
WrapWeakPersistent(this), monitor.accessing_main_frame));
// As long as RenderDocument isn't shipped, it can exist a CoopAccessMonitor
// for the same |accessing_main_frame|, because it might now host a different
// Document. Same is true for |this| DOMWindow, it might refer to a window
// hosting a different document.
// The new documents will still be part of a different virtual browsing
// context group, however the new COOPAccessMonitor might now contain updated
// URLs.
DisconnectCoopAccessMonitor(monitor.accessing_main_frame);
// Any attempts to access |this| window from |accessing_main_frame| will now
// trigger reports (network, ReportingObserver, Devtool).
coop_access_monitor_.push_back(std::move(monitor));
}
......@@ -632,4 +649,15 @@ void DOMWindow::Trace(Visitor* visitor) const {
EventTargetWithInlineData::Trace(visitor);
}
void DOMWindow::DisconnectCoopAccessMonitor(
base::UnguessableToken accessing_main_frame) {
auto* it = coop_access_monitor_.begin();
while (it != coop_access_monitor_.end()) {
if (it->accessing_main_frame == accessing_main_frame)
it = coop_access_monitor_.erase(it);
else
++it;
}
}
} // namespace blink
......@@ -165,6 +165,11 @@ class CORE_EXPORT DOMWindow : public EventTargetWithInlineData {
LocalDOMWindow* source,
ExceptionState&);
// Removed the CoopAccessMonitor with the given |accessing_main_frame| from
// the |coop_access_monitor| list. This is called when the COOP reporter is
// gone or a more recent CoopAccessMonitor is being added.
void DisconnectCoopAccessMonitor(base::UnguessableToken accessing_main_frame);
Member<Frame> frame_;
// Unlike |frame_|, |window_proxy_manager_| is available even after the
// window's frame gets detached from the DOM, until the end of the lifetime
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment