Use high resolution icons where possible for streamlined hosted app icons.

chrome/browser/extensions/tab_helper.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/extensions/tab_helper.h"
 
 #include "base/logging.h"
 #include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/activity_log/activity_log.h"
 #include "chrome/browser/extensions/api/declarative/rules_registry_service.h"
 #include "chrome/browser/extensions/api/declarative_content/content_rules_registry.h"
 #include "chrome/browser/extensions/crx_installer.h"
 #include "chrome/browser/extensions/error_console/error_console.h"
 #include "chrome/browser/extensions/extension_action.h"
 #include "chrome/browser/extensions/extension_action_manager.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
+#include "chrome/browser/extensions/favicon_downloader.h"
 #include "chrome/browser/extensions/image_loader.h"
 #include "chrome/browser/extensions/page_action_controller.h"
 #include "chrome/browser/extensions/script_badge_controller.h"
 #include "chrome/browser/extensions/script_bubble_controller.h"
 #include "chrome/browser/extensions/script_executor.h"
 #include "chrome/browser/extensions/webstore_inline_installer.h"
 #include "chrome/browser/extensions/webstore_inline_installer_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sessions/session_id.h"
 #include "chrome/browser/sessions/session_tab_helper.h"
@@ skipping 242 matching lines @@
                         OnContentScriptsExecuting)
     IPC_MESSAGE_HANDLER(ExtensionHostMsg_OnWatchedPageChange,
                         OnWatchedPageChange)
     IPC_MESSAGE_HANDLER(ChromeViewHostMsg_DetailedConsoleMessageAdded,
                         OnDetailedConsoleMessageAdded)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
 }
 
+void TabHelper::CreateHostedApp() {
+  // Add urls from the WebApplicationInfo.
+  std::vector<GURL> web_app_info_icon_urls;
+  for (std::vector<WebApplicationInfo::IconInfo>::const_iterator it =
+           web_app_info_.icons.begin();
+       it != web_app_info_.icons.end(); ++it) {
+    if (it->url.is_valid())
+      web_app_info_icon_urls.push_back(it->url);
+  }
+
+  favicon_downloader_.reset(
+      new FaviconDownloader(web_contents(),
+                            web_app_info_icon_urls,
+                            base::Bind(&TabHelper::FinishCreateHostedApp,
+                                       base::Unretained(this))));
+  favicon_downloader_->Start();
+}
+
+void TabHelper::FinishCreateHostedApp(
+    bool success,
+    const std::map<GURL, std::vector<SkBitmap> >& bitmaps) {
+  // The tab has navigated away during the icon download. Cancel the hosted app
+  // creation.
+  if (!success) {
+    favicon_downloader_.reset();
+    return;
+  }
+
+  WebApplicationInfo install_info(web_app_info_);
+  if (install_info.app_url.is_empty())
+    install_info.app_url = web_contents()->GetURL();
+
+  if (install_info.title.empty())
+    install_info.title = web_contents()->GetTitle();
+  if (install_info.title.empty())
+    install_info.title = UTF8ToUTF16(install_info.app_url.spec());
+
+  install_info.urls.push_back(install_info.app_url);
+  install_info.is_bookmark_app = true;
+
+  // Add the downloaded icons.
+  for (FaviconDownloader::FaviconMap::const_iterator map_it = bitmaps.begin();
+       map_it != bitmaps.end(); ++map_it) {
+    for (std::vector<SkBitmap>::const_iterator bitmap_it =
+             map_it->second.begin();
+         bitmap_it != map_it->second.end(); ++bitmap_it) {
+      if (!bitmap_it->empty()) {
+        WebApplicationInfo::IconInfo icon_info;
+        icon_info.data = *bitmap_it;
+        icon_info.width = icon_info.data.width();
+        icon_info.height = icon_info.data.height();
+        install_info.icons.push_back(icon_info);
+      }
+    }
+  }
+
+  Profile* profile =
+      Profile::FromBrowserContext(web_contents()->GetBrowserContext());
+  scoped_refptr<extensions::CrxInstaller> installer(
+      extensions::CrxInstaller::CreateSilent(profile->GetExtensionService()));
+  installer->set_error_on_unsupported_requirements(true);
+  installer->InstallWebApp(install_info);
+}
+
 void TabHelper::DidCloneToNewWebContents(WebContents* old_web_contents,
                                          WebContents* new_web_contents) {
   // When the WebContents that this is attached to is cloned, give the new clone
   // a TabHelper and copy state over.
   CreateForWebContents(new_web_contents);
   TabHelper* new_helper = FromWebContents(new_web_contents);
 
   new_helper->SetExtensionApp(extension_app());
   new_helper->extension_app_icon_ = extension_app_icon_;
 }
@@ skipping 10 matching lines @@
     return;
 
   switch (pending_web_app_action_) {
     case CREATE_SHORTCUT: {
       chrome::ShowCreateWebAppShortcutsDialog(
           web_contents()->GetView()->GetTopLevelNativeWindow(),
           web_contents());
       break;
     }
     case CREATE_HOSTED_APP: {
-      CreateHostedApp(info);
+      CreateHostedApp();
       break;
     }
     case UPDATE_SHORTCUT: {
       web_app::UpdateShortcutForTabContents(web_contents());
       break;
     }
     default:
       NOTREACHED();
       break;
   }
 
   // The hosted app action will be cleared once the installation completes or
   // fails.
   if (pending_web_app_action_ != CREATE_HOSTED_APP)
     pending_web_app_action_ = NONE;
 #endif
 }
 
-void TabHelper::CreateHostedApp(const WebApplicationInfo& info) {
-  ShellIntegration::ShortcutInfo shortcut_info;
-  web_app::GetShortcutInfoForTab(web_contents(), &shortcut_info);

[benwells, 2013/12/12 21:17:57]

Is GetShortcutInfoForTab still used?

[calamity, 2013/12/13 03:11:55]

Yes, in the create applications shortcut dialog =(

-  WebApplicationInfo web_app_info;
-
-  web_app_info.is_bookmark_app = true;
-  web_app_info.app_url = shortcut_info.url;
-  web_app_info.title = shortcut_info.title;
-  web_app_info.urls.push_back(web_app_info.app_url);
-
-  // TODO(calamity): this should attempt to download the best icon that it can
-  // from |info.icons| rather than just using the favicon as it scales up badly.
-  // Fix this once |info.icons| gets populated commonly.
-
-  // Get the smallest icon in the icon family (should have only 1).
-  const gfx::Image* icon = shortcut_info.favicon.GetBest(0, 0);
-  SkBitmap bitmap = icon ? icon->AsBitmap() : SkBitmap();
-
-  if (!icon->IsEmpty()) {
-    WebApplicationInfo::IconInfo icon_info;
-    icon_info.data = bitmap;
-    icon_info.width = icon_info.data.width();
-    icon_info.height = icon_info.data.height();
-    web_app_info.icons.push_back(icon_info);
-  }
-
-  ExtensionService* service = profile_->GetExtensionService();
-  scoped_refptr<extensions::CrxInstaller> installer(
-      extensions::CrxInstaller::CreateSilent(service));
-  installer->set_error_on_unsupported_requirements(true);
-  installer->InstallWebApp(web_app_info);
-}
-
 void TabHelper::OnInlineWebstoreInstall(
     int install_id,
     int return_route_id,
     const std::string& webstore_item_id,
     const GURL& requestor_url) {
   WebstoreStandaloneInstaller::Callback callback =
       base::Bind(&TabHelper::OnInlineInstallComplete, base::Unretained(this),
                  install_id, return_route_id);
   scoped_refptr<WebstoreInlineInstaller> installer(
       webstore_inline_installer_factory_->CreateInstaller(
@@ skipping 167 matching lines @@
           GetApplicationInfo(entry->GetPageID());
         else
           pending_web_app_action_ = NONE;
       }
       break;
     }
     case chrome::NOTIFICATION_CRX_INSTALLER_DONE: {
       if (pending_web_app_action_ != CREATE_HOSTED_APP)
         return;
 
+      favicon_downloader_.reset();

[benwells, 2013/12/12 21:17:57]

Can the downloader be removed once it is finished, instead of here? That would
be much cleaner as FinishCreateHostedApp could always do the reset and we
wouldn't need to reset in all possible install finishing notifications.

[calamity, 2013/12/13 03:11:55]

Done.

       pending_web_app_action_ = NONE;
 
       const Extension* extension =
           content::Details<const Extension>(details).ptr();
       if (!extension || !extension->from_bookmark())
         return;
 
       // If enabled, launch the app launcher and highlight the new app.
       // Otherwise, open the chrome://apps page in a new foreground tab.
       if (IsAppLauncherEnabled()) {
@@ skipping 16 matching lines @@
         browser->OpenURL(
             content::OpenURLParams(GURL(chrome::kChromeUIAppsURL),
                                    content::Referrer(),
                                    NEW_FOREGROUND_TAB,
                                    content::PAGE_TRANSITION_LINK,
                                    false));
       }
 #endif
     }
     case chrome::NOTIFICATION_EXTENSION_INSTALL_ERROR: {
-      if (pending_web_app_action_ == CREATE_HOSTED_APP)
+      if (pending_web_app_action_ == CREATE_HOSTED_APP) {
+        favicon_downloader_.reset();
         pending_web_app_action_ = NONE;
+      }
       break;
     }
     case chrome::NOTIFICATION_EXTENSION_UNLOADED: {
       if (script_bubble_controller_) {
         script_bubble_controller_->OnExtensionUnloaded(
             content::Details<extensions::UnloadedExtensionInfo>(
                 details)->extension->id());
         break;
       }
     }
   }
 }
 
 void TabHelper::SetTabId(RenderViewHost* render_view_host) {
   render_view_host->Send(
       new ExtensionMsg_SetTabId(render_view_host->GetRoutingID(),
                                 SessionID::IdForTab(web_contents())));
 }
 
 }  // namespace extensions