[Extensions] Expand bindings access checks

chrome/renderer/extensions/chrome_extensions_dispatcher_delegate.cc

Index: chrome/renderer/extensions/chrome_extensions_dispatcher_delegate.cc
diff --git a/chrome/renderer/extensions/chrome_extensions_dispatcher_delegate.cc b/chrome/renderer/extensions/chrome_extensions_dispatcher_delegate.cc
index 937c4d332b389bfe5069112adfda22d466c4a68a..4301ec88da032a7cc6253f720ddd9dd06329c925 100644
--- a/chrome/renderer/extensions/chrome_extensions_dispatcher_delegate.cc
+++ b/chrome/renderer/extensions/chrome_extensions_dispatcher_delegate.cc
@@ -37,6 +37,8 @@
 #include "extensions/common/permissions/permissions_data.h"
 #include "extensions/common/switches.h"
 #include "extensions/renderer/dispatcher.h"
+#include "extensions/renderer/i18n_custom_bindings.h"
+#include "extensions/renderer/lazy_background_page_native_handler.h"
 #include "extensions/renderer/native_handler.h"
 #include "extensions/renderer/resource_bundle_source_map.h"
 #include "extensions/renderer/script_context.h"
@@ -123,6 +125,19 @@ void ChromeExtensionsDispatcherDelegate::RegisterNativeHandlers(
       "automationInternal",
       scoped_ptr<NativeHandler>(
           new extensions::AutomationInternalCustomBindings(context)));
+
+  // The following are native handlers that are defined in //extensions, but
+  // are only used for APIs defined in Chrome.
+  // TODO(devlin): We should clean this up. If an API is defined in Chrome,
+  // there's no reason to have its native handlers residing and being compiled
+  // in //extensions.

[asargent_no_longer_on_chrome, 2016/04/11 17:30:14]

Can you explain why you moved these here from //extensions ? It's not
immediately obvious to me, since the code looks the same. Are there different
security mechanisms in place for the native handlers registered here vs the ones
in dispatcher.cc's Dispatcher::RegisterNativeHandlers ?

[Devlin, 2016/04/13 19:51:26]

These are for features that are only available in chrome, so we hit a DCHECK
when we try to restrict them to that feature (i18n, lazy_background_page) in
//extensions.

More fundamentally, we really shouldn't be including them in //extensions at
all, because they're unused and increase the size of the extensions target
(though that part isn't addressed here - that's the TODO).

[asargent_no_longer_on_chrome, 2016/04/13 23:02:40]

Ok, the part about i18n makes sense since its implementation is currently in
chrome/browser/extensions/api/i18n/ and the API feature is declared in
chrome/common/extensions/api/_api_features.json. 


Out of curiousity, what is it about LazyBackgroundPageNativeHandler that is
chrome specific? It looks like the methods there just send IPCs that get handled
in extensions/browser/process_manager.cc.


BTW, I imagine that both the i18n API and event pages are things that non-chrome
consumers of //extensions probably would want, so I guess we eventually will end
up needing to move these back. 
 

[Devlin, 2016/04/15 21:39:24]

Right now, it's only used by tts, and tts is a feature declared in chrome.
I imagine that's probably true, but we should move them over in bulk - if we can
help it, we shouldn't have features half-implemented in //chrome and
//extensions (though I admit this is somewhat a losing battle).

+  module_system->RegisterNativeHandler(
+      "i18n",
+      scoped_ptr<NativeHandler>(new extensions::I18NCustomBindings(context)));
+  module_system->RegisterNativeHandler(
+      "lazy_background_page",
+      scoped_ptr<NativeHandler>(
+          new extensions::LazyBackgroundPageNativeHandler(context)));
 }
 
 void ChromeExtensionsDispatcherDelegate::PopulateSourceMap(