Add "open as window" menu item to NTP app menu.

BUG=59697
TEST=BrowserTest.OpenAppWindowLikeNtp,SessionRestoreUITest.RestoreAfterClosing*

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=66646

Review URL: http://codereview.chromium.org/5019005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@66954 0039d316-1c4b-4281-b951-d872f2087c98

19 files changed, 122 insertions, 98 deletions

diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index 7dcd59a..81b8b87 100644
--- a/chrome/app/generated_resources.grd
+++ b/chrome/app/generated_resources.grd
@@ -7432,6 +7432,10 @@ Keep your key file in a safe place. You will need it to create new versions of y
                desc="Text for the button that opens the app in a regular tab.">
         Open as regular tab
       </message>
+      <message name="IDS_APP_CONTEXT_MENU_OPEN_WINDOW"
+               desc="Text for the button that opens the app in an app window.">
+        Open as window
+      </message>
       <message name="IDS_APP_CONTEXT_MENU_OPEN_FULLSCREEN"
                desc="Text for the button that opens the app full screen.">
         Open full screen
diff --git a/chrome/browser/background_application_list_model.cc b/chrome/browser/background_application_list_model.cc
index 63f2b94..64ccf2e 100644
--- a/chrome/browser/background_application_list_model.cc
+++ b/chrome/browser/background_application_list_model.cc
@@ -242,6 +242,7 @@ void BackgroundApplicationListModel::Observe(
   ExtensionsService* service = profile_->GetExtensionsService();
   if (!service || !service->is_ready())
     return;
+
   switch (type.value) {
     case NotificationType::EXTENSION_LOADED:
       OnExtensionLoaded(Details<Extension>(details).ptr());
@@ -286,7 +287,6 @@ void BackgroundApplicationListModel::RemoveObserver(Observer* observer) {
 // each observer.
 void BackgroundApplicationListModel::Update() {
   ExtensionsService* service = profile_->GetExtensionsService();
-  DCHECK(service->is_ready());
 
   // Discover current background applications, compare with previous list, which
   // is consistently sorted, and notify observers if they differ.
diff --git a/chrome/browser/browser_browsertest.cc b/chrome/browser/browser_browsertest.cc
index 2cfc68e..b306ec1 100644
--- a/chrome/browser/browser_browsertest.cc
+++ b/chrome/browser/browser_browsertest.cc
@@ -573,6 +573,42 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, CloseWithAppMenuOpen) {
                                    new RunCloseWithAppMenuTask(browser()));
 }
 
+#if !defined(OS_MACOSX)
+IN_PROC_BROWSER_TEST_F(BrowserTest, OpenAppWindowLikeNtp) {
+  ASSERT_TRUE(test_server()->Start());
+
+  // Load an app
+  host_resolver()->AddRule("www.example.com", "127.0.0.1");
+  ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII("app/")));
+  const Extension* extension_app = GetExtension();
+
+  // Launch it in a window, as AppLauncherHandler::HandleLaunchApp() would.
+  TabContents* app_window = Browser::OpenApplication(
+      browser()->profile(), extension_app, extension_misc::LAUNCH_WINDOW, NULL);
+  ASSERT_TRUE(app_window);
+
+  // Apps launched in a window from the NTP do not have extension_app set in
+  // tab contents.
+  EXPECT_FALSE(app_window->extension_app());
+  EXPECT_EQ(extension_app->GetFullLaunchURL(), app_window->GetURL());
+
+  // The launch should have created a new browser.
+  ASSERT_EQ(2u, BrowserList::GetBrowserCount(browser()->profile()));
+
+  // Find the new browser.
+  Browser* new_browser = NULL;
+  for (BrowserList::const_iterator i = BrowserList::begin();
+       i != BrowserList::end() && !new_browser; ++i) {
+    if (*i != browser())
+      new_browser = *i;
+  }
+  ASSERT_TRUE(new_browser);
+  ASSERT_TRUE(new_browser != browser());
+
+  EXPECT_EQ(Browser::TYPE_APP, new_browser->type());
+}
+#endif  // !defined(OS_MACOSX)
+
 // TODO(ben): this test was never enabled. It has bit-rotted since being added.
 // It originally lived in browser_unittest.cc, but has been moved here to make
 // room for real browser unit tests.
diff --git a/chrome/browser/dom_ui/app_launcher_handler.cc b/chrome/browser/dom_ui/app_launcher_handler.cc
index 77e3ee0..e57dfdc 100644
--- a/chrome/browser/dom_ui/app_launcher_handler.cc
+++ b/chrome/browser/dom_ui/app_launcher_handler.cc
@@ -190,6 +190,14 @@ void AppLauncherHandler::FillAppDictionary(DictionaryValue* dictionary) {
   bool showLauncher =
       CommandLine::ForCurrentProcess()->HasSwitch(switches::kEnableAppLauncher);
   dictionary->SetBoolean("showLauncher", showLauncher);
+
+#if defined(OS_MACOSX)
+  // App windows are not yet implemented on mac.
+  bool disable_app_window_launch = true;
+#else
+  bool disable_app_window_launch = false;
+#endif
+  dictionary->SetBoolean("disableAppWindowLaunch", disable_app_window_launch);
 }
 
 void AppLauncherHandler::HandleGetApps(const ListValue* args) {
@@ -248,8 +256,18 @@ void AppLauncherHandler::HandleLaunchApp(const ListValue* args) {
     old_contents = browser->GetSelectedTabContents();
 
   AnimateAppIcon(extension, rect);
+
+  extension_misc::LaunchContainer launch_container =
+      extension->launch_container();
+  ExtensionPrefs::LaunchType prefs_launch_type =
+      extensions_service_->extension_prefs()->GetLaunchType(extension_id);
+
+  // If the user chose to open in a window, change the container type.
+  if (prefs_launch_type == ExtensionPrefs::LAUNCH_WINDOW)
+    launch_container = extension_misc::LAUNCH_WINDOW;
+
   TabContents* new_contents = Browser::OpenApplication(
-      profile, extension, extension->launch_container(), old_contents);
+      profile, extension, launch_container, old_contents);
 
   if (new_contents != old_contents && browser->tab_count() > 1)
     browser->CloseTabContents(old_contents);
@@ -310,7 +328,7 @@ void AppLauncherHandler::HandleHideAppsPromo(const ListValue* args) {
   extensions_service_->default_apps()->SetPromoHidden();
 }
 
-//static
+// static
 void AppLauncherHandler::RecordWebStoreLaunch(bool promo_active) {
   if (!promo_active) return;
 
@@ -319,7 +337,7 @@ void AppLauncherHandler::RecordWebStoreLaunch(bool promo_active) {
                             extension_misc::PROMO_BUCKET_BOUNDARY);
 }
 
-//static
+// static
 void AppLauncherHandler::RecordAppLaunch(bool promo_active) {
   // TODO(jstritar): record app launches that occur when the promo is not
   // active using a different histogram.
diff --git a/chrome/browser/dom_ui/ntp_resource_cache.cc b/chrome/browser/dom_ui/ntp_resource_cache.cc
index 7d0a5d4..859c666 100644
--- a/chrome/browser/dom_ui/ntp_resource_cache.cc
+++ b/chrome/browser/dom_ui/ntp_resource_cache.cc
@@ -310,6 +310,8 @@ void NTPResourceCache::CreateNewTabHTML() {
       l10n_util::GetStringUTF16(IDS_APP_CONTEXT_MENU_OPEN_PINNED));
   localized_strings.SetString("applaunchtyperegular",
       l10n_util::GetStringUTF16(IDS_APP_CONTEXT_MENU_OPEN_REGULAR));
+  localized_strings.SetString("applaunchtypewindow",
+      l10n_util::GetStringUTF16(IDS_APP_CONTEXT_MENU_OPEN_WINDOW));
   localized_strings.SetString("applaunchtypefullscreen",
       l10n_util::GetStringUTF16(IDS_APP_CONTEXT_MENU_OPEN_FULLSCREEN));
   localized_strings.SetString("web_store_title",
diff --git a/chrome/browser/extensions/extension_dom_ui.cc b/chrome/browser/extensions/extension_dom_ui.cc
index c0522f0..faaa74b 100644
--- a/chrome/browser/extensions/extension_dom_ui.cc
+++ b/chrome/browser/extensions/extension_dom_ui.cc
@@ -243,13 +243,6 @@ bool ExtensionDOMUI::HandleChromeURLOverride(GURL* url, Profile* profile) {
     return false;
 
   ExtensionsService* service = profile->GetExtensionsService();
-  if (!service->is_ready()) {
-    // TODO(erikkay) So far, it looks like extensions load before the new tab
-    // page.  I don't know if we have anything that enforces this, so add this
-    // check for safety.
-    NOTREACHED() << "Chrome URL override requested before extensions loaded";
-    return false;
-  }
 
   size_t i = 0;
   while (i < url_list->GetSize()) {
diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc
index 57c5c9b..09b7dd6 100644
--- a/chrome/browser/extensions/extension_prefs.cc
+++ b/chrome/browser/extensions/extension_prefs.cc
@@ -456,12 +456,22 @@ void ExtensionPrefs::SetAllowFileAccess(const std::string& extension_id,
 ExtensionPrefs::LaunchType ExtensionPrefs::GetLaunchType(
     const std::string& extension_id) {
   int value;
-  if (ReadExtensionPrefInteger(extension_id, kPrefLaunchType, &value) && (
-      value == LAUNCH_PINNED ||
+  if (ReadExtensionPrefInteger(extension_id, kPrefLaunchType, &value) &&
+     (value == LAUNCH_PINNED ||
       value == LAUNCH_REGULAR ||
-      value == LAUNCH_FULLSCREEN)) {
+      value == LAUNCH_FULLSCREEN ||
+      value == LAUNCH_WINDOW)) {
+
+#if defined(OS_MACOSX)
+    // App windows are not yet supported on mac.  Pref sync could make
+    // the launch type LAUNCH_WINDOW, even if there is no UI to set it
+    // on mac.
+    if (value == LAUNCH_WINDOW)
+      return LAUNCH_REGULAR;
+#endif
     return static_cast<LaunchType>(value);
   }
+
   return LAUNCH_REGULAR;
 }
 
diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h
index 11c3843..0bcd9f9 100644
--- a/chrome/browser/extensions/extension_prefs.h
+++ b/chrome/browser/extensions/extension_prefs.h
@@ -35,7 +35,8 @@ class ExtensionPrefs {
   enum LaunchType {
     LAUNCH_PINNED,
     LAUNCH_REGULAR,
-    LAUNCH_FULLSCREEN
+    LAUNCH_FULLSCREEN,
+    LAUNCH_WINDOW
   };
 
   explicit ExtensionPrefs(PrefService* prefs, const FilePath& root_dir_);
diff --git a/chrome/browser/extensions/extension_process_manager.cc b/chrome/browser/extensions/extension_process_manager.cc
index 8fd5538..c273033 100644
--- a/chrome/browser/extensions/extension_process_manager.cc
+++ b/chrome/browser/extensions/extension_process_manager.cc
@@ -279,10 +279,11 @@ void ExtensionProcessManager::Observe(NotificationType type,
                                       const NotificationSource& source,
                                       const NotificationDetails& details) {
   switch (type.value) {
-    case NotificationType::EXTENSIONS_READY:
+    case NotificationType::EXTENSIONS_READY: {
       CreateBackgroundHosts(this,
           Source<Profile>(source).ptr()->GetExtensionsService()->extensions());
       break;
+    }
 
     case NotificationType::EXTENSION_LOADED: {
       ExtensionsService* service =
@@ -452,6 +453,9 @@ void IncognitoExtensionProcessManager::Observe(
       // it matches our profile.
       Browser* browser = Source<Browser>(source).ptr();
       if (browser->profile() == browsing_instance_->profile()) {
+        // On Chrome OS, a login screen is implemented as a browser.
+        // This browser has no extension service.  In this case,
+        // service will be NULL.
         ExtensionsService* service =
             browsing_instance_->profile()->GetExtensionsService();
         if (service && service->is_ready())
diff --git a/chrome/browser/extensions/extension_startup_browsertest.cc b/chrome/browser/extensions/extension_startup_browsertest.cc
index 1b15ce0..a5561ef 100644
--- a/chrome/browser/extensions/extension_startup_browsertest.cc
+++ b/chrome/browser/extensions/extension_startup_browsertest.cc
@@ -84,9 +84,6 @@ class ExtensionStartupTestBase : public InProcessBrowserTest {
   void WaitForServicesToStart(int num_expected_extensions,
                               bool expect_extensions_enabled) {
     ExtensionsService* service = browser()->profile()->GetExtensionsService();
-    if (!service->is_ready())
-      ui_test_utils::WaitForNotification(NotificationType::EXTENSIONS_READY);
-    ASSERT_TRUE(service->is_ready());
 
     // Count the number of non-component extensions.
     int found_extensions = 0;
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index 06fe068..02c4bcc 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -607,7 +607,7 @@ void ExtensionsService::InitEventRouters() {
 void ExtensionsService::Init() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  DCHECK(!ready_);
+  DCHECK(!ready_);  // Can't redo init.
   DCHECK_EQ(extensions_.size(), 0u);
 
   // Hack: we need to ensure the ResourceDispatcherHost is ready before we load
@@ -1483,10 +1483,11 @@ void ExtensionsService::GarbageCollectExtensions() {
 }
 
 void ExtensionsService::OnLoadedInstalledExtensions() {
-  ready_ = true;
   if (updater_.get()) {
     updater_->Start();
   }
+
+  ready_ = true;
   NotificationService::current()->Notify(
       NotificationType::EXTENSIONS_READY,
       Source<Profile>(profile_),
diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h
index f137a92..e37b452 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -386,6 +386,7 @@ class ExtensionsService
   ExtensionPrefs* extension_prefs() { return extension_prefs_.get(); }
 
   // Whether the extension service is ready.
+  // TODO(skerner): Get rid of this method.  crbug.com/63756
   bool is_ready() { return ready_; }
 
   // Note that this may return NULL if autoupdate is not turned on.
@@ -512,7 +513,8 @@ class ExtensionsService
   // Used by dispatchers to limit API quota for individual extensions.
   ExtensionsQuotaService quota_service_;
 
-  // Is the service ready to go?
+  // Record that Init() has been called, and NotificationType::EXTENSIONS_READY
+  // has fired.
   bool ready_;
 
   // Our extension updater, if updates are turned on.
diff --git a/chrome/browser/resources/new_new_tab.html b/chrome/browser/resources/new_new_tab.html
index bcf19b3..cd2613c 100644
--- a/chrome/browser/resources/new_new_tab.html
+++ b/chrome/browser/resources/new_new_tab.html
@@ -236,6 +236,8 @@ if ('mode' in hashParams) {
     launch-type="0">
 <command id="apps-launch-type-regular" i18n-values=".label:applaunchtyperegular"
     launch-type="1">
+<command id="apps-launch-type-window"
+    i18n-values=".label:applaunchtypewindow" launch-type="3">
 <command id="apps-launch-type-fullscreen"
     i18n-values=".label:applaunchtypefullscreen" launch-type="2">
 
@@ -249,6 +251,8 @@ if ('mode' in hashParams) {
   <hr>
   <button command="#apps-launch-type-regular" launch-type="1"></button>
   <button command="#apps-launch-type-pinned" launch-type="0"></button>
+  <button id="apps-launch-type-window-menu-item"
+      command="#apps-launch-type-window" launch-type="3"></button>
   <button command="#apps-launch-type-fullscreen" launch-type="2"></button>
   <hr>
   <button command="#apps-options-command"></button>
diff --git a/chrome/browser/resources/ntp/apps.js b/chrome/browser/resources/ntp/apps.js
index c8638c6..3cbfc9e 100644
--- a/chrome/browser/resources/ntp/apps.js
+++ b/chrome/browser/resources/ntp/apps.js
@@ -21,6 +21,10 @@ function getAppsCallback(data) {
   var appsPromoPing = PING_WEBSTORE_LAUNCH_PREFIX + '+' + data.showPromo;
   var webStoreEntry;
 
+  // Hide the app window menu option on platforms that do not support it.
+  $('apps-launch-type-window-menu-item').style.display =
+      (data.disableAppWindowLaunch ? 'none' : 'inline');
+
   appsMiniview.textContent = '';
   appsSectionContent.textContent = '';
 
@@ -145,10 +149,11 @@ var apps = (function() {
   var LaunchType = {
     LAUNCH_PINNED: 0,
     LAUNCH_REGULAR: 1,
-    LAUNCH_FULLSCREEN: 2
+    LAUNCH_FULLSCREEN: 2,
+    LAUNCH_WINDOW: 3
   };
 
-  // Keep in sync with LaunchContainer in extension.h
+  // Keep in sync with LaunchContainer in extension_constants.h
   var LaunchContainer = {
     LAUNCH_WINDOW: 0,
     LAUNCH_PANEL: 1,
@@ -184,7 +189,8 @@ var apps = (function() {
         // Update the commands related to the launch type.
         var launchTypeIds = ['apps-launch-type-pinned',
                              'apps-launch-type-regular',
-                             'apps-launch-type-fullscreen'];
+                             'apps-launch-type-fullscreen',
+                             'apps-launch-type-window'];
         launchTypeIds.forEach(function(id) {
           var command = $(id);
           command.disabled = isPanel;
@@ -215,6 +221,7 @@ var apps = (function() {
       case 'apps-launch-type-pinned':
       case 'apps-launch-type-regular':
       case 'apps-launch-type-fullscreen':
+      case 'apps-launch-type-window':
         chrome.send('setLaunchType',
             [currentApp['id'], e.command.getAttribute('launch-type')]);
         break;
diff --git a/chrome/browser/sessions/session_restore.cc b/chrome/browser/sessions/session_restore.cc
index 6f38e51..3417215 100644
--- a/chrome/browser/sessions/session_restore.cc
+++ b/chrome/browser/sessions/session_restore.cc
@@ -272,8 +272,7 @@ class SessionRestoreImpl : public NotificationObserver {
         synchronous_(synchronous),
         clobber_existing_window_(clobber_existing_window),
         always_create_tabbed_browser_(always_create_tabbed_browser),
-        urls_to_open_(urls_to_open),
-        waiting_for_extension_service_(false) {
+        urls_to_open_(urls_to_open) {
   }
 
   void Restore() {
@@ -339,19 +338,6 @@ class SessionRestoreImpl : public NotificationObserver {
         delete this;
         return;
 
-      case NotificationType::EXTENSIONS_READY: {
-        if (!waiting_for_extension_service_)
-          return;
-
-        waiting_for_extension_service_ = false;
-        if (synchronous_) {
-          MessageLoop::current()->Quit();
-          return;
-        }
-        ProcessSessionWindows(&windows_);
-        return;
-      }
-
       default:
         NOTREACHED();
         break;
@@ -396,18 +382,6 @@ class SessionRestoreImpl : public NotificationObserver {
 
   void OnGotSession(SessionService::Handle handle,
                     std::vector<SessionWindow*>* windows) {
-    if (HasExtensionApps(*windows) && profile_->GetExtensionsService() &&
-        !profile_->GetExtensionsService()->is_ready()) {
-      // At least one tab is an app tab and the extension service hasn't
-      // finished loading. Wait to continue processing until the extensions
-      // service finishes loading.
-      registrar_.Add(this, NotificationType::EXTENSIONS_READY,
-                     Source<Profile>(profile_));
-      windows_.swap(*windows);
-      waiting_for_extension_service_ = true;
-      return;
-    }
-
     if (synchronous_) {
       // See comment above windows_ as to why we don't process immediately.
       windows_.swap(*windows);
@@ -418,28 +392,6 @@ class SessionRestoreImpl : public NotificationObserver {
     ProcessSessionWindows(windows);
   }
 
-  // Returns true if any tab in |windows| has an application extension id.
-  bool HasExtensionApps(const std::vector<SessionWindow*>& windows) {
-    for (std::vector<SessionWindow*>::const_iterator i = windows.begin();
-         i != windows.end(); ++i) {
-      if (HasExtensionApps((*i)->tabs))
-        return true;
-    }
-
-    return false;
-  }
-
-  // Returns true if any tab in |tabs| has an application extension id.
-  bool HasExtensionApps(const std::vector<SessionTab*>& tabs) {
-    for (std::vector<SessionTab*>::const_iterator i = tabs.begin();
-         i != tabs.end(); ++i) {
-      if (!(*i)->extension_app_id.empty())
-        return true;
-    }
-
-    return false;
-  }
-
   void ProcessSessionWindows(std::vector<SessionWindow*>* windows) {
     if (windows->empty()) {
       // Restore was unsuccessful.
@@ -601,10 +553,6 @@ class SessionRestoreImpl : public NotificationObserver {
   // windows when the nested message loop exits.
   std::vector<SessionWindow*> windows_;
 
-  // If true, indicates at least one tab has an application extension id and
-  // we're waiting for the extension service to finish loading.
-  bool waiting_for_extension_service_;
-
   NotificationRegistrar registrar_;
 };
 
diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index 5f091ee..cfa1ac0 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -598,12 +598,13 @@ void TabContents::SetExtensionAppById(const std::string& extension_app_id) {
     return;
 
   ExtensionsService* extension_service = profile()->GetExtensionsService();
-  if (extension_service && extension_service->is_ready()) {
-    const Extension* extension =
-        extension_service->GetExtensionById(extension_app_id, false);
-    if (extension)
-      SetExtensionApp(extension);
-  }
+  if (!extension_service || !extension_service->is_ready())
+    return;
+
+  const Extension* extension =
+      extension_service->GetExtensionById(extension_app_id, false);
+  if (extension)
+    SetExtensionApp(extension);
 }
 
 SkBitmap* TabContents::GetExtensionAppIcon() {
diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
index cb6dd29..b566eb7 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -499,8 +499,6 @@ TabContents* Browser::OpenApplication(Profile* profile,
                                       const std::string& app_id,
                                       TabContents* existing_tab) {
   ExtensionsService* extensions_service = profile->GetExtensionsService();
-  if (!extensions_service->is_ready())
-    return NULL;
 
   // If the extension with |app_id| could't be found, most likely because it
   // was uninstalled.
@@ -523,9 +521,11 @@ TabContents* Browser::OpenApplication(
 
   UMA_HISTOGRAM_ENUMERATION("Extensions.AppLaunchContainer", container, 100);
 
-  // The app is not yet open.  Load it.
   switch (container) {
     case extension_misc::LAUNCH_WINDOW:
+      tab = Browser::OpenApplicationWindow(profile,
+                                           extension->GetFullLaunchURL());
+      break;
     case extension_misc::LAUNCH_PANEL:
       tab = Browser::OpenApplicationWindow(profile, extension, container,
                                            GURL());
@@ -589,7 +589,7 @@ TabContents* Browser::OpenApplicationWindow(
 }
 
 // static
-TabContents* Browser::OpenApplicationWindow(Profile* profile, GURL& url) {
+TabContents* Browser::OpenApplicationWindow(Profile* profile, const GURL& url) {
   return OpenApplicationWindow(profile, NULL, extension_misc::LAUNCH_WINDOW,
                                url);
 }
diff --git a/chrome/browser/ui/browser.h b/chrome/browser/ui/browser.h
index b70049b..901137f 100644
--- a/chrome/browser/ui/browser.h
+++ b/chrome/browser/ui/browser.h
@@ -250,8 +250,9 @@ class Browser : public TabHandlerDelegate,
       extension_misc::LaunchContainer container,
       const GURL& url);
 
-  // Open an application for |extension| in a new application window or panel.
-  static TabContents* OpenApplicationWindow(Profile* profile, GURL& url);
+  // Open an application in a new application window.  Used to implement
+  // app shortcuts.
+  static TabContents* OpenApplicationWindow(Profile* profile, const GURL& url);
 
   // Open an application for |extension| in a new application tab, or
   // |existing_tab| if not NULL.  Returns NULL if there are no appropriate
diff --git a/chrome/browser/ui/browser_init.cc b/chrome/browser/ui/browser_init.cc
index c231d46..2de3e11 100644
--- a/chrome/browser/ui/browser_init.cc
+++ b/chrome/browser/ui/browser_init.cc
@@ -605,14 +605,9 @@ bool BrowserInit::LaunchWithProfile::OpenApplicationWindow(Profile* profile) {
   if (!IsAppLaunch(&url_string, &app_id))
     return false;
 
-  // http://crbug.com/37548
-  // TODO(rafaelw): There are two legitimate cases where the extensions
-  // service could not be ready at this point which need to be handled:
-  // 1) The locale has changed and the manifests stored in the preferences
-  //    need to be relocalized.
-  // 2) An externally installed extension will be found and installed.
-  // Note that this can also fail if the app_id is simply invalid.
-  // TODO(rafaelw): Do something reasonable here. Pop up a warning panel?
+  // This can fail if the app_id is invalid.  It can also fail if the
+  // extension is external, and has not yet been installed.
+  // TODO(skerner): Do something reasonable here. Pop up a warning panel?
   // Open an URL to the gallery page of the extension id?
   if (!app_id.empty())
     return Browser::OpenApplication(profile, app_id, NULL) != NULL;