[Extensions] Limit Extension WebUI

chrome/browser/extensions/extension_web_ui.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/browser/extensions/extension_web_ui.h"
 
 #include <stddef.h>
 
 #include <set>
 #include <utility>
@@ skipping 314 matching lines @@
     }
     callback.Run(list);
   }
 }
 
 }  // namespace
 
 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::
   // UseWebUIBindingsForURL).
   int bindings = 0;
   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() {}
 
 extensions::BookmarkManagerPrivateDragEventRouter*
 ExtensionWebUI::bookmark_manager_private_drag_event_router() {
   return bookmark_manager_private_drag_event_router_.get();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ skipping 193 matching lines @@
         extensions::ImageLoader::ImageRepresentation::ALWAYS_RESIZE,
         gfx::Size(pixel_size, pixel_size),
         resource_scale_factor));
   }
 
   // LoadImagesAsync actually can run callback synchronously. We want to force
   // async.
   extensions::ImageLoader::Get(profile)->LoadImagesAsync(
       extension, info_list, base::Bind(&RunFaviconCallbackAsync, callback));
 }