Don't update icon of chrome app shortcuts from the favicon of the hosted URL.

Panel apps should always launch in a panel.

Get rid of the non-extension OpenApplicationWindow().  Add TODO to continue untangling.

BUG=65632
TEST=Create a desktop shortcut for a hosted app.  Quit chrome, open the shortcut.  Icon should not change.

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

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

4 files changed, 60 insertions, 24 deletions

diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc
index 14d4017..112a2df 100644
--- a/chrome/browser/extensions/extension_prefs.cc
+++ b/chrome/browser/extensions/extension_prefs.cc
@@ -608,6 +608,11 @@ extension_misc::LaunchContainer ExtensionPrefs::GetLaunchContainer(
   extension_misc::LaunchContainer launch_container =
       extension->launch_container();
 
+  // Apps with app.launch.container = 'panel' should always
+  // open in a panel.
+  if (launch_container == extension_misc::LAUNCH_PANEL)
+    return extension_misc::LAUNCH_PANEL;
+
   ExtensionPrefs::LaunchType prefs_launch_type =
       GetLaunchType(extension->id(), default_pref_value);
 
diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
index 6f1e1e9..a42483c 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -525,12 +525,18 @@ TabContents* Browser::OpenApplication(
 
   switch (container) {
     case extension_misc::LAUNCH_WINDOW:
-      tab = Browser::OpenApplicationWindow(profile,
-                                           extension->GetFullLaunchURL());
+      // TODO(skerner): Setting |extension| to NULL is odd.
+      // Not doing so triggers some vestigial extensions app window
+      // behavior that leads to crashes.  This sort of window is no
+      // longer supported, and its remains need to be cleaned up.
+      // crbug/65630 tracks this cleanup.
+      tab = Browser::OpenApplicationWindow(profile, NULL, container,
+                                           extension->GetFullLaunchURL(),
+                                           NULL);
       break;
     case extension_misc::LAUNCH_PANEL:
       tab = Browser::OpenApplicationWindow(profile, extension, container,
-                                           GURL());
+                                           GURL(), NULL);
       break;
     case extension_misc::LAUNCH_TAB: {
       tab = Browser::OpenApplicationTab(profile, extension, existing_tab);
@@ -548,7 +554,8 @@ TabContents* Browser::OpenApplicationWindow(
     Profile* profile,
     const Extension* extension,
     extension_misc::LaunchContainer container,
-    const GURL& url_input) {
+    const GURL& url_input,
+    Browser** app_browser) {
   GURL url;
   if (!url_input.is_empty()) {
     if (extension)
@@ -566,6 +573,9 @@ TabContents* Browser::OpenApplicationWindow(
   bool as_panel = extension && (container == extension_misc::LAUNCH_PANEL);
   Browser* browser = Browser::CreateForApp(app_name, extension, profile,
                                            as_panel);
+  if (app_browser)
+    *app_browser = browser;
+
   TabContentsWrapper* wrapper =
       browser->AddSelectedTabWithURL(url, PageTransition::START_PAGE);
   TabContents* contents = wrapper->tab_contents();
@@ -576,24 +586,33 @@ TabContents* Browser::OpenApplicationWindow(
   // TODO(jcampan): http://crbug.com/8123 we should not need to set the initial
   //                focus explicitly.
   contents->view()->SetInitialFocus();
+  return contents;
+}
+
+TabContents* Browser::OpenAppShortcutWindow(Profile* profile,
+                                            const GURL& url,
+                                            bool update_shortcut) {
+  Browser* app_browser;
+  TabContents* tab = OpenApplicationWindow(
+      profile,
+      NULL,  // this is a URL app.  No extension.
+      extension_misc::LAUNCH_WINDOW,
+      url,
+      &app_browser);
+
+  if (!tab)
+    return NULL;
 
-  if (!as_panel) {
+  if (update_shortcut) {
     // Set UPDATE_SHORTCUT as the pending web app action. This action is picked
     // up in LoadingStateChanged to schedule a GetApplicationInfo. And when
     // the web app info is available, TabContents notifies Browser via
     // OnDidGetApplicationInfo, which calls
     // web_app::UpdateShortcutForTabContents when it sees UPDATE_SHORTCUT as
     // pending web app action.
-    browser->pending_web_app_action_ = UPDATE_SHORTCUT;
+    app_browser->pending_web_app_action_ = UPDATE_SHORTCUT;
   }
-
-  return contents;
-}
-
-// static
-TabContents* Browser::OpenApplicationWindow(Profile* profile, const GURL& url) {
-  return OpenApplicationWindow(profile, NULL, extension_misc::LAUNCH_WINDOW,
-                               url);
+  return tab;
 }
 
 // static
diff --git a/chrome/browser/ui/browser.h b/chrome/browser/ui/browser.h
index 4331770..e7d4641 100644
--- a/chrome/browser/ui/browser.h
+++ b/chrome/browser/ui/browser.h
@@ -245,15 +245,24 @@ class Browser : public TabHandlerDelegate,
   // app panel window, otherwise it will be opened as as either
   // Browser::Type::APP a.k.a. "thin frame" (if |extension| is NULL) or
   // Browser::Type::EXTENSION_APP (if |extension| is non-NULL).
+  // If |app_browser| is not NULL, it is set to the browser that hosts the
+  // returned tab.
   static TabContents* OpenApplicationWindow(
       Profile* profile,
       const Extension* extension,
       extension_misc::LaunchContainer container,
-      const GURL& url);
-
-  // Open an application in a new application window.  Used to implement
-  // app shortcuts.
-  static TabContents* OpenApplicationWindow(Profile* profile, const GURL& url);
+      const GURL& url,
+      Browser** app_browser);
+
+  // Open |url| in an app shortcut window.  If |update_shortcut| is true,
+  // update the name, description, and favicon of the shortcut.
+  // There are two kinds of app shortcuts: Shortcuts to a URL,
+  // and shortcuts that open an installed application.  This function
+  // is used to open the former.  To open the latter, use
+  // Browser::OpenApplicationWindow().
+  static TabContents* OpenAppShortcutWindow(Profile* profile,
+                                            const GURL& url,
+                                            bool update_shortcut);
 
   // 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 cb1d465..60fd1b3 100644
--- a/chrome/browser/ui/browser_init.cc
+++ b/chrome/browser/ui/browser_init.cc
@@ -628,12 +628,12 @@ bool BrowserInit::LaunchWithProfile::OpenApplicationWindow(Profile* profile) {
     const Extension* extension =
         extensions_service->GetExtensionById(app_id, false);
 
-    // The extension with |app_id| may have been uninstalled.
+    // The extension with id |app_id| may have been uninstalled.
     if (!extension)
       return false;
 
-    // Look at prefs to find the container in which an app should launch.
-    // If no preference is set, launch in a window.
+    // Look at preferences to find the right launch container.  If no
+    // preference is set, launch as a window.
     extension_misc::LaunchContainer launch_container =
         extensions_service->extension_prefs()->GetLaunchContainer(
             extension, ExtensionPrefs::LAUNCH_WINDOW);
@@ -657,8 +657,11 @@ bool BrowserInit::LaunchWithProfile::OpenApplicationWindow(Profile* profile) {
         ChildProcessSecurityPolicy::GetInstance();
     if (policy->IsWebSafeScheme(url.scheme()) ||
         url.SchemeIs(chrome::kFileScheme)) {
-      Browser::OpenApplicationWindow(profile, url);
-      return true;
+      TabContents* app_tab = Browser::OpenAppShortcutWindow(
+          profile,
+          url,
+          true);  // Update app info.
+      return (app_tab != NULL);
     }
   }
   return false;