[Extensions] Limit Extension WebUI

chrome/browser/extensions/extension_web_ui.cc

Index: chrome/browser/extensions/extension_web_ui.cc
diff --git a/chrome/browser/extensions/extension_web_ui.cc b/chrome/browser/extensions/extension_web_ui.cc
index 024282ec85ba4d3bfad8881a86543d9e9cd08b31..4973b8f7156ecaad1e07b16ae1cee7af198c28cb 100644
--- a/chrome/browser/extensions/extension_web_ui.cc
+++ b/chrome/browser/extensions/extension_web_ui.cc
@@ -332,13 +332,28 @@ void ForEachOverrideList(
 const char ExtensionWebUI::kExtensionURLOverrides[] =
     "extensions.chrome_url_overrides";
 
+// static
+bool ExtensionWebUI::NeedsExtensionWebUI(
+    content::BrowserContext* browser_context,
+    const GURL& url) {
+  if (!browser_context)

[Dan Beam, 2016/10/26 21:44:19]

can we really not have a browser_context validly?

[Devlin, 2016/10/26 21:54:50]

I copied this from the old code, but I'd kind of hope not (given this is being
called for webui, which needs to have web contents, render frames, etc.). 
Switched to a CHECK.

[Devlin, 2016/10/29 17:42:24]

So, not validly - but apparently yes in a unittest. :)  Fixed the test to pass
in a valid context.

+    return false;
+
+  const Extension* extension =
+      extensions::ExtensionRegistry::Get(browser_context)->enabled_extensions().

[Dan Beam, 2016/10/26 21:44:19]

does this exist in all cases (i.e. guest mode)?  i can never remember whether
this or extension service (?) don't exist in that (or other) cases

[Devlin, 2016/10/26 21:54:50]

ExtensionRegistry should always, always, always exist... as long as extensions
are enabled (this isn't even compiled if they're not).

+          GetExtensionOrAppByURL(url);
+  // Only use bindings for the Bookmark Manager extension, which needs it as a
+  // hack (see ExtensionWebUI's constructor below).
+  return extension && extension->id() == extension_misc::kBookmarkManagerId;
+}
+
 ExtensionWebUI::ExtensionWebUI(content::WebUI* web_ui, const GURL& url)
-    : WebUIController(web_ui),
-      url_(url) {
+    : WebUIController(web_ui) {
   Profile* profile = Profile::FromWebUI(web_ui);
   const Extension* extension = extensions::ExtensionRegistry::Get(
       profile)->enabled_extensions().GetExtensionOrAppByURL(url);
   DCHECK(extension);
+  DCHECK_EQ(extension_misc::kBookmarkManagerId, extension->id());

[Dan Beam, 2016/10/26 21:44:19]

should we rename this class then?  to like BookmarkManagerExtensionWebUI?

[Devlin, 2016/10/26 21:54:50]

Yes.  But it's got too much unrelated stuff in it right now (because having
settings url overrides in a class called "BookmarkManagerExtensionWebUI" makes
even less sense).  I've added that as an explicit part of the TODO in the header
file for when we split this from the url overrides functionality.

 
   // The base class defaults to enabling WebUI bindings, but we don't need
   // those (this is also reflected in ChromeWebUIControllerFactory::
@@ -347,13 +362,11 @@ ExtensionWebUI::ExtensionWebUI(content::WebUI* web_ui, const GURL& url)
   web_ui->SetBindings(bindings);
 
   // Hack: A few things we specialize just for the bookmark manager.
-  if (extension->id() == extension_misc::kBookmarkManagerId) {
-    bookmark_manager_private_drag_event_router_.reset(
-        new extensions::BookmarkManagerPrivateDragEventRouter(
-            profile, web_ui->GetWebContents()));
+  bookmark_manager_private_drag_event_router_.reset(
+      new extensions::BookmarkManagerPrivateDragEventRouter(
+          profile, web_ui->GetWebContents()));
 
-    web_ui->SetLinkTransitionType(ui::PAGE_TRANSITION_AUTO_BOOKMARK);
-  }
+  web_ui->SetLinkTransitionType(ui::PAGE_TRANSITION_AUTO_BOOKMARK);
 }
 
 ExtensionWebUI::~ExtensionWebUI() {}