Extensions: Only load incognito-enabled extensions in an incognito renderer.

extensions/browser/renderer_startup_helper.cc

 // Copyright 2014 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 "extensions/browser/renderer_startup_helper.h"
 
 #include "base/stl_util.h"
 #include "base/values.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
+#include "content/public/browser/browser_context.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_process_host.h"
 #include "extensions/browser/extension_function_dispatcher.h"
 #include "extensions/browser/extension_registry.h"
+#include "extensions/browser/extension_util.h"
 #include "extensions/browser/extensions_browser_client.h"
 #include "extensions/browser/guest_view/web_view/web_view_guest.h"
 #include "extensions/common/extension_messages.h"
 #include "extensions/common/extension_set.h"
 #include "extensions/common/extensions_client.h"
 #include "extensions/common/features/feature_channel.h"
 #include "extensions/common/features/feature_session_type.h"
 #include "ui/base/webui/web_ui_util.h"
 
 using content::BrowserContext;
 
 namespace extensions {
 
+namespace {
+
+// Returns whether the |extension| should be loaded in the given
+// renderer |process|.
+bool IsExtensionVisibleToProcess(const Extension& extension,
+                                 content::RenderProcessHost* process) {
+  // Renderers don't need to know about themes.
+  if (extension.is_theme())
+    return false;
+
+  // Only extensions enabled in incognito mode should be loaded in an incognito
+  // renderer. However extensions which can't be loaded in the incognito mode
+  // (e.g. platform apps) should also be loaded in an incognito renderer to
+  // ensure connections from incognito tabs to such extensions work fine.

[Devlin, 2017/03/23 22:08:37]

nitty nit: omit ' fine' -> 'ensure connections from incognito tabs to such
extensions work.'

[karandeepb, 2017/04/04 03:44:15]

Done.

+  content::BrowserContext* browser_context = process->GetBrowserContext();
+  return !browser_context->IsOffTheRecord() ||
+         !util::CanBeIncognitoEnabled(&extension) ||
+         util::IsIncognitoEnabled(extension.id(), browser_context);
+}
+
+}  // namespace
+
 RendererStartupHelper::RendererStartupHelper(BrowserContext* browser_context)
     : browser_context_(browser_context) {
   DCHECK(browser_context);
   registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_CREATED,
                  content::NotificationService::AllBrowserContextsAndSources());
   registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_TERMINATED,
                  content::NotificationService::AllBrowserContextsAndSources());
 }
 
 RendererStartupHelper::~RendererStartupHelper() {}
@@ skipping 53 matching lines @@
     process->Send(new ExtensionMsg_SetWebViewPartitionID(
         WebViewGuest::GetPartitionID(process)));
   }
 
   std::set<ExtensionId>* loaded_extensions = &loaded_extensions_[process];
   // Load extensions.
   std::vector<ExtensionMsg_Loaded_Params> loaded_extensions_params;
   const ExtensionSet& extensions =
       ExtensionRegistry::Get(browser_context_)->enabled_extensions();
   for (const auto& ext : extensions) {
-    // Renderers don't need to know about themes.
-    if (!ext->is_theme()) {
-      // TODO(kalman): Only include tab specific permissions for extension
-      // processes, no other process needs it, so it's mildly wasteful.
-      // I am not sure this is possible to know this here, at such a low
-      // level of the stack. Perhaps site isolation can help.
-      bool include_tab_permissions = true;
-      loaded_extensions_params.push_back(
-          ExtensionMsg_Loaded_Params(ext.get(), include_tab_permissions));
-      loaded_extensions->insert(ext->id());
-    }
+    if (!IsExtensionVisibleToProcess(*ext, process))

[Devlin, 2017/03/23 22:08:37]

nitty nit: We actually only need the browser context here, so we can cache it
before iterating through the loop and just pass it directly, saving the
(admittedly very cheap) repeated call to RenderProcessHost::GetBrowserContext().

(Note that you'll still want to use process->GetBrowserContext() when you cache
it, rather than passing in browser_context_, since this handles both on- and
off-the-record versions.)

[karandeepb, 2017/04/04 03:44:15]

Done.

+      continue;
+
+    // TODO(kalman): Only include tab specific permissions for extension
+    // processes, no other process needs it, so it's mildly wasteful.
+    // I am not sure this is possible to know this here, at such a low
+    // level of the stack. Perhaps site isolation can help.
+    bool include_tab_permissions = true;
+    loaded_extensions_params.push_back(
+        ExtensionMsg_Loaded_Params(ext.get(), include_tab_permissions));
+    loaded_extensions->insert(ext->id());
   }
 
   // Activate pending extensions.
   process->Send(new ExtensionMsg_Loaded(loaded_extensions_params));
   auto iter = pending_active_extensions_.find(process);
   if (iter != pending_active_extensions_.end()) {
     for (const ExtensionId& id : iter->second) {
       DCHECK(extensions.Contains(id));
       process->Send(new ExtensionMsg_ActivateExtension(id));
     }
   }
 
   pending_active_extensions_.erase(process);
 }
 
 void RendererStartupHelper::UntrackProcess(
     content::RenderProcessHost* process) {
   if (!ExtensionsBrowserClient::Get()->IsSameContext(
           browser_context_, process->GetBrowserContext())) {
     return;
   }
 
   loaded_extensions_.erase(process);
   pending_active_extensions_.erase(process);
 }
 
 void RendererStartupHelper::ActivateExtensionInProcess(
     const Extension& extension,
     content::RenderProcessHost* process) {
-  // Renderers don't need to know about themes. We also don't normally
-  // "activate" themes, but this could happen if someone tries to open a tab
-  // to the e.g. theme's manifest.
-  if (extension.is_theme())
+  if (!IsExtensionVisibleToProcess(extension, process))
     return;
 
   const auto& process_extensions_pair = loaded_extensions_.find(process);
   if (process_extensions_pair != loaded_extensions_.end()) {
     // The extension should have already been loaded in the process.
     if (!base::ContainsKey(process_extensions_pair->second, extension.id())) {
       NOTREACHED() << "Extension " << extension.id()
                    << " was not loaded for activation";
       return;
     }
     process->Send(new ExtensionMsg_ActivateExtension(extension.id()));
   } else {
     pending_active_extensions_[process].insert(extension.id());
   }
 }
 
 void RendererStartupHelper::OnExtensionLoaded(const Extension& extension) {
-  // Renderers don't need to know about themes.
-  if (extension.is_theme())
-    return;

[Devlin, 2017/03/23 22:08:37]

I wonder if it's worth keeping this short-circuit, since the creation of
ExtensionMsg_Loaded_Params is non-trivial, and we know that no process ever
needs to know about themes.

[karandeepb, 2017/04/04 03:44:15]

Centralizing the logic in IsExtensionVisibleToProcess seemed better to me.
Though don't mind changing it either.

[Devlin, 2017/04/04 15:15:05]

Given it's somewhat performance-sensitive code, let's keep it with a comment
like:
// IsExtensionVisibleToProcess() would filter out themes, but we choose
// to early-out for performance reasons.

-
   // We don't need to include tab permisisons here, since the extension
   // was just loaded.
   // Uninitialized renderers will be informed of the extension load during the
   // first batch of messages.
   std::vector<ExtensionMsg_Loaded_Params> params(
       1,
       ExtensionMsg_Loaded_Params(&extension, false /* no tab permissions */));
 
   for (auto& process_extensions_pair : loaded_extensions_) {
     // If extension is already loaded in process, do nothing.
     if (base::ContainsKey(process_extensions_pair.second, extension.id()))
       continue;
 
     content::RenderProcessHost* process = process_extensions_pair.first;
+    if (!IsExtensionVisibleToProcess(extension, process))
+      continue;
+
     process->Send(new ExtensionMsg_Loaded(params));
     process_extensions_pair.second.insert(extension.id());
   }
 }
 
 void RendererStartupHelper::OnExtensionUnloaded(const Extension& extension) {
   for (auto& process_extensions_pair : loaded_extensions_) {
     // If extension is already unloaded, do nothing.
     if (!base::ContainsKey(process_extensions_pair.second, extension.id()))
       continue;
@@ skipping 39 matching lines @@
     BrowserContext* context) const {
   // Redirected in incognito.
   return ExtensionsBrowserClient::Get()->GetOriginalContext(context);
 }
 
 bool RendererStartupHelperFactory::ServiceIsCreatedWithBrowserContext() const {
   return true;
 }
 
 }  // namespace extensions