[Extensions] Clean up WebRequest API IO thread-related cruft.

When the Web Request API was moved to the UI thread, there was a fair
amount clean up to do be done. This CL cleans up a bit more.

Bug: 980774
Change-Id: I6a4ab52eda0508aa14c677a6fd5fddc393d3e570
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1982687Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: David Bertoni <dbertoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#728086}

extensions/browser/event_router.cc

@@ -60,14 +60,13 @@ const char kFilteredServiceWorkerEvents[] = "filtered_service_worker_events";
 // Sends a notification about an event to the API activity monitor and the
 // ExtensionHost for |extension_id| on the UI thread. Can be called from any
 // thread.
-void NotifyEventDispatched(void* browser_context_id,
+void NotifyEventDispatched(content::BrowserContext* browser_context,
                            const std::string& extension_id,
                            const std::string& event_name,
                            const base::ListValue& args) {
   // Notify the ApiActivityMonitor about the event dispatch.
-  BrowserContext* context = static_cast<BrowserContext*>(browser_context_id);
-  activity_monitor::OnApiEventDispatched(context, extension_id, event_name,
-                                         args);
+  activity_monitor::OnApiEventDispatched(browser_context, extension_id,
+                                         event_name, args);
 }
 
 LazyContextId LazyContextIdForBrowserContext(BrowserContext* browser_context,
@@ -93,17 +92,17 @@ const char EventRouter::kRegisteredServiceWorkerEvents[] =
     "serviceworkerevents";
 
 // static
-void EventRouter::DispatchExtensionMessage(IPC::Sender* ipc_sender,
+void EventRouter::DispatchExtensionMessage(
+    IPC::Sender* ipc_sender,
     int worker_thread_id,
-                                           void* browser_context_id,
+    content::BrowserContext* browser_context,
     const std::string& extension_id,
     int event_id,
     const std::string& event_name,
     ListValue* event_args,
     UserGestureState user_gesture,
     const EventFilteringInfo& info) {
-  NotifyEventDispatched(browser_context_id, extension_id, event_name,
-                        *event_args);
+  NotifyEventDispatched(browser_context, extension_id, event_name, *event_args);
   ExtensionMsg_DispatchEvent_Params params;
   params.worker_thread_id = worker_thread_id;
   params.extension_id = extension_id;
@@ -127,8 +126,9 @@ std::string EventRouter::GetBaseEventName(const std::string& full_event_name) {
 }
 
 // static
-void EventRouter::DispatchEventToSender(IPC::Sender* ipc_sender,
-                                        void* browser_context_id,
+void EventRouter::DispatchEventToSender(
+    IPC::Sender* ipc_sender,
+    content::BrowserContext* browser_context,
     const std::string& extension_id,
     events::HistogramValue histogram_value,
     const std::string& event_name,
@@ -140,11 +140,11 @@ void EventRouter::DispatchEventToSender(IPC::Sender* ipc_sender,
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   int event_id = g_extension_event_id.GetNext();
 
-  DoDispatchEventToSenderBookkeepingOnUI(
-      browser_context_id, extension_id, event_id, render_process_id,
+  DoDispatchEventToSenderBookkeeping(
+      browser_context, extension_id, event_id, render_process_id,
       service_worker_version_id, histogram_value, event_name);
 
-  DispatchExtensionMessage(ipc_sender, worker_thread_id, browser_context_id,
+  DispatchExtensionMessage(ipc_sender, worker_thread_id, browser_context,
                            extension_id, event_id, event_name, event_args.get(),
                            UserGestureState::USER_GESTURE_UNKNOWN, info);
 }
@@ -693,8 +693,8 @@ void EventRouter::DispatchEventToProcess(
 }
 
 // static
-void EventRouter::DoDispatchEventToSenderBookkeepingOnUI(
-    void* browser_context_id,
+void EventRouter::DoDispatchEventToSenderBookkeeping(
+    content::BrowserContext* browser_context,
     const std::string& extension_id,
     int event_id,
     int render_process_id,
@@ -702,8 +702,6 @@ void EventRouter::DoDispatchEventToSenderBookkeepingOnUI(
     events::HistogramValue histogram_value,
     const std::string& event_name) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  BrowserContext* browser_context =
-      reinterpret_cast<BrowserContext*>(browser_context_id);
   // TODO(https://crbug.com/897946): Remove after investigating the bug.
   if (ExtensionsBrowserClient::Get()->IsShuttingDown()) {
     LOG(ERROR)

extensions/browser/event_router.h

@@ -107,7 +107,7 @@ class EventRouter : public KeyedService,
   // Note that this method will dispatch the event with
   // UserGestureState:USER_GESTURE_UNKNOWN.
   static void DispatchEventToSender(IPC::Sender* ipc_sender,
-                                    void* browser_context_id,
+                                    content::BrowserContext* browser_context,
                                     const std::string& extension_id,
                                     events::HistogramValue histogram_value,
                                     const std::string& event_name,
@@ -277,7 +277,7 @@ class EventRouter : public KeyedService,
   static void DispatchExtensionMessage(
       IPC::Sender* ipc_sender,
       int worker_thread_id,
-      void* browser_context_id,
+      content::BrowserContext* browser_context,
       const std::string& extension_id,
       int event_id,
       const std::string& event_name,
@@ -350,8 +350,8 @@ class EventRouter : public KeyedService,
                                int64_t service_worker_version_id);
 
   // static
-  static void DoDispatchEventToSenderBookkeepingOnUI(
-      void* browser_context_id,
+  static void DoDispatchEventToSenderBookkeeping(
+      content::BrowserContext* context,
       const std::string& extension_id,
       int event_id,
       int render_process_id,

extensions/browser/extension_function_dispatcher.cc

@@ -67,41 +67,12 @@ bool IsRequestFromServiceWorker(
          blink::mojom::kInvalidServiceWorkerVersionId;
 }
 
-void CommonResponseCallback(IPC::Sender* ipc_sender,
-                            int routing_id,
-                            int worker_thread_id,
-                            int request_id,
-                            ExtensionFunction::ResponseType type,
-                            const base::ListValue& results,
-                            const std::string& error) {
-  DCHECK(ipc_sender);
-
-  if (type == ExtensionFunction::BAD_MESSAGE) {
-    // The renderer will be shut down from ExtensionFunction::SetBadMessage().
-    return;
-  }
-
-  if (routing_id != MSG_ROUTING_NONE) {
-    DCHECK_EQ(kMainThreadId, worker_thread_id);
-    ipc_sender->Send(new ExtensionMsg_Response(
-        routing_id, request_id, type == ExtensionFunction::SUCCEEDED, results,
-        error));
-  } else {
-    DCHECK_NE(kMainThreadId, worker_thread_id);
-    ipc_sender->Send(new ExtensionMsg_ResponseWorker(
-        worker_thread_id, request_id, type == ExtensionFunction::SUCCEEDED,
-        results, error));
-  }
-}
-
 }  // namespace
 
-// TODO(http://crbug.com/980774): Simplify this or change the name now that
-// IOThreadExtensionFunction is gone.
-class ExtensionFunctionDispatcher::UIThreadResponseCallbackWrapper
+class ExtensionFunctionDispatcher::ResponseCallbackWrapper
     : public content::WebContentsObserver {
  public:
-  UIThreadResponseCallbackWrapper(
+  ResponseCallbackWrapper(
       const base::WeakPtr<ExtensionFunctionDispatcher>& dispatcher,
       content::RenderFrameHost* render_frame_host)
       : content::WebContentsObserver(
@@ -109,26 +80,22 @@ class ExtensionFunctionDispatcher::UIThreadResponseCallbackWrapper
         dispatcher_(dispatcher),
         render_frame_host_(render_frame_host) {}
 
-  ~UIThreadResponseCallbackWrapper() override {}
+  ~ResponseCallbackWrapper() override = default;
 
   // content::WebContentsObserver overrides.
   void RenderFrameDeleted(
       content::RenderFrameHost* render_frame_host) override {
-    DCHECK_CURRENTLY_ON(BrowserThread::UI);
     if (render_frame_host != render_frame_host_)
       return;
 
     if (dispatcher_.get()) {
-      dispatcher_->ui_thread_response_callback_wrappers_
-          .erase(render_frame_host);
+      dispatcher_->response_callback_wrappers_.erase(render_frame_host);
     }
   }
 
   ExtensionFunction::ResponseCallback CreateCallback(int request_id) {
-    return base::Bind(
-        &UIThreadResponseCallbackWrapper::OnExtensionFunctionCompleted,
-        weak_ptr_factory_.GetWeakPtr(),
-        request_id);
+    return base::Bind(&ResponseCallbackWrapper::OnExtensionFunctionCompleted,
+                      weak_ptr_factory_.GetWeakPtr(), request_id);
   }
 
  private:
@@ -136,22 +103,27 @@ class ExtensionFunctionDispatcher::UIThreadResponseCallbackWrapper
                                     ExtensionFunction::ResponseType type,
                                     const base::ListValue& results,
                                     const std::string& error) {
-    CommonResponseCallback(render_frame_host_,
-                           render_frame_host_->GetRoutingID(), kMainThreadId,
-                           request_id, type, results, error);
+    if (type == ExtensionFunction::BAD_MESSAGE) {
+      // The renderer will be shut down from ExtensionFunction::SetBadMessage().
+      return;
+    }
+
+    render_frame_host_->Send(new ExtensionMsg_Response(
+        render_frame_host_->GetRoutingID(), request_id,
+        type == ExtensionFunction::SUCCEEDED, results, error));
   }
 
   base::WeakPtr<ExtensionFunctionDispatcher> dispatcher_;
   content::RenderFrameHost* render_frame_host_;
-  base::WeakPtrFactory<UIThreadResponseCallbackWrapper> weak_ptr_factory_{this};
+  base::WeakPtrFactory<ResponseCallbackWrapper> weak_ptr_factory_{this};
 
-  DISALLOW_COPY_AND_ASSIGN(UIThreadResponseCallbackWrapper);
+  DISALLOW_COPY_AND_ASSIGN(ResponseCallbackWrapper);
 };
 
-class ExtensionFunctionDispatcher::UIThreadWorkerResponseCallbackWrapper
+class ExtensionFunctionDispatcher::WorkerResponseCallbackWrapper
     : public content::RenderProcessHostObserver {
  public:
-  UIThreadWorkerResponseCallbackWrapper(
+  WorkerResponseCallbackWrapper(
       const base::WeakPtr<ExtensionFunctionDispatcher>& dispatcher,
       content::RenderProcessHost* render_process_host,
       int worker_thread_id)
@@ -164,7 +136,7 @@ class ExtensionFunctionDispatcher::UIThreadWorkerResponseCallbackWrapper
                ->ExtensionAPIEnabledInExtensionServiceWorkers());
   }
 
-  ~UIThreadWorkerResponseCallbackWrapper() override {}
+  ~WorkerResponseCallbackWrapper() override = default;
 
   // content::RenderProcessHostObserver override.
   void RenderProcessExited(
@@ -181,13 +153,12 @@ class ExtensionFunctionDispatcher::UIThreadWorkerResponseCallbackWrapper
   ExtensionFunction::ResponseCallback CreateCallback(int request_id,
                                                      int worker_thread_id) {
     return base::Bind(
-        &UIThreadWorkerResponseCallbackWrapper::OnExtensionFunctionCompleted,
+        &WorkerResponseCallbackWrapper::OnExtensionFunctionCompleted,
         weak_ptr_factory_.GetWeakPtr(), request_id, worker_thread_id);
   }
 
  private:
   void CleanUp() {
-    DCHECK_CURRENTLY_ON(BrowserThread::UI);
     if (dispatcher_) {
       dispatcher_->RemoveWorkerCallbacksForProcess(
           render_process_host_->GetID());
@@ -213,10 +184,9 @@ class ExtensionFunctionDispatcher::UIThreadWorkerResponseCallbackWrapper
   ScopedObserver<content::RenderProcessHost, content::RenderProcessHostObserver>
       observer_{this};
   content::RenderProcessHost* const render_process_host_;
-  base::WeakPtrFactory<UIThreadWorkerResponseCallbackWrapper> weak_ptr_factory_{
-      this};
+  base::WeakPtrFactory<WorkerResponseCallbackWrapper> weak_ptr_factory_{this};
 
-  DISALLOW_COPY_AND_ASSIGN(UIThreadWorkerResponseCallbackWrapper);
+  DISALLOW_COPY_AND_ASSIGN(WorkerResponseCallbackWrapper);
 };
 
 struct ExtensionFunctionDispatcher::WorkerResponseCallbackMapKey {
@@ -273,13 +243,13 @@ void ExtensionFunctionDispatcher::Dispatch(
   if (render_frame_host) {
     // Extension API from a non Service Worker context, e.g. extension page,
     // background page, content script.
-    UIThreadResponseCallbackWrapperMap::const_iterator iter =
-        ui_thread_response_callback_wrappers_.find(render_frame_host);
-    UIThreadResponseCallbackWrapper* callback_wrapper = nullptr;
-    if (iter == ui_thread_response_callback_wrappers_.end()) {
+    ResponseCallbackWrapperMap::const_iterator iter =
+        response_callback_wrappers_.find(render_frame_host);
+    ResponseCallbackWrapper* callback_wrapper = nullptr;
+    if (iter == response_callback_wrappers_.end()) {
       callback_wrapper =
-          new UIThreadResponseCallbackWrapper(AsWeakPtr(), render_frame_host);
-      ui_thread_response_callback_wrappers_[render_frame_host] =
+          new ResponseCallbackWrapper(AsWeakPtr(), render_frame_host);
+      response_callback_wrappers_[render_frame_host] =
           base::WrapUnique(callback_wrapper);
     } else {
       callback_wrapper = iter->second.get();
@@ -290,8 +260,7 @@ void ExtensionFunctionDispatcher::Dispatch(
   } else {
     content::RenderProcessHost* rph =
         content::RenderProcessHost::FromID(render_process_id);
-    // UIThreadWorkerResponseCallbackWrapper requires render process host to be
-    // around.
+    // WorkerResponseCallbackWrapper requires render process host to be around.
     if (!rph)
       return;
 
@@ -304,13 +273,13 @@ void ExtensionFunctionDispatcher::Dispatch(
 
     WorkerResponseCallbackMapKey key(render_process_id,
                                      params.service_worker_version_id);
-    UIThreadWorkerResponseCallbackWrapperMap::const_iterator iter =
-        ui_thread_response_callback_wrappers_for_worker_.find(key);
-    UIThreadWorkerResponseCallbackWrapper* callback_wrapper = nullptr;
-    if (iter == ui_thread_response_callback_wrappers_for_worker_.end()) {
-      callback_wrapper = new UIThreadWorkerResponseCallbackWrapper(
+    WorkerResponseCallbackWrapperMap::const_iterator iter =
+        response_callback_wrappers_for_worker_.find(key);
+    WorkerResponseCallbackWrapper* callback_wrapper = nullptr;
+    if (iter == response_callback_wrappers_for_worker_.end()) {
+      callback_wrapper = new WorkerResponseCallbackWrapper(
           AsWeakPtr(), rph, params.worker_thread_id);
-      ui_thread_response_callback_wrappers_for_worker_[key] =
+      response_callback_wrappers_for_worker_[key] =
           base::WrapUnique(callback_wrapper);
     } else {
       callback_wrapper = iter->second.get();
@@ -435,8 +404,8 @@ void ExtensionFunctionDispatcher::DispatchWithCallbackInternal(
 
 void ExtensionFunctionDispatcher::RemoveWorkerCallbacksForProcess(
     int render_process_id) {
-  UIThreadWorkerResponseCallbackWrapperMap& map =
-      ui_thread_response_callback_wrappers_for_worker_;
+  WorkerResponseCallbackWrapperMap& map =
+      response_callback_wrappers_for_worker_;
   for (auto it = map.begin(); it != map.end();) {
     if (it->first.render_process_id == render_process_id) {
       it = map.erase(it);

extensions/browser/extension_function_dispatcher.h

@@ -98,19 +98,19 @@ class ExtensionFunctionDispatcher
   void set_delegate(Delegate* delegate) { delegate_ = delegate; }
 
  private:
-  // For a given RenderFrameHost instance, UIThreadResponseCallbackWrapper
+  // For a given RenderFrameHost instance, ResponseCallbackWrapper
   // creates ExtensionFunction::ResponseCallback instances which send responses
   // to the corresponding render view in ExtensionMsg_Response messages.
   // This class tracks the lifespan of the RenderFrameHost instance, and will be
   // destroyed automatically when it goes away.
-  class UIThreadResponseCallbackWrapper;
+  class ResponseCallbackWrapper;
 
-  // Same as UIThreadResponseCallbackWrapper above, but applies to an extension
+  // Same as ResponseCallbackWrapper above, but applies to an extension
   // function from an extension Service Worker.
-  class UIThreadWorkerResponseCallbackWrapper;
+  class WorkerResponseCallbackWrapper;
 
-  // Key used to store UIThreadWorkerResponseCallbackWrapper in the map
-  // |ui_thread_response_callback_wrappers_for_worker_|.
+  // Key used to store WorkerResponseCallbackWrapper in the map
+  // |response_callback_wrappers_for_worker_|.
   struct WorkerResponseCallbackMapKey;
 
   // Helper to check whether an ExtensionFunction has the required permissions.
@@ -155,17 +155,16 @@ class ExtensionFunctionDispatcher
   // instance goes away, the corresponding entry in this map (if exists) will be
   // removed.
   typedef std::map<content::RenderFrameHost*,
-                   std::unique_ptr<UIThreadResponseCallbackWrapper>>
-      UIThreadResponseCallbackWrapperMap;
-  UIThreadResponseCallbackWrapperMap ui_thread_response_callback_wrappers_;
+                   std::unique_ptr<ResponseCallbackWrapper>>
+      ResponseCallbackWrapperMap;
+  ResponseCallbackWrapperMap response_callback_wrappers_;
 
-  using UIThreadWorkerResponseCallbackWrapperMap =
+  using WorkerResponseCallbackWrapperMap =
       std::map<WorkerResponseCallbackMapKey,
-               std::unique_ptr<UIThreadWorkerResponseCallbackWrapper>>;
+               std::unique_ptr<WorkerResponseCallbackWrapper>>;
   // TODO(lazyboy): The map entries are cleared upon RenderProcessHost shutown,
   // we should really be clearing it on service worker shutdown.
-  UIThreadWorkerResponseCallbackWrapperMap
-      ui_thread_response_callback_wrappers_for_worker_;
+  WorkerResponseCallbackWrapperMap response_callback_wrappers_for_worker_;
 };
 
 }  // namespace extensions