Stop using GetDefaultProfile() in Chrome OS implementation of platform_util::OpenExternal()

chrome/browser/platform_util_chromeos.cc

Index: chrome/browser/platform_util_chromeos.cc
diff --git a/chrome/browser/platform_util_chromeos.cc b/chrome/browser/platform_util_chromeos.cc
index 7ccdd5851ba477cabb03abf334f5e1ffefac105b..3143d849179cf4102bd03039c29296c45b8625b2 100644
--- a/chrome/browser/platform_util_chromeos.cc
+++ b/chrome/browser/platform_util_chromeos.cc
@@ -4,12 +4,8 @@
 
 #include "chrome/browser/platform_util.h"
 
-#include "base/bind.h"
 #include "chrome/browser/chromeos/file_manager/open_util.h"
-#include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/browser_navigator.h"
-#include "chrome/browser/ui/host_desktop.h"
-#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "content/public/browser/browser_thread.h"
 #include "url/gurl.h"
 
@@ -20,17 +16,6 @@ namespace {
 const char kGmailComposeUrl[] =
     "https://mail.google.com/mail/?extsrc=mailto&url=";
 
-void OpenURL(const std::string& url) {
-  // TODO(beng): improve this to locate context from call stack.
-  chrome::NavigateParams params(
-      ProfileManager::GetDefaultProfileOrOffTheRecord(),
-      GURL(url),
-      content::PAGE_TRANSITION_LINK);
-  params.disposition = NEW_FOREGROUND_TAB;
-  params.host_desktop_type = chrome::HOST_DESKTOP_TYPE_ASH;
-  chrome::Navigate(&params);
-}
-
 }  // namespace
 
 namespace platform_util {
@@ -45,7 +30,9 @@ void OpenItem(const base::FilePath& full_path) {
   file_manager::util::OpenItem(full_path);
 }
 
-void OpenExternal(const GURL& url) {
+void OpenExternal(Profile* profile, const GURL& url) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
   // This code should be obsolete since we have default handlers in ChromeOS
   // which should handle this. However - there are two things which make it
   // necessary to keep it in:
@@ -55,15 +42,18 @@ void OpenExternal(const GURL& url) {
   //     this function directly and which would therefore break (e.g.
   //     "Browser::EmailPageLocation" (to name only one).
   // As such we should keep this code here.
+  chrome::NavigateParams params(profile, url, content::PAGE_TRANSITION_LINK);
+  params.disposition = NEW_FOREGROUND_TAB;
+  params.host_desktop_type = chrome::HOST_DESKTOP_TYPE_ASH;
+
   if (url.SchemeIs("mailto")) {
     std::string string_url = kGmailComposeUrl;
     string_url.append(url.spec());
-    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-                            base::Bind(OpenURL, string_url));
-  } else if (url.is_valid()) {
-    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-                            base::Bind(OpenURL, url.spec()));
+    params.url = GURL(url);
   }
+
+  if (url.is_valid())

[sky, 2013/12/11 15:19:25]

Under what conditions can we get here with an invalid url? Shouldn't this check
be in the call site?

[hashimoto, 2013/12/12 05:55:58]

Among all 4 users of this function:
1. first_run_dialog.cc: Passing a constant URL.
2. browser_commands.cc: Passing a mailto: URL made by the caller. 
3. chrome_shell_window_delegate.cc: Cannot be sure. Too many call sites.
4. external_protocol_handler.cc: Invalid URL should not be passed because
job_factory->IsHandledURL(url) returns true for invalid URLs in
ResourceDispatcherHostImpl::HandleExternalProtocol()

So chrome_shell_window_delegate.cc is the only user who may pass invalid URLs
for this function.
But from what I see from the CL which introduced this check
(http://crrev.com/209751), it seems safe to remove this check.
It should not be dangerous to pass invalid URLs to chrome::Navigate anyways.

+    chrome::Navigate(&params);
 }
 
 }  // namespace platform_util