From e88fc9dbd34792aba1b49b8f7122c11205335b25 Mon Sep 17 00:00:00 2001
From: "aa@chromium.org" <aa@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Tue, 11 Jan 2011 22:36:48 +0000
Subject: [PATCH] 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 | 4 +-
chrome/browser/dom_ui/app_launcher_handler.cc | 79 ++++++++++-----
chrome/browser/dom_ui/app_launcher_handler.h | 10 +-
chrome/browser/dom_ui/new_tab_ui.cc | 2 +-
.../browser/dom_ui/shown_sections_handler.cc | 9 ++
.../browser/dom_ui/shown_sections_handler.h | 4 +
chrome/browser/extensions/default_apps.cc | 57 ++++++-----
chrome/browser/extensions/default_apps.h | 9 +-
.../extensions/default_apps_unittest.cc | 96 ++++++++++++++-----
chrome/browser/resources/new_new_tab.html | 2 +-
chrome/browser/resources/new_new_tab.js | 5 +-
chrome/browser/resources/ntp/apps.js | 5 +-
.../common/extensions/extension_constants.h | 3 +-
13 files changed, 193 insertions(+), 92 deletions(-)
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index f97ba5ff58cd3..fe58bc35e3eaa 100644
--- a/chrome/app/generated_resources.grd
+++ b/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 <a> Chrome Web Store</a> to add great apps and games to your new tab page in Chrome.
+ Visit the <a> Chrome Web Store</a> 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 -->
diff --git a/chrome/browser/dom_ui/app_launcher_handler.cc b/chrome/browser/dom_ui/app_launcher_handler.cc
index 21692b0e176b1..f1679c744118a 100644
--- a/chrome/browser/dom_ui/app_launcher_handler.cc
+++ b/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);
+ }
+}
diff --git a/chrome/browser/dom_ui/app_launcher_handler.h b/chrome/browser/dom_ui/app_launcher_handler.h
index 5f946d915c7c5..4940752a514b5 100644
--- a/chrome/browser/dom_ui/app_launcher_handler.h
+++ b/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);
};
diff --git a/chrome/browser/dom_ui/new_tab_ui.cc b/chrome/browser/dom_ui/new_tab_ui.cc
index c43a8f5017bea..ff0a86304b435 100644
--- a/chrome/browser/dom_ui/new_tab_ui.cc
+++ b/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
diff --git a/chrome/browser/dom_ui/shown_sections_handler.cc b/chrome/browser/dom_ui/shown_sections_handler.cc
index 291b9509545c5..164a23529fe66 100644
--- a/chrome/browser/dom_ui/shown_sections_handler.cc
+++ b/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);
diff --git a/chrome/browser/dom_ui/shown_sections_handler.h b/chrome/browser/dom_ui/shown_sections_handler.h
index ad227b4d46e59..41c7da831d8da 100644
--- a/chrome/browser/dom_ui/shown_sections_handler.h
+++ b/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();
diff --git a/chrome/browser/extensions/default_apps.cc b/chrome/browser/extensions/default_apps.cc
index 4afe3259ff409..cee5c324126db 100644
--- a/chrome/browser/extensions/default_apps.cc
+++ b/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 {
diff --git a/chrome/browser/extensions/default_apps.h b/chrome/browser/extensions/default_apps.h
index a8b162c0021cc..2ff880135fdde 100644
--- a/chrome/browser/extensions/default_apps.h
+++ b/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();
diff --git a/chrome/browser/extensions/default_apps_unittest.cc b/chrome/browser/extensions/default_apps_unittest.cc
index fd87d58e39b75..316568d0ae154 100644
--- a/chrome/browser/extensions/default_apps_unittest.cc
+++ b/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
diff --git a/chrome/browser/resources/new_new_tab.html b/chrome/browser/resources/new_new_tab.html
index f987bde74bced..68a60dcadd9df 100644
--- a/chrome/browser/resources/new_new_tab.html
+++ b/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>
diff --git a/chrome/browser/resources/new_new_tab.js b/chrome/browser/resources/new_new_tab.js
index a2dbc6bf1ecd3..f72fadd5eb21d 100644
--- a/chrome/browser/resources/new_new_tab.js
+++ b/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');
diff --git a/chrome/browser/resources/ntp/apps.js b/chrome/browser/resources/ntp/apps.js
index 973dada98aa7c..4dd8bc12ce14a 100644
--- a/chrome/browser/resources/ntp/apps.js
+++ b/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()) {
diff --git a/chrome/common/extensions/extension_constants.h b/chrome/common/extensions/extension_constants.h
index 7f09630cd5276..3ae072989e454 100644
--- a/chrome/common/extensions/extension_constants.h
+++ b/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
--
GitLab