diff options
author | jstritar@chromium.org <jstritar@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-06-07 00:16:33 +0000 |
---|---|---|
committer | jstritar@chromium.org <jstritar@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-06-07 00:16:33 +0000 |
commit | b6aed736288188a2b52d8d514f767cfcd666b897 (patch) | |
tree | fe53a149d1e5a7706de09eea372749da9524e2e0 | |
parent | fe8fec4d1562053f464fc6b2c960e51498e6a74f (diff) | |
download | chromium_src-b6aed736288188a2b52d8d514f767cfcd666b897.zip chromium_src-b6aed736288188a2b52d8d514f767cfcd666b897.tar.gz chromium_src-b6aed736288188a2b52d8d514f767cfcd666b897.tar.bz2 |
Updates when the web store promo and apps section is displayed on NTP.
The NTP will now show the apps section even if the web store promo fails to parse. The presence of part of the promo triggers the apps section.
This also adds the ability to choose what users will maximize a given web store promo and apps section. The promo can be maximized for no users, existing users, new users or everyone.
BUG=84561
TEST=PromoResourceServiceTest, ExtensionAppsPromo
Review URL: http://codereview.chromium.org/7054076
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@88061 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/browser/extensions/apps_promo.cc | 67 | ||||
-rw-r--r-- | chrome/browser/extensions/apps_promo.h | 42 | ||||
-rw-r--r-- | chrome/browser/extensions/apps_promo_unittest.cc | 174 | ||||
-rw-r--r-- | chrome/browser/resources/ntp/apps.js | 6 | ||||
-rw-r--r-- | chrome/browser/ui/webui/ntp/app_launcher_handler.cc | 3 | ||||
-rw-r--r-- | chrome/browser/web_resource/promo_resource_service.cc | 20 | ||||
-rw-r--r-- | chrome/browser/web_resource/promo_resource_service.h | 2 | ||||
-rw-r--r-- | chrome/browser/web_resource/promo_resource_service_unittest.cc | 30 | ||||
-rw-r--r-- | chrome/common/pref_names.cc | 6 | ||||
-rw-r--r-- | chrome/common/pref_names.h | 2 |
10 files changed, 291 insertions, 61 deletions
diff --git a/chrome/browser/extensions/apps_promo.cc b/chrome/browser/extensions/apps_promo.cc index 2644fea..98308c7 100644 --- a/chrome/browser/extensions/apps_promo.cc +++ b/chrome/browser/extensions/apps_promo.cc @@ -25,12 +25,14 @@ const char kDefaultPromoLogo[] = "chrome://theme/IDR_WEBSTORE_ICON"; // static void AppsPromo::RegisterPrefs(PrefService* local_state) { std::string empty; + local_state->RegisterBooleanPref(prefs::kNTPWebStoreEnabled, false); local_state->RegisterStringPref(prefs::kNTPWebStorePromoId, empty); local_state->RegisterStringPref(prefs::kNTPWebStorePromoHeader, empty); local_state->RegisterStringPref(prefs::kNTPWebStorePromoButton, empty); local_state->RegisterStringPref(prefs::kNTPWebStorePromoLink, empty); local_state->RegisterStringPref(prefs::kNTPWebStorePromoLogo, empty); local_state->RegisterStringPref(prefs::kNTPWebStorePromoExpire, empty); + local_state->RegisterIntegerPref(prefs::kNTPWebStorePromoUserGroup, 0); } // static @@ -51,12 +53,34 @@ void AppsPromo::RegisterUserPrefs(PrefService* prefs) { // static void AppsPromo::ClearPromo() { PrefService* local_state = g_browser_process->local_state(); + local_state->ClearPref(prefs::kNTPWebStoreEnabled); local_state->ClearPref(prefs::kNTPWebStorePromoId); local_state->ClearPref(prefs::kNTPWebStorePromoHeader); local_state->ClearPref(prefs::kNTPWebStorePromoButton); local_state->ClearPref(prefs::kNTPWebStorePromoLink); local_state->ClearPref(prefs::kNTPWebStorePromoLogo); local_state->ClearPref(prefs::kNTPWebStorePromoExpire); + local_state->ClearPref(prefs::kNTPWebStorePromoUserGroup); +} + +// static +bool AppsPromo::IsPromoSupportedForLocale() { + PrefService* local_state = g_browser_process->local_state(); + // PromoResourceService will clear the promo data if the current locale is + // not supported. + return local_state->HasPrefPath(prefs::kNTPWebStorePromoId) && + local_state->HasPrefPath(prefs::kNTPWebStorePromoHeader) && + local_state->HasPrefPath(prefs::kNTPWebStorePromoButton) && + local_state->HasPrefPath(prefs::kNTPWebStorePromoLink) && + local_state->HasPrefPath(prefs::kNTPWebStorePromoLogo) && + local_state->HasPrefPath(prefs::kNTPWebStorePromoExpire) && + local_state->HasPrefPath(prefs::kNTPWebStorePromoUserGroup); +} + +// static +bool AppsPromo::IsWebStoreSupportedForLocale() { + PrefService* local_state = g_browser_process->local_state(); + return local_state->GetBoolean(prefs::kNTPWebStoreEnabled); } // static @@ -99,12 +123,19 @@ std::string AppsPromo::GetPromoExpireText() { } // static +int AppsPromo::GetPromoUserGroup() { + PrefService* local_state = g_browser_process->local_state(); + return local_state->GetInteger(prefs::kNTPWebStorePromoUserGroup); +} + +// static void AppsPromo::SetPromo(const std::string& id, const std::string& header_text, const std::string& button_text, const GURL& link, const std::string& expire_text, - const GURL& logo) { + const GURL& logo, + const int user_group) { PrefService* local_state = g_browser_process->local_state(); local_state->SetString(prefs::kNTPWebStorePromoId, id); local_state->SetString(prefs::kNTPWebStorePromoButton, button_text); @@ -112,19 +143,13 @@ void AppsPromo::SetPromo(const std::string& id, local_state->SetString(prefs::kNTPWebStorePromoLink, link.spec()); local_state->SetString(prefs::kNTPWebStorePromoLogo, logo.spec()); local_state->SetString(prefs::kNTPWebStorePromoExpire, expire_text); + local_state->SetInteger(prefs::kNTPWebStorePromoUserGroup, user_group); } // static -bool AppsPromo::IsPromoSupportedForLocale() { +void AppsPromo::SetWebStoreSupportedForLocale(bool supported) { PrefService* local_state = g_browser_process->local_state(); - // PromoResourceService will clear the promo data if the current locale is - // not supported. - return local_state->HasPrefPath(prefs::kNTPWebStorePromoId) && - local_state->HasPrefPath(prefs::kNTPWebStorePromoHeader) && - local_state->HasPrefPath(prefs::kNTPWebStorePromoButton) && - local_state->HasPrefPath(prefs::kNTPWebStorePromoLink) && - local_state->HasPrefPath(prefs::kNTPWebStorePromoLogo) && - local_state->HasPrefPath(prefs::kNTPWebStorePromoExpire); + local_state->SetBoolean(prefs::kNTPWebStoreEnabled, supported); } AppsPromo::AppsPromo(PrefService* prefs) @@ -193,8 +218,9 @@ bool AppsPromo::ShouldShowAppLauncher(const ExtensionIdSet& installed_ids) { if (!installed_ids.empty()) return true; - // Otherwise, only show the app launcher if there's a promo for this locale. - return IsPromoSupportedForLocale(); + // Otherwise, only show the app launcher if the web store is supported for the + // current locale. + return IsWebStoreSupportedForLocale(); #endif } @@ -202,14 +228,16 @@ void AppsPromo::ExpireDefaultApps() { SetPromoCounter(kDefaultAppsCounterMax + 1); } -void AppsPromo::MaximizeAppsIfFirstView() { +void AppsPromo::MaximizeAppsIfNecessary() { std::string promo_id = GetPromoId(); + int maximize_setting = GetPromoUserGroup(); // Maximize the apps section of the NTP if this is the first time viewing the - // specific promo. + // specific promo and the current user group is targetted. if (GetLastPromoId() != promo_id) { - prefs_->SetString(prefs::kNTPWebStorePromoLastId, promo_id); - ShownSectionsHandler::SetShownSection(prefs_, APPS); + if ((maximize_setting & GetCurrentUserGroup()) != 0) + ShownSectionsHandler::SetShownSection(prefs_, APPS); + SetLastPromoId(promo_id); } } @@ -245,3 +273,10 @@ void AppsPromo::SetPromoCounter(int val) { bool AppsPromo::GetDefaultAppsInstalled() const { return prefs_->GetBoolean(prefs::kDefaultAppsInstalled); } + +AppsPromo::UserGroup AppsPromo::GetCurrentUserGroup() const { + const PrefService::Preference* last_promo_id + = prefs_->FindPreference(prefs::kNTPWebStorePromoLastId); + CHECK(last_promo_id); + return last_promo_id->IsDefaultValue() ? USERS_NEW : USERS_EXISTING; +} diff --git a/chrome/browser/extensions/apps_promo.h b/chrome/browser/extensions/apps_promo.h index 31abbe0..1715180 100644 --- a/chrome/browser/extensions/apps_promo.h +++ b/chrome/browser/extensions/apps_promo.h @@ -19,6 +19,19 @@ class PrefService; // - Whether to expire existing default apps class AppsPromo { public: + // Groups users by whether they have seen a web store promo before. This is + // used for deciding to maximize the promo and apps section on the NTP. + enum UserGroup { + // Matches no users. + USERS_NONE = 0, + + // Users who have not seen a promo (last promo id is default value). + USERS_NEW = 1, + + // Users who have already seen a promo (last promo id is non-default). + USERS_EXISTING = 1 << 1, + }; + // Register our preferences. Parts of the promo content are stored in Local // State since they're independent of the user profile. static void RegisterPrefs(PrefService* local_state); @@ -27,6 +40,12 @@ class AppsPromo { // Removes the current promo data. static void ClearPromo(); + // Returns true if a promo is available for the current locale. + static bool IsPromoSupportedForLocale(); + + // Returns true if the web store is active for the existing locale. + static bool IsWebStoreSupportedForLocale(); + // Gets the ID of the current promo. static std::string GetPromoId(); @@ -45,6 +64,10 @@ class AppsPromo { // Gets the text for the promo "hide this" link. static std::string GetPromoExpireText(); + // Gets the user groups for which we should maximize the promo and apps + // section. The return value is a bitwise OR of UserGroup enums. + static int GetPromoUserGroup(); + // Called to set the current promo data. The default web store logo will be // used if |logo| is empty or not valid. static void SetPromo(const std::string& id, @@ -52,7 +75,12 @@ class AppsPromo { const std::string& button_text, const GURL& link, const std::string& expire_text, - const GURL& logo); + const GURL& logo, + const int user_group); + + // Sets whether the web store and apps section is supported for the current + // locale. + static void SetWebStoreSupportedForLocale(bool supported); explicit AppsPromo(PrefService* prefs); ~AppsPromo(); @@ -71,8 +99,10 @@ class AppsPromo { // Called to hide the promo from the apps section. void HidePromo(); - // Maximizes the apps section the first time this is called for a given promo. - void MaximizeAppsIfFirstView(); + // Maximizes the apps section on the NTP if the following conditions are met: + // (a) the existing promo has not already been maximized + // (b) the current user group is targetted by the promo + void MaximizeAppsIfNecessary(); // Returns true if the app launcher should be displayed on the NTP. bool ShouldShowAppLauncher(const ExtensionIdSet& installed_ids); @@ -91,11 +121,11 @@ class AppsPromo { // between the first time we overflow and subsequent times. static const int kDefaultAppsCounterMax; - // Returns true if a promo is available for the current locale. - static bool IsPromoSupportedForLocale(); - bool GetDefaultAppsInstalled() const; + // Gets the UserGroup classification of the current user. + UserGroup GetCurrentUserGroup() const; + // Gets/sets the ID of the last promo shown. std::string GetLastPromoId(); void SetLastPromoId(const std::string& id); diff --git a/chrome/browser/extensions/apps_promo_unittest.cc b/chrome/browser/extensions/apps_promo_unittest.cc index e808f08..d7cf781 100644 --- a/chrome/browser/extensions/apps_promo_unittest.cc +++ b/chrome/browser/extensions/apps_promo_unittest.cc @@ -21,6 +21,22 @@ const char kPromoButton[] = "Click for apps!"; const char kPromoLink[] = "http://apps.com"; const char kPromoLogo[] = "chrome://theme/IDR_WEBSTORE_ICON"; const char kPromoExpire[] = "No thanks."; +const int kPromoUserGroup = + AppsPromo::USERS_NEW | AppsPromo::USERS_EXISTING; + +void ExpectAppsSectionMaximized(PrefService* prefs, bool maximized) { + EXPECT_EQ(maximized, + (ShownSectionsHandler::GetShownSections(prefs) & APPS) != 0); + EXPECT_EQ(!maximized, + (ShownSectionsHandler::GetShownSections(prefs) & THUMB) != 0); +} + +void ExpectAppsPromoHidden(PrefService* prefs) { + // Hiding the promo places the apps section in menu mode and maximizes the + // most visited section. + EXPECT_TRUE((ShownSectionsHandler::GetShownSections(prefs) & + (MENU_APPS | THUMB)) != 0); +} } // namespace @@ -73,17 +89,31 @@ TEST_F(ExtensionAppsPromo, HappyPath) { &promo_just_expired)); EXPECT_FALSE(promo_just_expired); + // Make sure the web store can be supported even when the promo is not active. + AppsPromo::SetWebStoreSupportedForLocale(true); + EXPECT_FALSE(AppsPromo::IsPromoSupportedForLocale()); + EXPECT_TRUE(apps_promo()->ShouldShowAppLauncher(installed_ids)); + EXPECT_FALSE(apps_promo()->ShouldShowPromo(installed_ids, + &promo_just_expired)); + + // We should be able to disable the web store as well. + AppsPromo::SetWebStoreSupportedForLocale(false); + EXPECT_FALSE(AppsPromo::IsPromoSupportedForLocale()); + EXPECT_FALSE(apps_promo()->ShouldShowAppLauncher(installed_ids)); + EXPECT_FALSE(apps_promo()->ShouldShowPromo(installed_ids, + &promo_just_expired)); + // Once the promo is set, we show both the promo and app launcher. AppsPromo::SetPromo(kPromoId, kPromoHeader, kPromoButton, - GURL(kPromoLink), kPromoExpire, GURL("")); - + GURL(kPromoLink), kPromoExpire, GURL(""), + kPromoUserGroup); + AppsPromo::SetWebStoreSupportedForLocale(true); EXPECT_TRUE(AppsPromo::IsPromoSupportedForLocale()); EXPECT_TRUE(apps_promo()->ShouldShowAppLauncher(installed_ids)); EXPECT_TRUE(apps_promo()->ShouldShowPromo(installed_ids, - &promo_just_expired)); + &promo_just_expired)); EXPECT_FALSE(promo_just_expired); - // Now install an app and the promo should not be shown. installed_ids.insert(*(default_app_ids.begin())); EXPECT_TRUE(AppsPromo::IsPromoSupportedForLocale()); @@ -115,7 +145,8 @@ TEST_F(ExtensionAppsPromo, HappyPath) { TEST_F(ExtensionAppsPromo, PromoPrefs) { // Store a promo.... AppsPromo::SetPromo(kPromoId, kPromoHeader, kPromoButton, - GURL(kPromoLink), kPromoExpire, GURL("")); + GURL(kPromoLink), kPromoExpire, GURL(""), + kPromoUserGroup); // ... then make sure AppsPromo can access it. EXPECT_EQ(kPromoId, AppsPromo::GetPromoId()); @@ -123,6 +154,7 @@ TEST_F(ExtensionAppsPromo, PromoPrefs) { EXPECT_EQ(kPromoButton, AppsPromo::GetPromoButtonText()); EXPECT_EQ(GURL(kPromoLink), AppsPromo::GetPromoLink()); EXPECT_EQ(kPromoExpire, AppsPromo::GetPromoExpireText()); + EXPECT_EQ(kPromoUserGroup, AppsPromo::GetPromoUserGroup()); // The promo logo should be the default value. EXPECT_EQ(GURL(kPromoLogo), AppsPromo::GetPromoLogo()); EXPECT_TRUE(AppsPromo::IsPromoSupportedForLocale()); @@ -133,65 +165,149 @@ TEST_F(ExtensionAppsPromo, PromoPrefs) { EXPECT_EQ("", AppsPromo::GetPromoButtonText()); EXPECT_EQ(GURL(""), AppsPromo::GetPromoLink()); EXPECT_EQ("", AppsPromo::GetPromoExpireText()); + EXPECT_EQ(AppsPromo::USERS_NONE, AppsPromo::GetPromoUserGroup()); EXPECT_EQ(GURL(kPromoLogo), AppsPromo::GetPromoLogo()); EXPECT_FALSE(AppsPromo::IsPromoSupportedForLocale()); // Make sure we can set the logo to something other than the default. std::string promo_logo = "data:image/png;base64,iVBORw0kGgoAAAN"; AppsPromo::SetPromo(kPromoId, kPromoHeader, kPromoButton, - GURL(kPromoLink), kPromoExpire, GURL(promo_logo)); + GURL(kPromoLink), kPromoExpire, GURL(promo_logo), + kPromoUserGroup); EXPECT_EQ(GURL(promo_logo), AppsPromo::GetPromoLogo()); EXPECT_TRUE(AppsPromo::IsPromoSupportedForLocale()); // Verify that the default is returned instead of http or https URLs. promo_logo = "http://google.com/logo.png"; AppsPromo::SetPromo(kPromoId, kPromoHeader, kPromoButton, - GURL(kPromoLink), kPromoExpire, GURL(promo_logo)); + GURL(kPromoLink), kPromoExpire, GURL(promo_logo), + kPromoUserGroup); EXPECT_EQ(GURL(kPromoLogo), AppsPromo::GetPromoLogo()); EXPECT_TRUE(AppsPromo::IsPromoSupportedForLocale()); promo_logo = "https://google.com/logo.png"; AppsPromo::SetPromo(kPromoId, kPromoHeader, kPromoButton, - GURL(kPromoLink), kPromoExpire, GURL(promo_logo)); + GURL(kPromoLink), kPromoExpire, GURL(promo_logo), + kPromoUserGroup); EXPECT_EQ(GURL(kPromoLogo), AppsPromo::GetPromoLogo()); EXPECT_TRUE(AppsPromo::IsPromoSupportedForLocale()); // Try an invalid URL. promo_logo = "sldkfjlsdn"; AppsPromo::SetPromo(kPromoId, kPromoHeader, kPromoButton, - GURL(kPromoLink), kPromoExpire, GURL(promo_logo)); + GURL(kPromoLink), kPromoExpire, GURL(promo_logo), + kPromoUserGroup); EXPECT_EQ(GURL(kPromoLogo), AppsPromo::GetPromoLogo()); EXPECT_TRUE(AppsPromo::IsPromoSupportedForLocale()); + + // Try the web store supported flag. + EXPECT_FALSE(AppsPromo::IsWebStoreSupportedForLocale()); + AppsPromo::SetWebStoreSupportedForLocale(true); + EXPECT_TRUE(AppsPromo::IsWebStoreSupportedForLocale()); + AppsPromo::SetWebStoreSupportedForLocale(false); + EXPECT_FALSE(AppsPromo::IsWebStoreSupportedForLocale()); } -// Tests that the apps section is maxmized when showing a promo for the first -// time. -TEST_F(ExtensionAppsPromo, UpdatePromoFocus) { - ExtensionIdSet installed_ids; +// Tests maximizing the promo for USERS_NONE. +TEST_F(ExtensionAppsPromo, UpdatePromoFocus_UsersNone) { + // Verify that the apps section is not already maximized. + ExpectAppsSectionMaximized(prefs(), false); - bool promo_just_expired = false; - EXPECT_FALSE(apps_promo()->ShouldShowPromo(installed_ids, - &promo_just_expired)); - EXPECT_FALSE(promo_just_expired); + // The promo shouldn't maximize for anyone. + AppsPromo::SetPromo(kPromoId, kPromoHeader, kPromoButton, + GURL(kPromoLink), kPromoExpire, GURL(""), + AppsPromo::USERS_NONE); + apps_promo()->MaximizeAppsIfNecessary(); + ExpectAppsSectionMaximized(prefs(), false); + + // The promo still shouldn't maximize if we change it's ID. + AppsPromo::SetPromo("lkksdf", kPromoHeader, kPromoButton, + GURL(kPromoLink), kPromoExpire, GURL(""), + AppsPromo::USERS_NONE); + apps_promo()->MaximizeAppsIfNecessary(); + ExpectAppsSectionMaximized(prefs(), false); +} + +// Tests maximizing the promo for USERS_EXISTING. +TEST_F(ExtensionAppsPromo, UpdatePromoFocus_UsersExisting) { + // Verify that the apps section is not already maximized. + ExpectAppsSectionMaximized(prefs(), false); // Set the promo content. AppsPromo::SetPromo(kPromoId, kPromoHeader, kPromoButton, - GURL(kPromoLink), kPromoExpire, GURL("")); + GURL(kPromoLink), kPromoExpire, GURL(""), + AppsPromo::USERS_EXISTING); - // After asking if we should show the promo, the - EXPECT_TRUE(AppsPromo::IsPromoSupportedForLocale()); - EXPECT_TRUE(apps_promo()->ShouldShowPromo(installed_ids, - &promo_just_expired)); - apps_promo()->MaximizeAppsIfFirstView(); + // This is a new user so the apps section shouldn't maximize. + apps_promo()->MaximizeAppsIfNecessary(); + ExpectAppsSectionMaximized(prefs(), false); + + + // Set a new promo and now it should maximize. + AppsPromo::SetPromo("lksdf", kPromoHeader, kPromoButton, + GURL(kPromoLink), kPromoExpire, GURL(""), + AppsPromo::USERS_EXISTING); - EXPECT_TRUE( - (ShownSectionsHandler::GetShownSections(prefs()) & APPS) != 0); - EXPECT_FALSE( - (ShownSectionsHandler::GetShownSections(prefs()) & THUMB) != 0); + apps_promo()->MaximizeAppsIfNecessary(); + ExpectAppsSectionMaximized(prefs(), true); apps_promo()->HidePromo(); + ExpectAppsPromoHidden(prefs()); +} - EXPECT_TRUE((ShownSectionsHandler::GetShownSections(prefs()) & - (MENU_APPS | THUMB)) != 0); +// Tests maximizing the promo for USERS_NEW. +TEST_F(ExtensionAppsPromo, UpdatePromoFocus_UsersNew) { + // Verify that the apps section is not already maximized. + ExpectAppsSectionMaximized(prefs(), false); + + // The promo should maximize for new users. + AppsPromo::SetPromo(kPromoId, kPromoHeader, kPromoButton, + GURL(kPromoLink), kPromoExpire, GURL(""), + AppsPromo::USERS_NEW); + apps_promo()->MaximizeAppsIfNecessary(); + ExpectAppsSectionMaximized(prefs(), true); + + // Try hiding the promo. + apps_promo()->HidePromo(); + ExpectAppsPromoHidden(prefs()); + + // The same promo should not maximize twice. + apps_promo()->MaximizeAppsIfNecessary(); + ExpectAppsSectionMaximized(prefs(), false); + + // Another promo targetting new users should not maximize. + AppsPromo::SetPromo("lksdf", kPromoHeader, kPromoButton, + GURL(kPromoLink), kPromoExpire, GURL(""), + AppsPromo::USERS_NEW); + apps_promo()->MaximizeAppsIfNecessary(); + ExpectAppsSectionMaximized(prefs(), false); } + +// Tests maximizing the promo for USERS_NEW | USERS_EXISTING. +TEST_F(ExtensionAppsPromo, UpdatePromoFocus_UsersAll) { + // Verify that the apps section is not already maximized. + ExpectAppsSectionMaximized(prefs(), false); + + // The apps section should maximize for all users. + AppsPromo::SetPromo(kPromoId, kPromoHeader, kPromoButton, + GURL(kPromoLink), kPromoExpire, GURL(""), + AppsPromo::USERS_NEW | AppsPromo::USERS_EXISTING); + apps_promo()->MaximizeAppsIfNecessary(); + ExpectAppsSectionMaximized(prefs(), true); + + apps_promo()->HidePromo(); + ExpectAppsPromoHidden(prefs()); + + // The same promo should not maximize twice. + apps_promo()->MaximizeAppsIfNecessary(); + ExpectAppsSectionMaximized(prefs(), false); + + // A promo with a new ID should maximize though. + AppsPromo::SetPromo("lkksdf", kPromoHeader, kPromoButton, + GURL(kPromoLink), kPromoExpire, GURL(""), + AppsPromo::USERS_NEW | AppsPromo::USERS_EXISTING); + apps_promo()->MaximizeAppsIfNecessary(); + ExpectAppsSectionMaximized(prefs(), true); +} + #endif // OS_CHROMEOS diff --git a/chrome/browser/resources/ntp/apps.js b/chrome/browser/resources/ntp/apps.js index 77b599d..7f4ec13 100644 --- a/chrome/browser/resources/ntp/apps.js +++ b/chrome/browser/resources/ntp/apps.js @@ -77,9 +77,9 @@ function getAppsCallback(data) { document.documentElement.classList.remove('apps-promo-visible'); } - // Only show the web store entry if there are apps installed, since the promo - // is sufficient otherwise. - if (data.apps.length > 0) { + // Only show the web store entry if there are apps installed or the promo + // is not available. + if (data.apps.length > 0 || !data.showPromo) { webStoreEntry = apps.createWebStoreElement(); webStoreEntry.querySelector('a').ping = appsPromoPing; appsSectionContent.appendChild(webStoreEntry); diff --git a/chrome/browser/ui/webui/ntp/app_launcher_handler.cc b/chrome/browser/ui/webui/ntp/app_launcher_handler.cc index b79de70..ecb9bcb 100644 --- a/chrome/browser/ui/webui/ntp/app_launcher_handler.cc +++ b/chrome/browser/ui/webui/ntp/app_launcher_handler.cc @@ -292,8 +292,7 @@ void AppLauncherHandler::HandleGetApps(const ListValue* args) { bool apps_promo_just_expired = false; if (apps_promo->ShouldShowPromo(extensions_service_->GetAppIds(), &apps_promo_just_expired)) { - // Maximize the apps section on the first promo view. - apps_promo->MaximizeAppsIfFirstView(); + apps_promo->MaximizeAppsIfNecessary(); dictionary.SetBoolean("showPromo", true); FillPromoDictionary(&dictionary); promo_active_ = true; diff --git a/chrome/browser/web_resource/promo_resource_service.cc b/chrome/browser/web_resource/promo_resource_service.cc index f66e791..5c55bea 100644 --- a/chrome/browser/web_resource/promo_resource_service.cc +++ b/chrome/browser/web_resource/promo_resource_service.cc @@ -303,6 +303,7 @@ void PromoResourceService::UnpackWebStoreSignal( DictionaryValue* topic_dict; ListValue* answer_list; + bool is_webstore_active = false; bool signal_found = false; std::string promo_id = ""; std::string promo_header = ""; @@ -310,6 +311,7 @@ void PromoResourceService::UnpackWebStoreSignal( std::string promo_link = ""; std::string promo_expire = ""; std::string promo_logo = ""; + int maximize_setting = 0; int target_builds = 0; if (!parsed_json.GetDictionary("topic", &topic_dict) || @@ -334,6 +336,10 @@ void PromoResourceService::UnpackWebStoreSignal( if (split == std::string::npos || name.substr(0, split) != "webstore_promo") continue; + // If the "webstore_promo" string was found, that's enough to activate the + // apps section even if the rest of the promo fails parsing. + is_webstore_active = true; + // (2) an integer specifying which builds the promo targets name = name.substr(split+1); split = name.find(':'); @@ -341,7 +347,14 @@ void PromoResourceService::UnpackWebStoreSignal( !base::StringToInt(name.substr(0, split), &target_builds)) continue; - // (3) optional text that specifies a URL of a logo image + // (3) an integer specifying what users should maximize the promo + name = name.substr(split+1); + split = name.find(':'); + if (split == std::string::npos || + !base::StringToInt(name.substr(0, split), &maximize_setting)) + continue; + + // (4) optional text that specifies a URL of a logo image promo_logo = name.substr(split+1); if (!a_dic->GetString(kAnswerIdProperty, &promo_id) || @@ -354,7 +367,8 @@ void PromoResourceService::UnpackWebStoreSignal( if (IsThisBuildTargeted(target_builds)) { // Store the first web store promo that targets the current build. AppsPromo::SetPromo(promo_id, promo_header, promo_button, - GURL(promo_link), promo_expire, GURL(promo_logo)); + GURL(promo_link), promo_expire, GURL(promo_logo), + maximize_setting); signal_found = true; break; } @@ -365,6 +379,8 @@ void PromoResourceService::UnpackWebStoreSignal( AppsPromo::ClearPromo(); } + AppsPromo::SetWebStoreSupportedForLocale(is_webstore_active); + NotificationService::current()->Notify( NotificationType::WEB_STORE_PROMO_LOADED, Source<PromoResourceService>(this), diff --git a/chrome/browser/web_resource/promo_resource_service.h b/chrome/browser/web_resource/promo_resource_service.h index ad1af83..469abf0 100644 --- a/chrome/browser/web_resource/promo_resource_service.h +++ b/chrome/browser/web_resource/promo_resource_service.h @@ -48,6 +48,8 @@ class PromoResourceService FRIEND_TEST_ALL_PREFIXES(PromoResourceServiceTest, UnpackLogoSignal); FRIEND_TEST_ALL_PREFIXES(PromoResourceServiceTest, UnpackPromoSignal); FRIEND_TEST_ALL_PREFIXES(PromoResourceServiceTest, UnpackWebStoreSignal); + FRIEND_TEST_ALL_PREFIXES( + PromoResourceServiceTest, UnpackPartialWebStoreSignal); // Identifies types of Chrome builds for promo targeting. enum BuildType { diff --git a/chrome/browser/web_resource/promo_resource_service_unittest.cc b/chrome/browser/web_resource/promo_resource_service_unittest.cc index cd6f141..218af2f 100644 --- a/chrome/browser/web_resource/promo_resource_service_unittest.cc +++ b/chrome/browser/web_resource/promo_resource_service_unittest.cc @@ -169,14 +169,12 @@ TEST_F(PromoResourceServiceTest, UnpackPromoSignal) { TEST_F(PromoResourceServiceTest, UnpackWebStoreSignal) { web_resource_service_->set_channel(platform_util::CHANNEL_DEV); - // Set up start and end dates and promo line in a Dictionary as if parsed - // from the service. std::string json = "{ " " \"topic\": {" " \"answers\": [" " {" " \"answer_id\": \"341252\"," - " \"name\": \"webstore_promo:15:\"," + " \"name\": \"webstore_promo:15:1:\"," " \"question\": \"The header!\"," " \"inproduct_target\": \"The button label!\"," " \"inproduct\": \"http://link.com\"," @@ -201,10 +199,36 @@ TEST_F(PromoResourceServiceTest, UnpackWebStoreSignal) { EXPECT_EQ("The button label!", AppsPromo::GetPromoButtonText()); EXPECT_EQ(GURL("http://link.com"), AppsPromo::GetPromoLink()); EXPECT_EQ("No thanks, hide this.", AppsPromo::GetPromoExpireText()); + EXPECT_EQ(AppsPromo::USERS_NEW, AppsPromo::GetPromoUserGroup()); EXPECT_EQ(GURL("chrome://theme/IDR_WEBSTORE_ICON"), AppsPromo::GetPromoLogo()); } +// Tests that the "web store active" flag is set even when the web store promo +// fails parsing. +TEST_F(PromoResourceServiceTest, UnpackPartialWebStoreSignal) { + std::string json = "{ " + " \"topic\": {" + " \"answers\": [" + " {" + " \"answer_id\": \"sdlfj32\"," + " \"name\": \"webstore_promo:#klsdjlfSD\"" + " }" + " ]" + " }" + "}"; + scoped_ptr<DictionaryValue> test_json(static_cast<DictionaryValue*>( + base::JSONReader::Read(json, false))); + + // Initialize a message loop for this to run on. + MessageLoop loop; + + // Check that prefs are set correctly. + web_resource_service_->UnpackWebStoreSignal(*(test_json.get())); + EXPECT_FALSE(AppsPromo::IsPromoSupportedForLocale()); + EXPECT_TRUE(AppsPromo::IsWebStoreSupportedForLocale()); +} + TEST_F(PromoResourceServiceTest, IsBuildTargeted) { // canary const platform_util::Channel canary = platform_util::CHANNEL_CANARY; diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc index a063f40..80a07af 100644 --- a/chrome/common/pref_names.cc +++ b/chrome/common/pref_names.cc @@ -1132,6 +1132,9 @@ const char kNTPPromoLine[] = "ntp.promo_line"; const char kNTPPromoStart[] = "ntp.promo_start"; const char kNTPPromoEnd[] = "ntp.promo_end"; +// Boolean indicating whether the web store is active for the current locale. +const char kNTPWebStoreEnabled[] = "ntp.webstore_enabled"; + // The id of the last web store promo actually displayed on the NTP. const char kNTPWebStorePromoLastId[] = "ntp.webstore_last_promo_id"; @@ -1153,6 +1156,9 @@ const char kNTPWebStorePromoLogo[] = "ntp.webstorepromo.logo"; // The "hide this" link text for the NTP web store promo. const char kNTPWebStorePromoExpire[] = "ntp.webstorepromo.expire"; +// Specifies what users should maximize the NTP web store promo. +const char kNTPWebStorePromoUserGroup[] = "ntp.webstorepromo.usergroup"; + // The most up-to-date GPU blacklist downloaded from the web, which replaces // the one that's installed with chrome. const char kGpuBlacklist[] = "gpu_blacklist"; diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h index be072f9..4182c7b 100644 --- a/chrome/common/pref_names.h +++ b/chrome/common/pref_names.h @@ -398,6 +398,7 @@ extern const char kNTPPromoClosed[]; extern const char kNTPPromoGroup[]; extern const char kNTPPromoGroupTimeSlice[]; extern const char kNTPPromoBuild[]; +extern const char kNTPWebStoreEnabled[]; extern const char kNTPWebStorePromoLastId[]; extern const char kNTPWebStorePromoId[]; extern const char kNTPWebStorePromoHeader[]; @@ -405,6 +406,7 @@ extern const char kNTPWebStorePromoButton[]; extern const char kNTPWebStorePromoLink[]; extern const char kNTPWebStorePromoLogo[]; extern const char kNTPWebStorePromoExpire[]; +extern const char kNTPWebStorePromoUserGroup[]; extern const char kGpuBlacklist[]; extern const char kGpuBlacklistUpdate[]; |