Commit bc5da8c9 authored by Minoru Chikamune's avatar Minoru Chikamune Committed by Commit Bot

Migrate LocalFrame to use GC mojo wrappers.

No behavior change. This CL reduces potential risks of use-after-free bugs.

Bug: 1049056
Change-Id: Ibf4e509a959108ddee42bef7233a0436ee7376cf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2192574
Commit-Queue: Minoru Chikamune <chikamune@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Reviewed-by: default avatarKouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771491}
parent ed7f2284
...@@ -322,7 +322,8 @@ void LocalFrame::Init() { ...@@ -322,7 +322,8 @@ void LocalFrame::Init() {
CoreInitializer::GetInstance().InitLocalFrame(*this); CoreInitializer::GetInstance().InitLocalFrame(*this);
GetRemoteNavigationAssociatedInterfaces()->GetInterface( GetRemoteNavigationAssociatedInterfaces()->GetInterface(
local_frame_host_remote_.BindNewEndpointAndPassReceiver()); local_frame_host_remote_.BindNewEndpointAndPassReceiver(
GetTaskRunner(blink::TaskType::kInternalDefault)));
GetInterfaceRegistry()->AddAssociatedInterface(WTF::BindRepeating( GetInterfaceRegistry()->AddAssociatedInterface(WTF::BindRepeating(
&LocalFrame::BindToReceiver, WrapWeakPersistent(this))); &LocalFrame::BindToReceiver, WrapWeakPersistent(this)));
...@@ -413,6 +414,14 @@ void LocalFrame::Trace(Visitor* visitor) const { ...@@ -413,6 +414,14 @@ void LocalFrame::Trace(Visitor* visitor) const {
visitor->Trace(system_clipboard_); visitor->Trace(system_clipboard_);
visitor->Trace(raw_system_clipboard_); visitor->Trace(raw_system_clipboard_);
visitor->Trace(virtual_keyboard_overlay_changed_observers_); visitor->Trace(virtual_keyboard_overlay_changed_observers_);
visitor->Trace(pause_handle_receivers_);
visitor->Trace(reporting_service_);
#if defined(OS_MACOSX)
visitor->Trace(text_input_host_);
#endif
visitor->Trace(local_frame_host_remote_);
visitor->Trace(receiver_);
visitor->Trace(main_frame_receiver_);
Frame::Trace(visitor); Frame::Trace(visitor);
Supplementable<LocalFrame>::Trace(visitor); Supplementable<LocalFrame>::Trace(visitor);
} }
...@@ -1115,7 +1124,8 @@ LocalFrame::LocalFrame(LocalFrameClient* client, ...@@ -1115,7 +1124,8 @@ LocalFrame::LocalFrame(LocalFrameClient* client,
// It should be bound before accessing TextInputHost which is the interface to // It should be bound before accessing TextInputHost which is the interface to
// respond to GetCharacterIndexAtPoint. // respond to GetCharacterIndexAtPoint.
GetBrowserInterfaceBroker().GetInterface( GetBrowserInterfaceBroker().GetInterface(
text_input_host_.BindNewPipeAndPassReceiver()); text_input_host_.BindNewPipeAndPassReceiver(
GetTaskRunner(blink::TaskType::kInternalDefault)));
#endif #endif
} }
...@@ -1714,7 +1724,8 @@ void LocalFrame::PauseSubresourceLoading( ...@@ -1714,7 +1724,8 @@ void LocalFrame::PauseSubresourceLoading(
auto handle = GetFrameScheduler()->GetPauseSubresourceLoadingHandle(); auto handle = GetFrameScheduler()->GetPauseSubresourceLoadingHandle();
if (!handle) if (!handle)
return; return;
pause_handle_receivers_.Add(std::move(handle), std::move(receiver)); pause_handle_receivers_.Add(std::move(handle), std::move(receiver),
GetTaskRunner(blink::TaskType::kInternalDefault));
} }
void LocalFrame::ResumeSubresourceLoading() { void LocalFrame::ResumeSubresourceLoading() {
...@@ -1761,13 +1772,13 @@ const base::UnguessableToken& LocalFrame::GetAgentClusterId() const { ...@@ -1761,13 +1772,13 @@ const base::UnguessableToken& LocalFrame::GetAgentClusterId() const {
: base::UnguessableToken::Null(); : base::UnguessableToken::Null();
} }
const mojo::Remote<mojom::blink::ReportingServiceProxy>& mojom::blink::ReportingServiceProxy* LocalFrame::GetReportingService() {
LocalFrame::GetReportingService() const { if (!reporting_service_.is_bound()) {
if (!reporting_service_) {
Platform::Current()->GetBrowserInterfaceBroker()->GetInterface( Platform::Current()->GetBrowserInterfaceBroker()->GetInterface(
reporting_service_.BindNewPipeAndPassReceiver()); reporting_service_.BindNewPipeAndPassReceiver(
GetTaskRunner(blink::TaskType::kInternalDefault)));
} }
return reporting_service_; return reporting_service_.get();
} }
// static // static
...@@ -2310,7 +2321,8 @@ void LocalFrame::AddMessageToConsole(mojom::blink::ConsoleMessageLevel level, ...@@ -2310,7 +2321,8 @@ void LocalFrame::AddMessageToConsole(mojom::blink::ConsoleMessageLevel level,
void LocalFrame::AddInspectorIssue(mojom::blink::InspectorIssueInfoPtr info) { void LocalFrame::AddInspectorIssue(mojom::blink::InspectorIssueInfoPtr info) {
if (GetPage()) { if (GetPage()) {
GetPage()->GetInspectorIssueStorage().AddInspectorIssue(DomWindow(), std::move(info)); GetPage()->GetInspectorIssueStorage().AddInspectorIssue(DomWindow(),
std::move(info));
} }
} }
...@@ -2732,7 +2744,7 @@ bool LocalFrame::ShouldThrottleDownload() { ...@@ -2732,7 +2744,7 @@ bool LocalFrame::ShouldThrottleDownload() {
#if defined(OS_MACOSX) #if defined(OS_MACOSX)
mojom::blink::TextInputHost& LocalFrame::GetTextInputHost() { mojom::blink::TextInputHost& LocalFrame::GetTextInputHost() {
DCHECK(text_input_host_); DCHECK(text_input_host_.is_bound());
return *text_input_host_.get(); return *text_input_host_.get();
} }
#endif #endif
......
...@@ -35,8 +35,6 @@ ...@@ -35,8 +35,6 @@
#include "base/time/default_tick_clock.h" #include "base/time/default_tick_clock.h"
#include "base/unguessable_token.h" #include "base/unguessable_token.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "mojo/public/cpp/bindings/associated_receiver.h"
#include "mojo/public/cpp/bindings/associated_remote.h"
#include "mojo/public/cpp/bindings/pending_associated_receiver.h" #include "mojo/public/cpp/bindings/pending_associated_receiver.h"
#include "mojo/public/cpp/bindings/pending_receiver.h" #include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/unique_receiver_set.h" #include "mojo/public/cpp/bindings/unique_receiver_set.h"
...@@ -63,6 +61,11 @@ ...@@ -63,6 +61,11 @@
#include "third_party/blink/renderer/platform/heap/handle.h" #include "third_party/blink/renderer/platform/heap/handle.h"
#include "third_party/blink/renderer/platform/instrumentation/instance_counters.h" #include "third_party/blink/renderer/platform/instrumentation/instance_counters.h"
#include "third_party/blink/renderer/platform/loader/fetch/client_hints_preferences.h" #include "third_party/blink/renderer/platform/loader/fetch/client_hints_preferences.h"
#include "third_party/blink/renderer/platform/mojo/heap_mojo_associated_receiver.h"
#include "third_party/blink/renderer/platform/mojo/heap_mojo_associated_remote.h"
#include "third_party/blink/renderer/platform/mojo/heap_mojo_remote.h"
#include "third_party/blink/renderer/platform/mojo/heap_mojo_unique_receiver_set.h"
#include "third_party/blink/renderer/platform/mojo/heap_mojo_wrapper_mode.h"
#include "third_party/blink/renderer/platform/scheduler/public/frame_scheduler.h" #include "third_party/blink/renderer/platform/scheduler/public/frame_scheduler.h"
#include "third_party/blink/renderer/platform/supplementable.h" #include "third_party/blink/renderer/platform/supplementable.h"
#include "third_party/blink/renderer/platform/wtf/hash_set.h" #include "third_party/blink/renderer/platform/wtf/hash_set.h"
...@@ -447,8 +450,7 @@ class CORE_EXPORT LocalFrame final : public Frame, ...@@ -447,8 +450,7 @@ class CORE_EXPORT LocalFrame final : public Frame,
SmoothScrollSequencer& GetSmoothScrollSequencer(); SmoothScrollSequencer& GetSmoothScrollSequencer();
const mojo::Remote<mojom::blink::ReportingServiceProxy>& GetReportingService() mojom::blink::ReportingServiceProxy* GetReportingService();
const;
// Returns the frame host ptr. The interface returned is backed by an // Returns the frame host ptr. The interface returned is backed by an
// associated interface with the legacy Chrome IPC channel. // associated interface with the legacy Chrome IPC channel.
...@@ -680,8 +682,13 @@ class CORE_EXPORT LocalFrame final : public Frame, ...@@ -680,8 +682,13 @@ class CORE_EXPORT LocalFrame final : public Frame,
// Holds all PauseSubresourceLoadingHandles allowing either |this| to delete // Holds all PauseSubresourceLoadingHandles allowing either |this| to delete
// them explicitly or the pipe closing to delete them. // them explicitly or the pipe closing to delete them.
mojo::UniqueReceiverSet<blink::mojom::blink::PauseSubresourceLoadingHandle> //
pause_handle_receivers_; // LocalFrame can be reused by multiple ExecutionContext.
HeapMojoUniqueReceiverSet<
blink::mojom::blink::PauseSubresourceLoadingHandle,
std::default_delete<blink::mojom::blink::PauseSubresourceLoadingHandle>,
HeapMojoWrapperMode::kWithoutContextObserver>
pause_handle_receivers_{nullptr};
// Keeps track of all the registered VK observers. // Keeps track of all the registered VK observers.
HeapHashSet<WeakMember<VirtualKeyboardOverlayChangedObserver>> HeapHashSet<WeakMember<VirtualKeyboardOverlayChangedObserver>>
...@@ -738,10 +745,17 @@ class CORE_EXPORT LocalFrame final : public Frame, ...@@ -738,10 +745,17 @@ class CORE_EXPORT LocalFrame final : public Frame,
InterfaceRegistry* const interface_registry_; InterfaceRegistry* const interface_registry_;
// This is declared mutable so that the service endpoint can be cached by // This is declared mutable so that the service endpoint can be cached by
// const methods. // const methods.
mutable mojo::Remote<mojom::blink::ReportingServiceProxy> reporting_service_; //
// LocalFrame can be reused by multiple ExecutionContext.
mutable HeapMojoRemote<mojom::blink::ReportingServiceProxy,
HeapMojoWrapperMode::kWithoutContextObserver>
reporting_service_{nullptr};
#if defined(OS_MACOSX) #if defined(OS_MACOSX)
mojo::Remote<mojom::blink::TextInputHost> text_input_host_; // LocalFrame can be reused by multiple ExecutionContext.
HeapMojoRemote<mojom::blink::TextInputHost,
HeapMojoWrapperMode::kWithoutContextObserver>
text_input_host_{nullptr};
#endif #endif
ViewportIntersectionState intersection_state_; ViewportIntersectionState intersection_state_;
...@@ -771,10 +785,20 @@ class CORE_EXPORT LocalFrame final : public Frame, ...@@ -771,10 +785,20 @@ class CORE_EXPORT LocalFrame final : public Frame,
std::unique_ptr<WebPrescientNetworking> prescient_networking_; std::unique_ptr<WebPrescientNetworking> prescient_networking_;
mojo::AssociatedRemote<mojom::blink::LocalFrameHost> local_frame_host_remote_; // LocalFrame can be reused by multiple ExecutionContext.
mojo::AssociatedReceiver<mojom::blink::LocalFrame> receiver_{this}; HeapMojoAssociatedRemote<mojom::blink::LocalFrameHost,
mojo::AssociatedReceiver<mojom::blink::LocalMainFrame> main_frame_receiver_{ HeapMojoWrapperMode::kWithoutContextObserver>
this}; local_frame_host_remote_{nullptr};
// LocalFrame can be reused by multiple ExecutionContext.
HeapMojoAssociatedReceiver<mojom::blink::LocalFrame,
LocalFrame,
HeapMojoWrapperMode::kWithoutContextObserver>
receiver_{this, nullptr};
// LocalFrame can be reused by multiple ExecutionContext.
HeapMojoAssociatedReceiver<mojom::blink::LocalMainFrame,
LocalFrame,
HeapMojoWrapperMode::kWithoutContextObserver>
main_frame_receiver_{this, nullptr};
// Variable to control burst of download requests. // Variable to control burst of download requests.
int num_burst_download_requests_ = 0; int num_burst_download_requests_ = 0;
......
...@@ -260,7 +260,8 @@ TEST_F(LocalFrameTest, CharacterIndexAtPointWithPinchZoom) { ...@@ -260,7 +260,8 @@ TEST_F(LocalFrameTest, CharacterIndexAtPointWithPinchZoom) {
TestTextInputHostWaiter waiter; TestTextInputHostWaiter waiter;
waiter.Init(run_loop.QuitClosure(), main_frame->GetBrowserInterfaceBroker()); waiter.Init(run_loop.QuitClosure(), main_frame->GetBrowserInterfaceBroker());
main_frame->GetBrowserInterfaceBroker().GetInterface( main_frame->GetBrowserInterfaceBroker().GetInterface(
main_frame->text_input_host_.BindNewPipeAndPassReceiver()); main_frame->text_input_host_.BindNewPipeAndPassReceiver(
main_frame->GetTaskRunner(blink::TaskType::kInternalDefault)));
// Since we're zoomed in to 2X, each char of Ahem is 20px wide/tall in // Since we're zoomed in to 2X, each char of Ahem is 20px wide/tall in
// viewport space. We expect to hit the fifth char on the first line. // viewport space. We expect to hit the fifth char on the first line.
main_frame->GetCharacterIndexAtPoint(gfx::Point(100, 15)); main_frame->GetCharacterIndexAtPoint(gfx::Point(100, 15));
......
...@@ -35,7 +35,6 @@ class HeapMojoAssociatedReceiverSet { ...@@ -35,7 +35,6 @@ class HeapMojoAssociatedReceiverSet {
"Owner should implement Interface"); "Owner should implement Interface");
static_assert(IsGarbageCollectedType<Owner>::value, static_assert(IsGarbageCollectedType<Owner>::value,
"Owner needs to be a garbage collected object"); "Owner needs to be a garbage collected object");
DCHECK(context);
} }
HeapMojoAssociatedReceiverSet(const HeapMojoAssociatedReceiverSet&) = delete; HeapMojoAssociatedReceiverSet(const HeapMojoAssociatedReceiverSet&) = delete;
HeapMojoAssociatedReceiverSet& operator=( HeapMojoAssociatedReceiverSet& operator=(
......
...@@ -35,7 +35,6 @@ class HeapMojoReceiverSet { ...@@ -35,7 +35,6 @@ class HeapMojoReceiverSet {
"Owner should implement Interface"); "Owner should implement Interface");
static_assert(IsGarbageCollectedType<Owner>::value, static_assert(IsGarbageCollectedType<Owner>::value,
"Owner needs to be a garbage collected object"); "Owner needs to be a garbage collected object");
DCHECK(context);
} }
HeapMojoReceiverSet(const HeapMojoReceiverSet&) = delete; HeapMojoReceiverSet(const HeapMojoReceiverSet&) = delete;
HeapMojoReceiverSet& operator=(const HeapMojoReceiverSet&) = delete; HeapMojoReceiverSet& operator=(const HeapMojoReceiverSet&) = delete;
......
...@@ -31,9 +31,7 @@ class HeapMojoUniqueReceiverSet { ...@@ -31,9 +31,7 @@ class HeapMojoUniqueReceiverSet {
mojo::UniquePtrImplRefTraits<Interface, Deleter>>::ImplPointerType; mojo::UniquePtrImplRefTraits<Interface, Deleter>>::ImplPointerType;
explicit HeapMojoUniqueReceiverSet(ContextLifecycleNotifier* context) explicit HeapMojoUniqueReceiverSet(ContextLifecycleNotifier* context)
: wrapper_(MakeGarbageCollected<Wrapper>(context)) { : wrapper_(MakeGarbageCollected<Wrapper>(context)) {}
DCHECK(context);
}
HeapMojoUniqueReceiverSet(const HeapMojoUniqueReceiverSet&) = delete; HeapMojoUniqueReceiverSet(const HeapMojoUniqueReceiverSet&) = delete;
HeapMojoUniqueReceiverSet& operator=(const HeapMojoUniqueReceiverSet&) = HeapMojoUniqueReceiverSet& operator=(const HeapMojoUniqueReceiverSet&) =
delete; delete;
......
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