ntp4: improved app install, try 2

chrome/browser/extensions/extension_install_ui.cc

 // Copyright (c) 2011 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/extension_install_ui.h"
 
 #include <map>
 
 #include "base/command_line.h"
 #include "base/file_util.h"
@@ skipping 214 matching lines @@
     default:
       NOTREACHED() << "Unknown message";
       break;
   }
 }
 
 // static
 void ExtensionInstallUI::OpenAppInstalledNTP(Browser* browser,
                                              const std::string& app_id) {
   std::string url = base::StringPrintf(
       "%s#app-id=%s", chrome::kChromeUINewTabURL, app_id.c_str());

[Rick Byers, 2011/08/29 16:01:00]

Why do you still need the code for setting and reading the hash?  I would have
thought that supporting the "already open NTP" case with the notification could
also easily handle the case where no NTP is open.  It's confusing having two
different channels for the information about which app to highlight....

[Evan Stade, 2011/08/29 18:15:16]

hmm. I tried to do what you're suggesting and it didn't work, so I assumed that
the new AppLauncherHandler wasn't being constructed till after this notification
(i.e. that Navigate wasn't creating it synchronously), although now that I test
construction order explicitly that doesn't seem to be the case. I'll see if I
can fix it because I agree the #app-id hack is annoying.

   browser::NavigateParams params =
       browser->GetSingletonTabNavigateParams(GURL(url));
-  params.path_behavior = browser::NavigateParams::IGNORE_AND_NAVIGATE;
   browser::Navigate(&params);
+
+  NotificationService::current()->Notify(
+      chrome::NOTIFICATION_APP_INSTALLED_TO_NTP,
+      Source<TabContents>(params.target_contents->tab_contents()),
+      Details<const std::string>(&app_id));
 }
 
 // static
 void ExtensionInstallUI::DisableFailureUIForTests() {
   disable_failure_ui_for_tests = true;
 }
 
 void ExtensionInstallUI::ShowThemeInfoBar(const std::string& previous_theme_id,
                                           bool previous_using_native_theme,
                                           const Extension* new_theme,
@@ skipping 51 matching lines @@
 
 InfoBarDelegate* ExtensionInstallUI::GetNewThemeInstalledInfoBarDelegate(
     TabContents* tab_contents,
     const Extension* new_theme,
     const std::string& previous_theme_id,
     bool previous_using_native_theme) {
   return new ThemeInstalledInfoBarDelegate(tab_contents, new_theme,
                                            previous_theme_id,
                                            previous_using_native_theme);
 }