Changes to default apps promo per ui leads:

1. Change hide message from 'Hide this message' to 'No thanks, hide this'.

2. Hide promo text when a default app is launched (this was already happening on uninstall).

3. Uninstall default apps when promo expires or is purposefully hidden.

4. Collapse apps section when promo expires or is purposefully hidden.

BUG=67292,69268
TEST=none

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

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

chrome/app/generated_resources.grd

@@ -3945,13 +3945,13 @@ Keep your key file in a safe place. You will need it to create new versions of y
         New! Discover a world of apps & games
       </message>
       <message name="IDS_APPS_PROMO_TEXT_1" desc="First paragraph of text on the apps promo">
-        Visit the &lt;a&gt; Chrome Web Store&lt;/a&gt; to add great apps and games to your new tab page in Chrome.
+        Visit the &lt;a&gt; Chrome Web Store&lt;/a&gt; to add great apps and games to this area of the New Tab page.
       </message>
       <message name="IDS_APPS_PROMO_TEXT_2" desc="Second paragraph of text on the apps promo">
         What's an app? Try one of these:
       </message>
       <message name="IDS_APPS_PROMO_HIDE" desc="Text on the button that hides the apps promo">
-        Hide this message
+        No thanks, hide this
       </message>
       <!-- Plugins -->

chrome/browser/dom_ui/app_launcher_handler.cc

@@ -14,6 +14,7 @@
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/app_launched_animation.h"
+#include "chrome/browser/dom_ui/shown_sections_handler.h"
 #include "chrome/browser/extensions/default_apps.h"
 #include "chrome/browser/extensions/extension_prefs.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ -77,7 +78,8 @@ bool IsPromoActive(const std::string& path) {
 
 AppLauncherHandler::AppLauncherHandler(ExtensionService* extension_service)
     : extensions_service_(extension_service),
-      promo_active_(false) {
+      promo_active_(false),
+      ignore_changes_(false) {
 }
 
 AppLauncherHandler::~AppLauncherHandler() {}
@@ -113,16 +115,28 @@ void AppLauncherHandler::CreateAppInfo(const Extension* extension,
 }
 
 // static
-bool AppLauncherHandler::HandlePing(const std::string& path) {
-  if (path.find(kLaunchWebStorePingURL) != std::string::npos) {
-    RecordWebStoreLaunch(IsPromoActive(path));
-    return true;
-  } else if (path.find(kLaunchAppPingURL) != std::string::npos) {
-    RecordAppLaunch(IsPromoActive(path));
-    return true;
-  }
+bool AppLauncherHandler::HandlePing(Profile* profile, const std::string& path) {
+  bool is_web_store_ping =
+      path.find(kLaunchWebStorePingURL) != std::string::npos;
+  bool is_app_launch_ping =
+      path.find(kLaunchAppPingURL) != std::string::npos;
 
-  return false;
+  // We get called for every URL in chrome://newtab/. Return false if it isn't
+  // one we handle.
+  if (!is_web_store_ping && !is_app_launch_ping)
+    return false;
+
+  bool is_promo_active = IsPromoActive(path);
+
+  if (is_web_store_ping)
+    RecordWebStoreLaunch(is_promo_active);
+  else
+    RecordAppLaunch(is_promo_active);
+
+  if (is_promo_active)
+    profile->GetExtensionService()->default_apps()->SetPromoHidden();
+
+  return true;
 }
 
 DOMMessageHandler* AppLauncherHandler::Attach(DOMUI* dom_ui) {
@@ -148,6 +162,9 @@ void AppLauncherHandler::RegisterMessages() {
 void AppLauncherHandler::Observe(NotificationType type,
                                  const NotificationSource& source,
                                  const NotificationDetails& details) {
+  if (ignore_changes_)
+    return;
+
   switch (type.value) {
     case NotificationType::EXTENSION_LOADED:
     case NotificationType::EXTENSION_UNLOADED:
@@ -203,7 +220,6 @@ void AppLauncherHandler::FillAppDictionary(DictionaryValue* dictionary) {
 
 void AppLauncherHandler::HandleGetApps(const ListValue* args) {
   DictionaryValue dictionary;
-  FillAppDictionary(&dictionary);
 
   // Tell the client whether to show the promo for this view. We don't do this
   // in the case of PREF_CHANGED because:
@@ -214,15 +230,24 @@ void AppLauncherHandler::HandleGetApps(const ListValue* args) {
   // b) Conceptually, it doesn't really make sense to count a
   //    prefchange-triggered refresh as a promo 'view'.
   DefaultApps* default_apps = extensions_service_->default_apps();
-  if (default_apps->ShouldShowPromo(extensions_service_->GetAppIds())) {
+  bool promo_just_expired = false;
+  if (default_apps->ShouldShowPromo(extensions_service_->GetAppIds(),
+                                    &promo_just_expired)) {
     dictionary.SetBoolean("showPromo", true);
-    default_apps->DidShowPromo();
     promo_active_ = true;
   } else {
+    if (promo_just_expired) {
+      ignore_changes_ = true;
+      UninstallDefaultApps();
+      ignore_changes_ = false;
+      ShownSectionsHandler::SetShownSection(dom_ui_->GetProfile()->GetPrefs(),
+                                            THUMB);
+    }
     dictionary.SetBoolean("showPromo", false);
     promo_active_ = false;
   }
 
+  FillAppDictionary(&dictionary);
   dom_ui_->CallJavascriptFunction(L"getAppsCallback", dictionary);
 
   // First time we get here we set up the observer so that we can tell update
@@ -289,8 +314,10 @@ void AppLauncherHandler::HandleLaunchApp(const ListValue* args) {
   if (new_contents != old_contents && browser->tab_count() > 1)
     browser->CloseTabContents(old_contents);
 
-  if (extension_id != extension_misc::kWebStoreAppId)
+  if (extension_id != extension_misc::kWebStoreAppId) {
     RecordAppLaunch(promo_active_);
+    extensions_service_->default_apps()->SetPromoHidden();
+  }
 }
 
 void AppLauncherHandler::HandleSetLaunchType(const ListValue* args) {
@@ -333,15 +360,13 @@ void AppLauncherHandler::HandleHideAppsPromo(const ListValue* args) {
                             extension_misc::PROMO_CLOSE,
                             extension_misc::PROMO_BUCKET_BOUNDARY);
 
-  DefaultApps* default_apps = extensions_service_->default_apps();
-  const ExtensionIdSet& app_ids = default_apps->default_apps();
-  for (ExtensionIdSet::const_iterator iter = app_ids.begin();
-       iter != app_ids.end(); ++iter) {
-    if (extensions_service_->GetExtensionById(*iter, true))
-      extensions_service_->UninstallExtension(*iter, false);
-  }
-
+  ShownSectionsHandler::SetShownSection(dom_ui_->GetProfile()->GetPrefs(),
+                                        THUMB);
+  ignore_changes_ = true;
+  UninstallDefaultApps();
   extensions_service_->default_apps()->SetPromoHidden();
+  ignore_changes_ = false;
+  HandleGetApps(NULL);
 }
 
 void AppLauncherHandler::HandleCreateAppShortcut(const ListValue* args) {
@@ -420,3 +445,13 @@ void AppLauncherHandler::AnimateAppIcon(const Extension* extension,
 #endif
   }
 }
+
+void AppLauncherHandler::UninstallDefaultApps() {
+  DefaultApps* default_apps = extensions_service_->default_apps();
+  const ExtensionIdSet& app_ids = default_apps->default_apps();
+  for (ExtensionIdSet::const_iterator iter = app_ids.begin();
+       iter != app_ids.end(); ++iter) {
+    if (extensions_service_->GetExtensionById(*iter, true))
+      extensions_service_->UninstallExtension(*iter, false);
+  }
+}

chrome/browser/dom_ui/app_launcher_handler.h

@@ -18,6 +18,7 @@ class ExtensionPrefs;
 class ExtensionService;
 class NotificationRegistrar;
 class PrefChangeRegistrar;
+class Profile;
 
 namespace gfx {
   class Rect;
@@ -38,7 +39,7 @@ class AppLauncherHandler
                             DictionaryValue* value);
 
   // Callback for pings related to launching apps on the NTP.
-  static bool HandlePing(const std::string& path);
+  static bool HandlePing(Profile* profile, const std::string& path);
 
   // DOMMessageHandler implementation.
   virtual DOMMessageHandler* Attach(DOMUI* dom_ui);
@@ -91,6 +92,9 @@ class AppLauncherHandler
   // Starts the animation of the app icon.
   void AnimateAppIcon(const Extension* extension, const gfx::Rect& rect);
 
+  // Helper that uninstalls all the default apps.
+  void UninstallDefaultApps();
+
   // The apps are represented in the extensions model.
   scoped_refptr<ExtensionService> extensions_service_;
 
@@ -111,6 +115,10 @@ class AppLauncherHandler
   // Whether the promo is currently being shown.
   bool promo_active_;
 
+  // When true, we ignore changes to the underlying data rather than immediately
+  // refreshing. This is useful when making many batch updates to avoid flicker.
+  bool ignore_changes_;
+
   DISALLOW_COPY_AND_ASSIGN(AppLauncherHandler);
 };
 

chrome/browser/dom_ui/new_tab_ui.cc

@@ -605,7 +605,7 @@ void NewTabUI::NewTabHTMLSource::StartDataRequest(const std::string& path,
                                                   int request_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  if (AppLauncherHandler::HandlePing(path)) {
+  if (AppLauncherHandler::HandlePing(profile_, path)) {
     return;
   } else if (!path.empty() && path[0] != '#') {
     // A path under new-tab was requested; it's likely a bad relative

chrome/browser/dom_ui/shown_sections_handler.cc

@@ -48,6 +48,15 @@ int ShownSectionsHandler::GetShownSections(PrefService* prefs) {
   return prefs->GetInteger(prefs::kNTPShownSections);
 }
 
+// static
+void ShownSectionsHandler::SetShownSection(PrefService* prefs,
+                                           Section section) {
+  int shown_sections = GetShownSections(prefs);
+  shown_sections &= ~ALL_SECTIONS_MASK;
+  shown_sections |= section;
+  prefs->SetInteger(prefs::kNTPShownSections, shown_sections);
+}
+
 ShownSectionsHandler::ShownSectionsHandler(PrefService* pref_service)
     : pref_service_(pref_service) {
   pref_registrar_.Init(pref_service);

chrome/browser/dom_ui/shown_sections_handler.h

@@ -43,6 +43,10 @@ class ShownSectionsHandler : public DOMMessageHandler,
   // Helper to get the current shown sections.
   static int GetShownSections(PrefService* pref_service);
 
+  // Expands |section|, collapsing any previously expanded section. This is the
+  // same thing that happens if a user clicks on |section|.
+  static void SetShownSection(PrefService* prefs, Section section);
+
   // DOMMessageHandler implementation.
   virtual void RegisterMessages();
 

chrome/browser/extensions/default_apps.cc

@@ -87,7 +87,10 @@ bool DefaultApps::ShouldShowAppLauncher(const ExtensionIdSet& installed_ids) {
 #endif
 }
 
-bool DefaultApps::ShouldShowPromo(const ExtensionIdSet& installed_ids) {
+bool DefaultApps::ShouldShowPromo(const ExtensionIdSet& installed_ids,
+                                  bool* just_expired) {
+  *just_expired = false;
+
   if (CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kForceAppsPromoVisible)) {
     return true;
@@ -96,16 +99,35 @@ bool DefaultApps::ShouldShowPromo(const ExtensionIdSet& installed_ids) {
   if (!DefaultAppSupported())
     return false;
 
-  if (GetDefaultAppsInstalled() && GetPromoCounter() < kAppsPromoCounterMax) {
+  if (!GetDefaultAppsInstalled())
+    return false;
+
+  int promo_counter = GetPromoCounter();
+  if (promo_counter <= kAppsPromoCounterMax) {
     // If we have the exact set of default apps, show the promo. If we don't
     // have the exact set of default apps, this means that the user manually
     // installed or uninstalled one. The promo doesn't make sense if it shows
     // apps the user manually installed, so expire it immediately in that
     // situation.
-    if (installed_ids == ids_)
-      return true;
-    else
+    if (ids_ != installed_ids) {
       SetPromoHidden();
+      return false;
+    }
+
+    if (promo_counter == kAppsPromoCounterMax) {
+      *just_expired = true;
+      UMA_HISTOGRAM_ENUMERATION(extension_misc::kAppsPromoHistogram,
+                                extension_misc::PROMO_EXPIRE,
+                                extension_misc::PROMO_BUCKET_BOUNDARY);
+      SetPromoCounter(++promo_counter);
+      return false;
+    } else {
+      UMA_HISTOGRAM_ENUMERATION(extension_misc::kAppsPromoHistogram,
+                                extension_misc::PROMO_SEEN,
+                                extension_misc::PROMO_BUCKET_BOUNDARY);
+      SetPromoCounter(++promo_counter);
+      return true;
+    }
   }
 
   return false;
@@ -123,29 +145,6 @@ void DefaultApps::DidInstallApp(const ExtensionIdSet& installed_ids) {
   }
 }
 
-void DefaultApps::DidShowPromo() {
-  if (!GetDefaultAppsInstalled()) {
-    NOTREACHED() << "Should not show promo until default apps are installed.";
-    return;
-  }
-
-  int promo_counter = GetPromoCounter();
-  if (promo_counter == kAppsPromoCounterMax) {
-    NOTREACHED() << "Promo has already been shown the maximum number of times.";
-    return;
-  }
-
-  if (promo_counter < kAppsPromoCounterMax) {
-    if (promo_counter + 1 == kAppsPromoCounterMax)
-      UMA_HISTOGRAM_ENUMERATION(extension_misc::kAppsPromoHistogram,
-                                extension_misc::PROMO_EXPIRE,
-                                extension_misc::PROMO_BUCKET_BOUNDARY);
-    SetPromoCounter(++promo_counter);
-  } else {
-    SetPromoHidden();
-  }
-}
-
 bool DefaultApps::NonDefaultAppIsInstalled(
     const ExtensionIdSet& installed_ids) const {
   for (ExtensionIdSet::const_iterator iter = installed_ids.begin();
@@ -158,7 +157,7 @@ bool DefaultApps::NonDefaultAppIsInstalled(
 }
 
 void DefaultApps::SetPromoHidden() {
-  SetPromoCounter(kAppsPromoCounterMax);
+  SetPromoCounter(kAppsPromoCounterMax + 1);
 }
 
 int DefaultApps::GetPromoCounter() const {

chrome/browser/extensions/default_apps.h

@@ -19,7 +19,9 @@ class PrefService;
 // - Whether to show the apps promo in the launcher
 class DefaultApps {
  public:
-  // The maximum number of times to show the apps promo.
+  // The maximum number of times to show the apps promo. The promo counter
+  // actually goes up to this number + 1 because we need to differentiate
+  // between the first time we overflow and subsequent times.
   static const int kAppsPromoCounterMax;
 
   // Register our preferences.
@@ -42,15 +44,12 @@ class DefaultApps {
   //
   // NOTE: If the default apps have been installed, but |installed_ids| is
   // different than GetDefaultApps(), this will permanently expire the promo.
-  bool ShouldShowPromo(const ExtensionIdSet& installed_ids);
+  bool ShouldShowPromo(const ExtensionIdSet& installed_ids, bool* just_expired);
 
   // Should be called after each app is installed. Once installed_ids contains
   // all the default apps, GetAppsToInstall() will start returning NULL.
   void DidInstallApp(const ExtensionIdSet& installed_ids);
 
-  // Should be called after each time the promo is installed.
-  void DidShowPromo();
-
   // Force the promo to be hidden.
   void SetPromoHidden();
 

chrome/browser/extensions/default_apps_unittest.cc

@@ -30,7 +30,10 @@ TEST(ExtensionDefaultApps, HappyPath) {
   EXPECT_FALSE(default_apps.ShouldShowAppLauncher(installed_app_ids));
 
   // The promo should not be shown until the default apps have been installed.
-  EXPECT_FALSE(default_apps.ShouldShowPromo(installed_app_ids));
+  bool promo_just_expired = false;
+  EXPECT_FALSE(default_apps.ShouldShowPromo(installed_app_ids,
+                                            &promo_just_expired));
+  EXPECT_FALSE(promo_just_expired);
 
   // Simulate installing the apps one by one and notifying default_apps after
   // each intallation. Nothing should change until we have installed all the
@@ -44,7 +47,9 @@ TEST(ExtensionDefaultApps, HappyPath) {
     EXPECT_FALSE(default_apps.GetDefaultAppsInstalled());
     EXPECT_TRUE(default_apps.ShouldInstallDefaultApps(installed_app_ids));
     EXPECT_FALSE(default_apps.ShouldShowAppLauncher(installed_app_ids));
-    EXPECT_FALSE(default_apps.ShouldShowPromo(installed_app_ids));
+    EXPECT_FALSE(default_apps.ShouldShowPromo(installed_app_ids,
+                                              &promo_just_expired));
+    EXPECT_FALSE(promo_just_expired);
   }
 
   // Simulate all the default apps being installed. Now we should stop getting
@@ -56,23 +61,36 @@ TEST(ExtensionDefaultApps, HappyPath) {
 
   // And the promo and launcher should become available.
   EXPECT_TRUE(default_apps.ShouldShowAppLauncher(installed_app_ids));
-  EXPECT_TRUE(default_apps.ShouldShowPromo(installed_app_ids));
+  EXPECT_TRUE(default_apps.ShouldShowPromo(installed_app_ids,
+                                           &promo_just_expired));
+  EXPECT_FALSE(promo_just_expired);
 
   // The promo should be available up to the max allowed times, then stop.
-  for (int i = 0; i < DefaultApps::kAppsPromoCounterMax; ++i) {
-    EXPECT_TRUE(default_apps.ShouldShowPromo(installed_app_ids));
-    default_apps.DidShowPromo();
+  // We start counting at 1 because of the call to ShouldShowPromo() above.
+  for (int i = 1; i < DefaultApps::kAppsPromoCounterMax; ++i) {
+    EXPECT_TRUE(default_apps.ShouldShowPromo(installed_app_ids,
+                                             &promo_just_expired));
+    EXPECT_FALSE(promo_just_expired);
     EXPECT_EQ(i + 1, default_apps.GetPromoCounter());
   }
-  EXPECT_FALSE(default_apps.ShouldShowPromo(installed_app_ids));
-  EXPECT_EQ(DefaultApps::kAppsPromoCounterMax, default_apps.GetPromoCounter());
+
+  // The first time, should_show_promo should flip to true, then back to false.
+  EXPECT_FALSE(default_apps.ShouldShowPromo(installed_app_ids,
+                                            &promo_just_expired));
+  EXPECT_TRUE(promo_just_expired);
+  EXPECT_EQ(DefaultApps::kAppsPromoCounterMax + 1,
+            default_apps.GetPromoCounter());
 
   // Even if all the apps are subsequently removed, the apps section should
   // remain.
   installed_app_ids.clear();
   EXPECT_FALSE(default_apps.ShouldInstallDefaultApps(installed_app_ids));
   EXPECT_TRUE(default_apps.ShouldShowAppLauncher(installed_app_ids));
-  EXPECT_FALSE(default_apps.ShouldShowPromo(installed_app_ids));
+  EXPECT_FALSE(default_apps.ShouldShowPromo(installed_app_ids,
+                                            &promo_just_expired));
+  EXPECT_FALSE(promo_just_expired);
+  EXPECT_EQ(DefaultApps::kAppsPromoCounterMax + 1,
+            default_apps.GetPromoCounter());
 }
 
 TEST(ExtensionDefaultApps, UnsupportedLocale) {
@@ -88,28 +106,38 @@ TEST(ExtensionDefaultApps, UnsupportedLocale) {
   ExtensionIdSet installed_ids;
   EXPECT_FALSE(default_apps.ShouldInstallDefaultApps(installed_ids));
   EXPECT_FALSE(default_apps.ShouldShowAppLauncher(installed_ids));
-  EXPECT_FALSE(default_apps.ShouldShowPromo(installed_ids));
+
+  bool promo_just_expired = false;
+  EXPECT_FALSE(default_apps.ShouldShowPromo(installed_ids,
+                                            &promo_just_expired));
+  EXPECT_FALSE(promo_just_expired);
 
   // If the user installs an app manually, then we show the apps section, but
   // no promotion or default apps.
   installed_ids.insert(*(default_app_ids.begin()));
   EXPECT_FALSE(default_apps.ShouldInstallDefaultApps(installed_ids));
   EXPECT_TRUE(default_apps.ShouldShowAppLauncher(installed_ids));
-  EXPECT_FALSE(default_apps.ShouldShowPromo(installed_ids));
+  EXPECT_FALSE(default_apps.ShouldShowPromo(installed_ids,
+                                            &promo_just_expired));
+  EXPECT_FALSE(promo_just_expired);
 
   // Even if the user installs the exact set of default apps, we don't show the
   // promo.
   installed_ids = default_app_ids;
   EXPECT_FALSE(default_apps.ShouldInstallDefaultApps(installed_ids));
   EXPECT_TRUE(default_apps.ShouldShowAppLauncher(installed_ids));
-  EXPECT_FALSE(default_apps.ShouldShowPromo(installed_ids));
+  EXPECT_FALSE(default_apps.ShouldShowPromo(installed_ids,
+                                            &promo_just_expired));
+  EXPECT_FALSE(promo_just_expired);
 
   // If the user uninstalls the apps again, we go back to not showing the
   // apps section.
   installed_ids.clear();
   EXPECT_FALSE(default_apps.ShouldInstallDefaultApps(installed_ids));
   EXPECT_FALSE(default_apps.ShouldShowAppLauncher(installed_ids));
-  EXPECT_FALSE(default_apps.ShouldShowPromo(installed_ids));
+  EXPECT_FALSE(default_apps.ShouldShowPromo(installed_ids,
+                                            &promo_just_expired));
+  EXPECT_FALSE(promo_just_expired);
 }
 
 TEST(ExtensionDefaultApps, HidePromo) {
@@ -120,13 +148,18 @@ TEST(ExtensionDefaultApps, HidePromo) {
   const ExtensionIdSet& default_app_ids = default_apps.default_apps();
   default_apps.DidInstallApp(default_app_ids);
 
-  EXPECT_TRUE(default_apps.ShouldShowPromo(default_app_ids));
-  default_apps.DidShowPromo();
+  bool promo_just_expired = false;
+  EXPECT_TRUE(default_apps.ShouldShowPromo(default_app_ids,
+                                           &promo_just_expired));
+  EXPECT_FALSE(promo_just_expired);
   EXPECT_EQ(1, default_apps.GetPromoCounter());
 
   default_apps.SetPromoHidden();
-  EXPECT_FALSE(default_apps.ShouldShowPromo(default_app_ids));
-  EXPECT_EQ(DefaultApps::kAppsPromoCounterMax, default_apps.GetPromoCounter());
+  EXPECT_FALSE(default_apps.ShouldShowPromo(default_app_ids,
+                                            &promo_just_expired));
+  EXPECT_FALSE(promo_just_expired);
+  EXPECT_EQ(DefaultApps::kAppsPromoCounterMax + 1,
+            default_apps.GetPromoCounter());
 }
 
 TEST(ExtensionDefaultApps, InstallingAnAppHidesPromo) {
@@ -138,15 +171,20 @@ TEST(ExtensionDefaultApps, InstallingAnAppHidesPromo) {
   ExtensionIdSet installed_app_ids = default_app_ids;
   default_apps.DidInstallApp(installed_app_ids);
 
-  EXPECT_TRUE(default_apps.ShouldShowPromo(installed_app_ids));
-  default_apps.DidShowPromo();
+  bool promo_just_expired = false;
+  EXPECT_TRUE(default_apps.ShouldShowPromo(installed_app_ids,
+                                           &promo_just_expired));
+  EXPECT_FALSE(promo_just_expired);
   EXPECT_EQ(1, default_apps.GetPromoCounter());
 
   // Now simulate a new extension being installed. This should cause the promo
   // to be hidden.
   installed_app_ids.insert("foo");
-  EXPECT_FALSE(default_apps.ShouldShowPromo(installed_app_ids));
-  EXPECT_EQ(DefaultApps::kAppsPromoCounterMax, default_apps.GetPromoCounter());
+  EXPECT_FALSE(default_apps.ShouldShowPromo(installed_app_ids,
+                                            &promo_just_expired));
+  EXPECT_FALSE(promo_just_expired);
+  EXPECT_EQ(DefaultApps::kAppsPromoCounterMax + 1,
+            default_apps.GetPromoCounter());
 }
 
 TEST(ExtensionDefaultApps, ManualAppInstalledWhileInstallingDefaultApps) {
@@ -173,8 +211,12 @@ TEST(ExtensionDefaultApps, ManualAppInstalledWhileInstallingDefaultApps) {
 
   // The promo shouldn't turn on though, because it would look weird with the
   // user's extra, manually installed extensions.
-  EXPECT_FALSE(default_apps.ShouldShowPromo(installed_ids));
-  EXPECT_EQ(DefaultApps::kAppsPromoCounterMax, default_apps.GetPromoCounter());
+  bool promo_just_expired = false;
+  EXPECT_FALSE(default_apps.ShouldShowPromo(installed_ids,
+                                            &promo_just_expired));
+  EXPECT_FALSE(promo_just_expired);
+  EXPECT_EQ(DefaultApps::kAppsPromoCounterMax + 1,
+            default_apps.GetPromoCounter());
 
   // Going back to a subset of the default apps shouldn't allow the default app
   // install to continue.
@@ -182,9 +224,13 @@ TEST(ExtensionDefaultApps, ManualAppInstalledWhileInstallingDefaultApps) {
   EXPECT_FALSE(default_apps.ShouldInstallDefaultApps(installed_ids));
   EXPECT_TRUE(default_apps.GetDefaultAppsInstalled());
   EXPECT_TRUE(default_apps.ShouldShowAppLauncher(installed_ids));
-  EXPECT_FALSE(default_apps.ShouldShowPromo(installed_ids));
+  EXPECT_FALSE(default_apps.ShouldShowPromo(installed_ids,
+                                            &promo_just_expired));
+  EXPECT_FALSE(promo_just_expired);
 
   // Going to the exact set of default apps shouldn't show the promo.
-  EXPECT_FALSE(default_apps.ShouldShowPromo(default_apps.default_apps()));
+  EXPECT_FALSE(default_apps.ShouldShowPromo(default_apps.default_apps(),
+                                            &promo_just_expired));
+  EXPECT_FALSE(promo_just_expired);
 }
 #endif  // OS_CHROMEOS

chrome/browser/resources/new_new_tab.html

@@ -143,7 +143,7 @@ if ('mode' in hashParams) {
     <div id="apps-promo">
       <button id="apps-promo-hide" i18n-content="appspromohide"></button>
       <h3 i18n-content="appspromoheader"></h3>
-      <p id="apps-promo-text1" i18n-content="appspromotext1"></p>
+      <p id="apps-promo-text1" i18n-values=".innerHTML:appspromotext1"></p>
       <p id="apps-promo-text2" i18n-content="appspromotext2"></p>
     </div>
     <div id="apps-content"></div>

chrome/browser/resources/new_new_tab.js

@@ -1379,10 +1379,7 @@ function isDoneLoading() {
 
 // Initialize the apps promo.
 document.addEventListener('DOMContentLoaded', function() {
-  var promoText1 = $('apps-promo-text1');
-  promoText1.innerHTML = promoText1.textContent;
-
-  var promoLink = promoText1.querySelector('a');
+  var promoLink = document.querySelector('#apps-promo-text1 a');
   promoLink.id = 'apps-promo-link';
   promoLink.href = localStrings.getString('web_store_url');
 

chrome/browser/resources/ntp/apps.js

@@ -78,7 +78,10 @@ function getAppsCallback(data) {
     document.documentElement.classList.add('apps-promo-visible');
   else
     document.documentElement.classList.remove('apps-promo-visible');
-  $('apps-promo-link').setAttribute('ping', appsPromoPing);
+
+  var appsPromoLink = $('apps-promo-link');
+  if (appsPromoLink)
+    appsPromoLink.setAttribute('ping', appsPromoPing);
   maybeDoneLoading();
 
   if (isDoneLoading()) {

chrome/common/extensions/extension_constants.h

@@ -256,7 +256,8 @@ namespace extension_misc {
     PROMO_LAUNCH_WEB_STORE,
     PROMO_CLOSE,
     PROMO_EXPIRE,
-    PROMO_BUCKET_BOUNDARY = PROMO_EXPIRE + 1
+    PROMO_SEEN,
+    PROMO_BUCKET_BOUNDARY
   };
 }  // extension_misc