This patch allows WebViewRendererState to be accessible from both the UI and IO threads. This access is mediated by locks for the two internal maps in the class.

This patch is tested by a sebsequent patch that depends on it: https://codereview.chromium.org/1312653003/

Review URL: https://codereview.chromium.org/1313673003

Cr-Commit-Position: refs/heads/master@{#345917}

2 files changed, 34 insertions, 15 deletions

diff --git a/extensions/browser/guest_view/web_view/web_view_renderer_state.cc b/extensions/browser/guest_view/web_view/web_view_renderer_state.cc
index 3068ae3..aa0ee09 100644
--- a/extensions/browser/guest_view/web_view/web_view_renderer_state.cc
+++ b/extensions/browser/guest_view/web_view/web_view_renderer_state.cc
@@ -26,8 +26,8 @@ WebViewRendererState::WebViewRendererState() {
 WebViewRendererState::~WebViewRendererState() {
 }
 
-bool WebViewRendererState::IsGuest(int render_process_id) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+bool WebViewRendererState::IsGuest(int render_process_id) const {
+  base::AutoLock auto_lock(web_view_partition_id_map_lock_);
   return web_view_partition_id_map_.find(render_process_id) !=
          web_view_partition_id_map_.end();
 }
@@ -35,7 +35,9 @@ bool WebViewRendererState::IsGuest(int render_process_id) {
 void WebViewRendererState::AddGuest(int guest_process_id,
                                     int guest_routing_id,
                                     const WebViewInfo& web_view_info) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  base::AutoLock auto_lock(web_view_info_map_lock_);
+  base::AutoLock auto_lock2(web_view_partition_id_map_lock_);
+
   RenderId render_id(guest_process_id, guest_routing_id);
   bool updating =
       web_view_info_map_.find(render_id) != web_view_info_map_.end();
@@ -54,7 +56,9 @@ void WebViewRendererState::AddGuest(int guest_process_id,
 
 void WebViewRendererState::RemoveGuest(int guest_process_id,
                                        int guest_routing_id) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  base::AutoLock auto_lock(web_view_info_map_lock_);
+  base::AutoLock auto_lock2(web_view_partition_id_map_lock_);
+
   RenderId render_id(guest_process_id, guest_routing_id);
   web_view_info_map_.erase(render_id);
   auto iter = web_view_partition_id_map_.find(guest_process_id);
@@ -68,8 +72,9 @@ void WebViewRendererState::RemoveGuest(int guest_process_id,
 
 bool WebViewRendererState::GetInfo(int guest_process_id,
                                    int guest_routing_id,
-                                   WebViewInfo* web_view_info) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+                                   WebViewInfo* web_view_info) const {
+  base::AutoLock auto_lock(web_view_info_map_lock_);
+
   RenderId render_id(guest_process_id, guest_routing_id);
   auto iter = web_view_info_map_.find(render_id);
   if (iter != web_view_info_map_.end()) {
@@ -82,7 +87,8 @@ bool WebViewRendererState::GetInfo(int guest_process_id,
 bool WebViewRendererState::GetOwnerInfo(int guest_process_id,
                                         int* owner_process_id,
                                         std::string* owner_host) const {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  base::AutoLock auto_lock(web_view_info_map_lock_);
+
   // TODO(fsamuel): Store per-process info in WebViewPartitionInfo instead of in
   // WebViewInfo.
   for (const auto& info : web_view_info_map_) {
@@ -98,8 +104,8 @@ bool WebViewRendererState::GetOwnerInfo(int guest_process_id,
 }
 
 bool WebViewRendererState::GetPartitionID(int guest_process_id,
-                                          std::string* partition_id) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+                                          std::string* partition_id) const {
+  base::AutoLock auto_lock(web_view_partition_id_map_lock_);
 
   auto iter = web_view_partition_id_map_.find(guest_process_id);
   if (iter != web_view_partition_id_map_.end()){
@@ -113,7 +119,7 @@ void WebViewRendererState::AddContentScriptIDs(
     int embedder_process_id,
     int view_instance_id,
     const std::set<int>& script_ids) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  base::AutoLock auto_lock(web_view_info_map_lock_);
 
   for (auto& render_id_info : web_view_info_map_) {
     WebViewInfo& info = render_id_info.second;
@@ -130,7 +136,7 @@ void WebViewRendererState::RemoveContentScriptIDs(
     int embedder_process_id,
     int view_instance_id,
     const std::set<int>& script_ids) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  base::AutoLock auto_lock(web_view_info_map_lock_);
 
   for (auto& render_id_info : web_view_info_map_) {
     WebViewInfo& info = render_id_info.second;
diff --git a/extensions/browser/guest_view/web_view/web_view_renderer_state.h b/extensions/browser/guest_view/web_view/web_view_renderer_state.h
index b6be959..c43afd2 100644
--- a/extensions/browser/guest_view/web_view/web_view_renderer_state.h
+++ b/extensions/browser/guest_view/web_view/web_view_renderer_state.h
@@ -2,6 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// WebViewRendererState manages state data for WebView guest renderer processes.
+//
+// This class's data can be accessed via its methods from both the UI and IO
+// threads, and uses locks to mediate this access. When making changes to this
+// class, ensure that you avoid introducing any reentrant code in the methods,
+// and that you always aquire the locks in the order |web_view_info_map_lock_|
+// -> |web_view_partition_id_map_lock_| (if both are needed in one method).
+
 #ifndef EXTENSIONS_BROWSER_GUEST_VIEW_WEB_VIEW_WEB_VIEW_RENDERER_STATE_H_
 #define EXTENSIONS_BROWSER_GUEST_VIEW_WEB_VIEW_WEB_VIEW_RENDERER_STATE_H_
 
@@ -36,8 +44,9 @@ class WebViewRendererState {
 
   // Looks up the information for the embedder <webview> for a given render
   // view, if one exists. Called on the IO thread.
-  bool GetInfo(int guest_process_id, int guest_routing_id,
-               WebViewInfo* web_view_info);
+  bool GetInfo(int guest_process_id,
+               int guest_routing_id,
+               WebViewInfo* web_view_info) const;
 
   // Looks up the information for the owner for a given guest process in a
   // <webview>. Called on the IO thread.
@@ -47,10 +56,10 @@ class WebViewRendererState {
 
   // Looks up the partition info for the embedder <webview> for a given guest
   // process. Called on the IO thread.
-  bool GetPartitionID(int guest_process_id, std::string* partition_id);
+  bool GetPartitionID(int guest_process_id, std::string* partition_id) const;
 
   // Returns true if the given renderer is used by webviews.
-  bool IsGuest(int render_process_id);
+  bool IsGuest(int render_process_id) const;
 
   void AddContentScriptIDs(int embedder_process_id,
                            int view_instance_id,
@@ -85,8 +94,12 @@ class WebViewRendererState {
                 const WebViewInfo& web_view_info);
   void RemoveGuest(int render_process_host_id, int routing_id);
 
+  // Locks are used to mediate access to these maps from both the UI and IO
+  // threads.
   WebViewInfoMap web_view_info_map_;
+  mutable base::Lock web_view_info_map_lock_;
   WebViewPartitionIDMap web_view_partition_id_map_;
+  mutable base::Lock web_view_partition_id_map_lock_;
 
   DISALLOW_COPY_AND_ASSIGN(WebViewRendererState);
 };