Allow links inside <extensionoptions> to be opened in new tabs

chrome/browser/guest_view/extension_options/extension_options_guest.cc

Index: chrome/browser/guest_view/extension_options/extension_options_guest.cc
diff --git a/chrome/browser/guest_view/extension_options/extension_options_guest.cc b/chrome/browser/guest_view/extension_options/extension_options_guest.cc
index f7ca2908f72de01663b09ff9edfdecbc4ea2d6c1..5574f634ae17dd9540262cad5ae25a0a7c772b16 100644
--- a/chrome/browser/guest_view/extension_options/extension_options_guest.cc
+++ b/chrome/browser/guest_view/extension_options/extension_options_guest.cc
@@ -6,8 +6,13 @@
 
 #include "base/values.h"
 #include "chrome/browser/extensions/chrome_extension_web_contents_observer.h"
+#include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/guest_view/extension_options/extension_options_constants.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/extensions/api/extension_options_internal.h"
 #include "chrome/common/extensions/manifest_url_handler.h"
 #include "content/public/browser/render_process_host.h"
@@ -15,6 +20,7 @@
 #include "content/public/browser/web_contents.h"
 #include "extensions/browser/extension_function_dispatcher.h"
 #include "extensions/browser/extension_registry.h"
+#include "extensions/browser/guest_view/guest_view_manager.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_messages.h"
 #include "extensions/common/feature_switch.h"
@@ -177,3 +183,23 @@ void ExtensionOptionsGuest::SetUpAutoSize() {
   SetAutoSize(
       true, gfx::Size(min_width, min_height), gfx::Size(max_width, max_height));
 }
+
+bool ExtensionOptionsGuest::ShouldCreateWebContents(
+      content::WebContents* web_contents,
+      int route_id,
+      WindowContainerType window_container_type,
+      const base::string16& frame_name,
+      const GURL& target_url,
+      const std::string& partition_id,
+      content::SessionStorageNamespace* session_storage_namespace) {
+  Browser* browser =

[Fady Samuel, 2014/08/15 20:22:06]

A comment on what this does would be very helpful for future readers. :-)

Also comment on why we're returning false: Since we are loading the link in a
browser window, we do not want to create a new WebContents here.

[ericzeng, 2014/08/20 20:00:31]

Done.

+      chrome::FindBrowserWithWebContents(embedder_web_contents());
+  content::OpenURLParams params(target_url,
+                                content::Referrer(),
+                                NEW_FOREGROUND_TAB,
+                                content::PAGE_TRANSITION_LINK,
+                                false);
+  browser->OpenURL(params);
+  browser->window()->Show();

[not at google - send to devlin, 2014/08/15 20:31:38]

You should also add a TODO to respect the user's choice about how they want the
link opened. *Hopefully* there is code somewhere which implements that for you
without you needing to duplicate it. But, for example, you wouldn't go to the
new tab if it was a ctrl-click.

Btw - why the need for Show() at all? If the browser is in the foreground
shouldn't the URL it opens be automatically in the foreground?

[Fady Samuel, 2014/08/15 20:35:51]

Hmm, this can be determined by the disposition of the new window. Unfortunately,
it doesn't seem like the window disposition is passed in on
ShouldCreateWebContents. We might need to add that to the content API.

+  return false;
+}