ChromeContentRendererClient should not rely on Dispatcher::is_extension_process

chrome/renderer/chrome_content_renderer_client.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/renderer/chrome_content_renderer_client.h"
 
 #include "base/command_line.h"
 #include "base/debug/crash_logging.h"
 #include "base/logging.h"
 #include "base/metrics/field_trial.h"
@@ skipping 265 matching lines @@
   for (size_t i = 0; i < params.attributeNames.size(); ++i) {
     if (params.attributeNames[i].utf8() == "poster" &&
         !params.attributeValues[i].isEmpty()) {
       return page_base_url.Resolve(params.attributeValues[i].utf8());
     }
   }
   return GURL();
 }
 #endif
 
+bool IsSingleProcess() {

[Lei Zhang, 2015/02/26 22:17:58]

Given you want to do this check in every place in the file that also calls
is_extension_process(), you probably want to add a method to
ChromeContentRendererClient to do both.

[not at google - send to devlin, 2015/02/26 22:25:03]

I don't know this code that well either, but note that in dispatcher.cc we
already lie about it being an extension process in single process mode:

https://code.google.com/p/chromium/codesearch#chromium/src/extensions/rendere...

and all it uses that for is deciding whether to monitor idle time.

It may be more appropriate to change dispatcher.cc rather than this file.

[Devlin, 2015/02/27 16:54:32]

Two things:
- I tend to agree with Ben - if "lying" in the dispatcher is causing us to have
to check IsSingleProcess at every point here, then the problem sounds like it's
in dispatcher, not here.
- This function looks wrong - shouldn't IsSingleProcess return true if the
command line *does* have the single process flag?

[João Eiras, 2015/03/02 17:02:48]

I changed the dispatcher code then. Thanks for your input.
Was wrong yes. But now the code is different.

+  return !base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kSingleProcess);
+}
+
 }  // namespace
 
 ChromeContentRendererClient::ChromeContentRendererClient() {
   g_current_client = this;
 
 #if defined(ENABLE_EXTENSIONS)
   extensions::ExtensionsClient::Set(
       extensions::ChromeExtensionsClient::GetInstance());
   extensions::ExtensionsRendererClient::Set(
       ChromeExtensionsRendererClient::GetInstance());
@@ skipping 857 matching lines @@
   }
 
   if (error_description) {
     if (!extension)
       *error_description = LocalizedError::GetErrorDetails(error, is_post);
   }
 }
 
 bool ChromeContentRendererClient::RunIdleHandlerWhenWidgetsHidden() {
 #if defined(ENABLE_EXTENSIONS)
-  return !extension_dispatcher_->is_extension_process();
+  return !extension_dispatcher_->is_extension_process() || IsSingleProcess();
 #else
   return true;
 #endif
 }
 
 bool ChromeContentRendererClient::AllowPopup() {
 #if defined(ENABLE_EXTENSIONS)
   extensions::ScriptContext* current_context =
       extension_dispatcher_->script_context_set().GetCurrent();
   if (!current_context || !current_context->extension())
@@ skipping 94 matching lines @@
           extension_misc::APP_LAUNCH_CONTENT_NAVIGATION, extension->GetType());
     }
     return true;
   }
 
   // If this is a reload, check whether it has the wrong process type.  We
   // should send it to the browser if it's an extension URL (e.g., hosted app)
   // in a normal process, or if it's a process for an extension that has been
   // uninstalled.
   if (frame->top()->document().url() == url) {
-    if (is_extension_url != extension_dispatcher_->is_extension_process())
+    if (is_extension_url != extension_dispatcher_->is_extension_process() &&
+        !IsSingleProcess())
       return true;
   }
 #endif  // defined(ENABLE_EXTENSIONS)
 
   return false;
 }
 
 #if defined(ENABLE_EXTENSIONS)
 bool ChromeContentRendererClient::ShouldForwardToGuestContainer(
     const IPC::Message& msg) {
@@ skipping 104 matching lines @@
     // in an extension process, we want to keep it in process to allow the
     // opener to script it.
     WebDocument opener_document = frame->opener()->document();
     WebSecurityOrigin opener = frame->opener()->document().securityOrigin();
     bool opener_is_extension_url =
         !opener.isUnique() && extensions.GetExtensionOrAppByURL(
             opener_document.url()) != NULL;
     if (!is_extension_url &&
         !opener_is_extension_url &&
         extension_dispatcher_->is_extension_process() &&
+        !IsSingleProcess() &&
         opener.canRequest(WebURL(new_url)))
       return false;
 
     // In all other cases, we want to compare against the top frame's URL (as
     // opposed to the opener frame's), since that's what determines the type of
     // process.  This allows iframes outside an app to open a popup in the app.
     old_url = frame->top()->opener()->top()->document().url();
   }
 
   // Only consider keeping non-app URLs in an app process if this window
@@ skipping 167 matching lines @@
   if (mime_type == content::kBrowserPluginMimeType) {
     return new extensions::ExtensionsGuestViewContainer(render_frame);
   } else {
     return new extensions::MimeHandlerViewContainer(
         render_frame, mime_type, original_url);
   }
 #else
   return NULL;
 #endif
 }