Service CreateNewWindow on the UI thread with a new mojo interface

chrome/browser/android/webapps/single_tab_mode_tab_helper.cc

Index: chrome/browser/android/webapps/single_tab_mode_tab_helper.cc
diff --git a/chrome/browser/android/webapps/single_tab_mode_tab_helper.cc b/chrome/browser/android/webapps/single_tab_mode_tab_helper.cc
index 85f4755f4d300ac4d957b768728ad81174254ee1..10ebded82753c486f9bd5ed5990a19e53b545977 100644
--- a/chrome/browser/android/webapps/single_tab_mode_tab_helper.cc
+++ b/chrome/browser/android/webapps/single_tab_mode_tab_helper.cc
@@ -15,34 +15,18 @@ DEFINE_WEB_CONTENTS_USER_DATA_KEY(SingleTabModeTabHelper);
 
 namespace {
 
+// TODO(csharrison): This should be a content::GlobalFrameRoutingId.
 typedef std::pair<int32_t, int32_t> RenderFrameHostID;
 typedef std::set<RenderFrameHostID> SingleTabIDSet;
 base::LazyInstance<SingleTabIDSet>::DestructorAtExit g_blocked_ids =
     LAZY_INSTANCE_INITIALIZER;
 
-void AddPairOnIOThread(int32_t process_id, int32_t frame_routing_id) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
-  RenderFrameHostID single_tab_pair(process_id, frame_routing_id);
-  g_blocked_ids.Get().insert(single_tab_pair);
-}
-
-void RemovePairOnIOThread(int32_t process_id, int32_t frame_routing_id) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
-  RenderFrameHostID single_tab_pair(process_id, frame_routing_id);
-  SingleTabIDSet::iterator itr = g_blocked_ids.Get().find(single_tab_pair);
-  DCHECK(itr != g_blocked_ids.Get().end());
-  g_blocked_ids.Get().erase(itr);
-}
-
 void AddPair(content::RenderFrameHost* render_frame_host) {
   if (!render_frame_host)
     return;
-
   int32_t process_id = render_frame_host->GetProcess()->GetID();
   int32_t frame_routing_id = render_frame_host->GetRoutingID();
-  content::BrowserThread::PostTask(
-      content::BrowserThread::IO, FROM_HERE,
-      base::Bind(&AddPairOnIOThread, process_id, frame_routing_id));
+  g_blocked_ids.Get().insert(RenderFrameHostID(process_id, frame_routing_id));
 }
 
 void RemovePair(content::RenderFrameHost* render_frame_host) {
@@ -51,9 +35,10 @@ void RemovePair(content::RenderFrameHost* render_frame_host) {
 
   int32_t process_id = render_frame_host->GetProcess()->GetID();
   int32_t frame_routing_id = render_frame_host->GetRoutingID();
-  content::BrowserThread::PostTask(
-      content::BrowserThread::IO, FROM_HERE,
-      base::Bind(&RemovePairOnIOThread, process_id, frame_routing_id));
+  RenderFrameHostID single_tab_pair(process_id, frame_routing_id);
+  SingleTabIDSet::iterator itr = g_blocked_ids.Get().find(single_tab_pair);
+  DCHECK(itr != g_blocked_ids.Get().end());
+  g_blocked_ids.Get().erase(itr);
 }
 
 }  // namespace
@@ -89,7 +74,7 @@ void SingleTabModeTabHelper::PermanentlyBlockAllNewWindows() {
 
 bool SingleTabModeTabHelper::IsRegistered(int32_t process_id,
                                           int32_t frame_routing_id) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   RenderFrameHostID single_tab_pair(process_id, frame_routing_id);
   SingleTabIDSet::iterator itr = g_blocked_ids.Get().find(single_tab_pair);
   return itr != g_blocked_ids.Get().end();

[ncarter (slow), 2017/04/17 20:44:26]

On the UI thread, I think we we don't need g_blocked_ids/AddPair/RemovePair at
all?

Instead we can just do something like this:

  WebContents* web_contents =
WebContents::FromRenderFrameHost(RenderFrameHost::FromID(process_id,
frame_routing_id));
  if (web_contents) {
    SingleTabModeTabHelper* tab_helper =
      SingleTabModeTabHelper::FromWebContents(web_contents);
    return tab_helper && tab_helper->block_all_new_windows_;
  }

If that works, then we can delete both the globals and the WebContentsObserver
methods.

[Charlie Harrison, 2017/04/18 20:38:52]

Looks good, done.