Links in platform apps should open in the system default browser.

chrome/browser/ui/extensions/shell_window.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/ui/extensions/shell_window.h"
 
+#if defined(OS_WIN)
+#include <windows.h>
+#include <shellapi.h>
+#endif // OS_WIN
+
+#if defined(OS_POSIX)
+#include "chrome/browser/shell_integration_linux.h"
+#endif // OS_POSIX
+
+#if defined(OS_MACOSX)
+#include <CoreFoundation/CFBundle.h>
+#include <ApplicationServices/ApplicationServices.h>
+#endif // OS_MACOSX

[Marijn Kruisselbrink, 2012/09/04 15:40:53]

I think the chromium coding style says platform-specific #includes should come
after "regular" #includes
(http://www.chromium.org/developers/coding-style#TOC-Platform-defines)

+
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/extensions/extension_process_manager.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/extensions/shell_window_geometry_cache.h"
 #include "chrome/browser/extensions/shell_window_registry.h"
 #include "chrome/browser/file_select_helper.h"
 #include "chrome/browser/infobars/infobar_tab_helper.h"
 #include "chrome/browser/intents/web_intents_util.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/browser/profiles/profile.h"
@@ skipping 177 matching lines @@
       extension()->HasAPIPermission(APIPermission::kVideoCapture) &&
       !iter->second.empty()) {
     devices.push_back(iter->second[0]);
   }
 
   callback.Run(devices);
 }
 
 WebContents* ShellWindow::OpenURLFromTab(WebContents* source,
                                          const content::OpenURLParams& params) {
+#if defined(OS_WIN)
+  ShellExecuteA(NULL, "open", params.url.spec().c_str(),
+                NULL, NULL, SW_SHOWNORMAL);
+#elif defined(OS_POSIX)
+  ShellIntegrationLinux::LaunchDefaultBrowser(params.url);
+#elif defined(OS_MACOSX)
+  CFURLRef url = CFURLCreateWithBytes(
+      NULL,
+      (UInt8*)params.url.spec().c_str(),
+      params.url.spec().length(),
+      kCFStringEncodingASCII,
+      NULL
+  );
+  LSOpenCFURLRef(url,0);
+  CFRelease(url);

[jeremya, 2012/09/03 02:42:29]

It's kind of icky to have all these #ifdefs here. Perhaps move them into a
method on ShellIntegration? Something like
ShellIntegration::OpenURLInSystemBrowser()

+#else
   DCHECK(source == web_contents_);
 
   if (params.url.host() == extension_->id()) {
     AddMessageToDevToolsConsole(
         content::CONSOLE_MESSAGE_LEVEL_ERROR,
         base::StringPrintf(
             "Can't navigate to \"%s\"; apps do not support navigation.",
             params.url.spec().c_str()));
     return NULL;
   }
@@ skipping 21 matching lines @@
 
   // Force all links to open in a new tab, even if they were trying to open a
   // window.
   content::OpenURLParams new_tab_params = params;
   new_tab_params.disposition =
       disposition == NEW_BACKGROUND_TAB ? disposition : NEW_FOREGROUND_TAB;
   Browser* browser = browser::FindOrCreateTabbedBrowser(profile_);
   WebContents* new_tab = browser->OpenURL(new_tab_params);
   browser->window()->Show();
   return new_tab;
+#endif
+  source->SetDelegate(NULL);

[jeremya, 2012/09/03 02:42:29]

Rather than setting the delegate to the ShellWindow (which might get deleted
between AddNewContents and OpenURLFromTab!), perhaps it would be better to make
a new kind of WebContentsDelegate object that just listens for the
OpenURLFromTab message and then deletes itself (and the related WebContents).

It would be nice if we never had to create a new WebContents for this
navigation. From a little reading, I think that would require modifying the
dispatchDecidePolicyForNewWindowAction code in
third_party/WebKit/Source/WebKit/chromium/src/FrameLoaderClientImpl.cpp, so
perhaps that refactor could be done in a separate patch.

darin@ is probably a good person to comment on this, since he probably wrote
that code (though it's hard to tell through the mists of time).

+  return NULL;
 }
 
 void ShellWindow::AddNewContents(WebContents* source,
                                  WebContents* new_contents,
                                  WindowOpenDisposition disposition,
                                  const gfx::Rect& initial_pos,
                                  bool user_gesture) {
   DCHECK(source == web_contents_);
   DCHECK(Profile::FromBrowserContext(new_contents->GetBrowserContext()) ==
       profile_);
+
+#if defined(OS_WIN) || defined(OS_MACOSX) || defined(OS_POSIX)
+  new_contents->SetDelegate(this);
+#else
   Browser* browser = browser::FindOrCreateTabbedBrowser(profile_);
   // Force all links to open in a new tab, even if they were trying to open a
   // new window.
   disposition =
       disposition == NEW_BACKGROUND_TAB ? disposition : NEW_FOREGROUND_TAB;
   chrome::AddWebContents(browser, NULL, new_contents, disposition, initial_pos,
                          user_gesture);
+#endif
 }
 
 void ShellWindow::HandleKeyboardEvent(
     WebContents* source,
     const content::NativeWebKeyboardEvent& event) {
   DCHECK_EQ(source, web_contents_);
   native_window_->HandleKeyboardEvent(event);
 }
 
 void ShellWindow::OnNativeClose() {
@@ skipping 142 matching lines @@
     return;
 
   extensions::ShellWindowGeometryCache* cache =
       extensions::ExtensionSystem::Get(profile())->
           shell_window_geometry_cache();
 
   gfx::Rect bounds = native_window_->GetBounds();
   cache->SaveGeometry(extension()->id(), window_key_, bounds);
 }