From a3448376a1f9d3a381e03277fddf6c46b76c9104 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Fri, 9 Jun 2023 15:28:11 -0500 Subject: [PATCH] refactor: api web contents ownership (#38695) * refactor: aggregate api::WebContents::exclusive_access_manager_ directly * refactor: make WebContents::devtools_file_system_indexer_ scoped_refptr const * refactor: make WebContents::file_task_runner_ scoped_refptr const * refactor: make WebContents::print_task_runner_ scoped_refptr const --- .../browser/api/electron_api_web_contents.cc | 41 +++++++------------ shell/browser/api/electron_api_web_contents.h | 14 ++++--- 2 files changed, 22 insertions(+), 33 deletions(-) diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc index 0980148caeb1..4a2e08a0803a 100644 --- a/shell/browser/api/electron_api_web_contents.cc +++ b/shell/browser/api/electron_api_web_contents.cc @@ -783,12 +783,7 @@ WebContents::WebContents(v8::Isolate* isolate, content::WebContents* web_contents) : content::WebContentsObserver(web_contents), type_(Type::kRemote), - id_(GetAllWebContents().Add(this)), - devtools_file_system_indexer_( - base::MakeRefCounted()), - exclusive_access_manager_(std::make_unique(this)), - file_task_runner_( - base::ThreadPool::CreateSequencedTaskRunner({base::MayBlock()})) + id_(GetAllWebContents().Add(this)) #if BUILDFLAG(ENABLE_PRINTING) , print_task_runner_(CreatePrinterHandlerTaskRunner()) @@ -821,12 +816,7 @@ WebContents::WebContents(v8::Isolate* isolate, Type type) : content::WebContentsObserver(web_contents.get()), type_(type), - id_(GetAllWebContents().Add(this)), - devtools_file_system_indexer_( - base::MakeRefCounted()), - exclusive_access_manager_(std::make_unique(this)), - file_task_runner_( - base::ThreadPool::CreateSequencedTaskRunner({base::MayBlock()})) + id_(GetAllWebContents().Add(this)) #if BUILDFLAG(ENABLE_PRINTING) , print_task_runner_(CreatePrinterHandlerTaskRunner()) @@ -842,12 +832,7 @@ WebContents::WebContents(v8::Isolate* isolate, WebContents::WebContents(v8::Isolate* isolate, const gin_helper::Dictionary& options) - : id_(GetAllWebContents().Add(this)), - devtools_file_system_indexer_( - base::MakeRefCounted()), - exclusive_access_manager_(std::make_unique(this)), - file_task_runner_( - base::ThreadPool::CreateSequencedTaskRunner({base::MayBlock()})) + : id_(GetAllWebContents().Add(this)) #if BUILDFLAG(ENABLE_PRINTING) , print_task_runner_(CreatePrinterHandlerTaskRunner()) @@ -1409,7 +1394,7 @@ bool WebContents::PlatformHandleKeyboardEvent( content::KeyboardEventProcessingResult WebContents::PreHandleKeyboardEvent( content::WebContents* source, const content::NativeWebKeyboardEvent& event) { - if (exclusive_access_manager_->HandleUserKeyEvent(event)) + if (exclusive_access_manager_.HandleUserKeyEvent(event)) return content::KeyboardEventProcessingResult::HANDLED; if (event.GetType() == blink::WebInputEvent::Type::kRawKeyDown || @@ -1497,7 +1482,7 @@ void WebContents::OnEnterFullscreenModeForTab( owner_window()->set_fullscreen_transition_type( NativeWindow::FullScreenTransitionType::kHTML); - exclusive_access_manager_->fullscreen_controller()->EnterFullscreenModeForTab( + exclusive_access_manager_.fullscreen_controller()->EnterFullscreenModeForTab( requesting_frame, options.display_id); SetHtmlApiFullscreen(true); @@ -1515,7 +1500,7 @@ void WebContents::ExitFullscreenModeForTab(content::WebContents* source) { // This needs to be called before we exit fullscreen on the native window, // or the controller will incorrectly think we weren't fullscreen and bail. - exclusive_access_manager_->fullscreen_controller()->ExitFullscreenModeForTab( + exclusive_access_manager_.fullscreen_controller()->ExitFullscreenModeForTab( source); SetHtmlApiFullscreen(false); @@ -1574,7 +1559,7 @@ void WebContents::RequestExclusivePointerAccess( bool last_unlocked_by_target, bool allowed) { if (allowed) { - exclusive_access_manager_->mouse_lock_controller()->RequestToLockMouse( + exclusive_access_manager_.mouse_lock_controller()->RequestToLockMouse( web_contents, user_gesture, last_unlocked_by_target); } else { web_contents->GotResponseToLockMouseRequest( @@ -1594,18 +1579,18 @@ void WebContents::RequestToLockMouse(content::WebContents* web_contents, } void WebContents::LostMouseLock() { - exclusive_access_manager_->mouse_lock_controller()->LostMouseLock(); + exclusive_access_manager_.mouse_lock_controller()->LostMouseLock(); } void WebContents::RequestKeyboardLock(content::WebContents* web_contents, bool esc_key_locked) { - exclusive_access_manager_->keyboard_lock_controller()->RequestKeyboardLock( + exclusive_access_manager_.keyboard_lock_controller()->RequestKeyboardLock( web_contents, esc_key_locked); } void WebContents::CancelKeyboardLockRequest( content::WebContents* web_contents) { - exclusive_access_manager_->keyboard_lock_controller() + exclusive_access_manager_.keyboard_lock_controller() ->CancelKeyboardLockRequest(web_contents); } @@ -3875,8 +3860,10 @@ bool WebContents::IsFullscreenForTabOrPending( content::FullscreenState WebContents::GetFullscreenState( const content::WebContents* source) const { - return exclusive_access_manager_->fullscreen_controller()->GetFullscreenState( - source); + // `const_cast` here because EAM does not have const getters + return const_cast(&exclusive_access_manager_) + ->fullscreen_controller() + ->GetFullscreenState(source); } bool WebContents::TakeFocus(content::WebContents* source, bool reverse) { diff --git a/shell/browser/api/electron_api_web_contents.h b/shell/browser/api/electron_api_web_contents.h index d7424056b487..da2757f4d6c8 100644 --- a/shell/browser/api/electron_api_web_contents.h +++ b/shell/browser/api/electron_api_web_contents.h @@ -16,9 +16,11 @@ #include "base/memory/weak_ptr.h" #include "base/observer_list.h" #include "base/observer_list_types.h" +#include "base/task/thread_pool.h" #include "chrome/browser/devtools/devtools_eye_dropper.h" #include "chrome/browser/devtools/devtools_file_system_indexer.h" #include "chrome/browser/ui/exclusive_access/exclusive_access_context.h" // nogncheck +#include "chrome/browser/ui/exclusive_access/exclusive_access_manager.h" #include "content/common/frame.mojom.h" #include "content/public/browser/devtools_agent_host.h" #include "content/public/browser/keyboard_event_processing_result.h" @@ -77,8 +79,6 @@ namespace gin { class Arguments; } -class ExclusiveAccessManager; - class SkRegion; namespace electron { @@ -803,9 +803,10 @@ class WebContents : public ExclusiveAccessContext, // Whether window is fullscreened by window api. bool native_fullscreen_ = false; - scoped_refptr devtools_file_system_indexer_; + const scoped_refptr devtools_file_system_indexer_ = + base::MakeRefCounted(); - std::unique_ptr exclusive_access_manager_; + ExclusiveAccessManager exclusive_access_manager_{this}; std::unique_ptr eye_dropper_; @@ -832,10 +833,11 @@ class WebContents : public ExclusiveAccessContext, DevToolsIndexingJobsMap; DevToolsIndexingJobsMap devtools_indexing_jobs_; - scoped_refptr file_task_runner_; + const scoped_refptr file_task_runner_ = + base::ThreadPool::CreateSequencedTaskRunner({base::MayBlock()}); #if BUILDFLAG(ENABLE_PRINTING) - scoped_refptr print_task_runner_; + const scoped_refptr print_task_runner_; #endif // Stores the frame thats currently in fullscreen, nullptr if there is none.