<webview>: Inject denyWebView into all extensions

chrome/renderer/extensions/dispatcher.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/extensions/dispatcher.h"
 
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/debug/alias.h"
 #include "base/json/json_reader.h"
@@ skipping 1168 matching lines @@
   }
 
   if (context_type == Feature::BLESSED_EXTENSION_CONTEXT &&
       is_within_platform_app &&
       GetCurrentChannel() <= chrome::VersionInfo::CHANNEL_DEV &&
       CommandLine::ForCurrentProcess()->HasSwitch(
           ::switches::kEnableAppWindowControls)) {
     module_system->Require("windowControls");
   }
 
-  // Currently only platform apps and whitelisted component extensions support
-  // the <webview> tag, because the "denyWebView" module will affect the
-  // performance of DOM modifications (http://crbug.com/196453).
   // We used to limit WebView to |BLESSED_EXTENSION_CONTEXT| within platform
   // apps. An ext/app runs in a blessed extension context, if it is the active
   // extension in the current process, in other words, if it is loaded in a top
   // frame. To support webview in a non-frame extension, we have to allow
   // unblessed extension context as well.
-  if (context_type == Feature::BLESSED_EXTENSION_CONTEXT ||
-      context_type == Feature::UNBLESSED_EXTENSION_CONTEXT) {
-    // Note: setting up the WebView class here, not the chrome.webview API.
-    // The API will be automatically set up when first used.
+  // Note: setting up the WebView class here, not the chrome.webview API.
+  // The API will be automatically set up when first used.
+  if (extension) {
     if (extension->HasAPIPermission(APIPermission::kWebView)) {

[not at google - send to devlin, 2014/01/28 19:08:45]

If I'm reading this correctly... you can merge this condition with the outer
conditional?

if (extension && extension->HasAPIPermission(APIPermission::kWebView)) {
  module_system->Require("webView");
}

[Fady Samuel, 2014/01/31 22:31:45]

Done.

       module_system->Require("webView");
-      bool includeExperimental =
-          GetCurrentChannel() <= chrome::VersionInfo::CHANNEL_DEV;
-      if (!includeExperimental) {
+      if (GetCurrentChannel() <= chrome::VersionInfo::CHANNEL_DEV) {
+        module_system->Require("webViewExperimental");

[not at google - send to devlin, 2014/01/28 19:08:45]

(is there any way to (eventually, not now obviously) only set up the webview
bindings after a <webview> element is detected? all this setup is pretty
wasteful in the common case.)

[Fady Samuel, 2014/01/31 22:31:45]

I'm not aware of anyway to do that. <webview> is a custom element so I would
imagine it's fairly light-weight to inject (versus using MutationObservers which
is what we were doing previously).

+      } else {
         // TODO(asargent) We need a whitelist for webview experimental.
         // crbug.com/264852
         std::string id_hash = base::SHA1HashString(extension->id());
         std::string hexencoded_id_hash = base::HexEncode(id_hash.c_str(),
-                                                         id_hash.length());
+                                                          id_hash.length());

[not at google - send to devlin, 2014/01/28 19:08:45]

indentation

[Fady Samuel, 2014/01/31 22:31:45]

Done.

         if (hexencoded_id_hash == "8C3741E3AF0B93B6E8E0DDD499BB0B74839EA578" ||
             hexencoded_id_hash == "E703483CEF33DEC18B4B6DD84B5C776FB9182BDB" ||
             hexencoded_id_hash == "1A26E32DE447A17CBE5E9750CDBA78F58539B39C" ||
-            hexencoded_id_hash == "59048028102D7B4C681DBC7BC6CD980C3DC66DA3")
-          includeExperimental = true;
+            hexencoded_id_hash == "59048028102D7B4C681DBC7BC6CD980C3DC66DA3") {
+          module_system->Require("webViewExperimental");
+        } else {
+          module_system->Require("denyWebView");

[not at google - send to devlin, 2014/01/28 19:08:45]

err I don't understand. this code is executed even after requiring 'webView',
which is weird, why is webview *and* denyWebView being injected?

[Fady Samuel, 2014/01/31 22:31:45]

Wow, that's a silly refactor bug :P Thanks for catching that.

+        }
       }
-      if (includeExperimental)
-        module_system->Require("webViewExperimental");
-    } else {
-      module_system->Require("denyWebView");
     }
   }
 
   // Same comment as above for <adview> tag.
   if (context_type == Feature::BLESSED_EXTENSION_CONTEXT &&
       is_within_platform_app) {
     if (CommandLine::ForCurrentProcess()->HasSwitch(
             ::switches::kEnableAdview)) {
       if (extension->HasAPIPermission(APIPermission::kAdView)) {
         module_system->Require("adView");
@@ skipping 435 matching lines @@
     RenderView* background_view =
         ExtensionHelper::GetBackgroundPage(extension_id);
     if (background_view) {
       background_view->Send(new ExtensionHostMsg_EventAck(
           background_view->GetRoutingID()));
     }
   }
 }
 
 }  // namespace extensions