Commit 9a40eecd authored by Weilun Shi's avatar Weilun Shi Committed by Commit Bot

[NTP] Update NTP to respect the HideWebStoreIcon policy

Now, changing the HideWebStoreIcon policy will affect the
prepopulated_list when a new profile opens or reopen the browser. The
policy doesn't affect current profile on the NTP. This is because the
browser will cache the prepopulated list so that it can fetch the tiles
quickly.

Bug: 855603
Change-Id: I99ac05040fc44f64ac61151218dcbcc612465e72
Reviewed-on: https://chromium-review.googlesource.com/1137432Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Weilun Shi <sweilun@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580699}
parent 22af4282
...@@ -21,6 +21,7 @@ ...@@ -21,6 +21,7 @@
#include "chrome/browser/history/history_utils.h" #include "chrome/browser/history/history_utils.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/common/chrome_features.h" #include "chrome/common/chrome_features.h"
#include "chrome/common/pref_names.h"
#include "chrome/grit/chromium_strings.h" #include "chrome/grit/chromium_strings.h"
#include "chrome/grit/generated_resources.h" #include "chrome/grit/generated_resources.h"
#include "chrome/grit/locale_settings.h" #include "chrome/grit/locale_settings.h"
...@@ -32,6 +33,7 @@ ...@@ -32,6 +33,7 @@
#include "components/keyed_service/content/browser_context_dependency_manager.h" #include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/ntp_tiles/constants.h" #include "components/ntp_tiles/constants.h"
#include "components/pref_registry/pref_registry_syncable.h" #include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/pref_service.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -66,12 +68,16 @@ const RawPrepopulatedPage kRawPrepopulatedPages[] = { ...@@ -66,12 +68,16 @@ const RawPrepopulatedPage kRawPrepopulatedPages[] = {
#endif #endif
void InitializePrepopulatedPageList( void InitializePrepopulatedPageList(
PrefService* prefs,
history::PrepopulatedPageList* prepopulated_pages) { history::PrepopulatedPageList* prepopulated_pages) {
#if !defined(OS_ANDROID) #if !defined(OS_ANDROID)
DCHECK(prepopulated_pages); DCHECK(prepopulated_pages);
bool hide_web_store_icon = prefs->GetBoolean(prefs::kHideWebStoreIcon);
prepopulated_pages->reserve(arraysize(kRawPrepopulatedPages)); prepopulated_pages->reserve(arraysize(kRawPrepopulatedPages));
for (size_t i = 0; i < arraysize(kRawPrepopulatedPages); ++i) { for (size_t i = 0; i < arraysize(kRawPrepopulatedPages); ++i) {
const RawPrepopulatedPage& page = kRawPrepopulatedPages[i]; const RawPrepopulatedPage& page = kRawPrepopulatedPages[i];
if (hide_web_store_icon && page.url_id == IDS_WEBSTORE_URL)
continue;
prepopulated_pages->push_back(history::PrepopulatedPage( prepopulated_pages->push_back(history::PrepopulatedPage(
GURL(l10n_util::GetStringUTF8(page.url_id)), GURL(l10n_util::GetStringUTF8(page.url_id)),
l10n_util::GetStringUTF16(page.title_id), l10n_util::GetStringUTF16(page.title_id),
...@@ -116,7 +122,6 @@ scoped_refptr<history::TopSites> TopSitesFactory::BuildTopSites( ...@@ -116,7 +122,6 @@ scoped_refptr<history::TopSites> TopSitesFactory::BuildTopSites(
history::HistoryService* history_service = history::HistoryService* history_service =
HistoryServiceFactory::GetForProfile(profile, HistoryServiceFactory::GetForProfile(profile,
ServiceAccessType::EXPLICIT_ACCESS); ServiceAccessType::EXPLICIT_ACCESS);
scoped_refptr<history::TopSitesImpl> top_sites(new history::TopSitesImpl( scoped_refptr<history::TopSitesImpl> top_sites(new history::TopSitesImpl(
profile->GetPrefs(), history_service, profile->GetPrefs(), history_service,
CreateTopSitesProvider(profile, history_service), prepopulated_page_list, CreateTopSitesProvider(profile, history_service), prepopulated_page_list,
...@@ -142,7 +147,8 @@ TopSitesFactory::~TopSitesFactory() { ...@@ -142,7 +147,8 @@ TopSitesFactory::~TopSitesFactory() {
scoped_refptr<RefcountedKeyedService> TopSitesFactory::BuildServiceInstanceFor( scoped_refptr<RefcountedKeyedService> TopSitesFactory::BuildServiceInstanceFor(
content::BrowserContext* context) const { content::BrowserContext* context) const {
history::PrepopulatedPageList prepopulated_pages; history::PrepopulatedPageList prepopulated_pages;
InitializePrepopulatedPageList(&prepopulated_pages); InitializePrepopulatedPageList(
Profile::FromBrowserContext(context)->GetPrefs(), &prepopulated_pages);
return BuildTopSites(context, prepopulated_pages); return BuildTopSites(context, prepopulated_pages);
} }
......
...@@ -97,6 +97,8 @@ ...@@ -97,6 +97,8 @@
#include "chrome/browser/ui/browser_window.h" #include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/location_bar/location_bar.h" #include "chrome/browser/ui/location_bar/location_bar.h"
#include "chrome/browser/ui/permission_bubble/mock_permission_prompt_factory.h" #include "chrome/browser/ui/permission_bubble/mock_permission_prompt_factory.h"
#include "chrome/browser/ui/search/instant_test_utils.h"
#include "chrome/browser/ui/search/local_ntp_test_utils.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/browser/ui/toolbar/component_toolbar_actions_factory.h" #include "chrome/browser/ui/toolbar/component_toolbar_actions_factory.h"
#include "chrome/browser/ui/toolbar/media_router_action_controller.h" #include "chrome/browser/ui/toolbar/media_router_action_controller.h"
...@@ -113,7 +115,9 @@ ...@@ -113,7 +115,9 @@
#include "chrome/common/pref_names.h" #include "chrome/common/pref_names.h"
#include "chrome/common/url_constants.h" #include "chrome/common/url_constants.h"
#include "chrome/common/web_application_info.h" #include "chrome/common/web_application_info.h"
#include "chrome/common/webui_url_constants.h"
#include "chrome/grit/generated_resources.h" #include "chrome/grit/generated_resources.h"
#include "chrome/grit/locale_settings.h"
#include "chrome/test/base/in_process_browser_test.h" #include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/search_test_utils.h" #include "chrome/test/base/search_test_utils.h"
#include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile.h"
...@@ -353,6 +357,14 @@ void GetTestDataDirectory(base::FilePath* test_data_directory) { ...@@ -353,6 +357,14 @@ void GetTestDataDirectory(base::FilePath* test_data_directory) {
base::PathService::Get(chrome::DIR_TEST_DATA, test_data_directory)); base::PathService::Get(chrome::DIR_TEST_DATA, test_data_directory));
} }
content::RenderFrameHost* GetMostVisitedIframe(content::WebContents* tab) {
for (content::RenderFrameHost* frame : tab->GetAllFrames()) {
if (frame->GetFrameName() == "mv-single")
return frame;
}
return nullptr;
}
// Filters requests to the hosts in |urls| and redirects them to the test data // Filters requests to the hosts in |urls| and redirects them to the test data
// dir through URLRequestMockHTTPJobs. // dir through URLRequestMockHTTPJobs.
void RedirectHostsToTestData(const char* const urls[], size_t size) { void RedirectHostsToTestData(const char* const urls[], size_t size) {
...@@ -598,6 +610,16 @@ bool ContainsVisibleElement(content::WebContents* contents, ...@@ -598,6 +610,16 @@ bool ContainsVisibleElement(content::WebContents* contents,
return result; return result;
} }
bool ContainsWebstoreTile(content::RenderFrameHost* iframe) {
int num_webstore_tiles = 0;
EXPECT_TRUE(instant_test_utils::GetIntFromJS(
iframe,
"document.querySelectorAll(\".md-tile[href='" +
l10n_util::GetStringUTF8(IDS_WEBSTORE_URL) + "']\").length",
&num_webstore_tiles));
return num_webstore_tiles == 1;
}
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
class TestAudioObserver : public chromeos::CrasAudioHandler::AudioObserver { class TestAudioObserver : public chromeos::CrasAudioHandler::AudioObserver {
public: public:
...@@ -2037,12 +2059,30 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, DeveloperToolsDisabledExtensionsDevMode) { ...@@ -2037,12 +2059,30 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, DeveloperToolsDisabledExtensionsDevMode) {
EXPECT_TRUE(is_toggle_dev_mode_checkbox_disabled); EXPECT_TRUE(is_toggle_dev_mode_checkbox_disabled);
} }
// TODO(samarth): remove along with rest of NTP4 code. IN_PROC_BROWSER_TEST_F(PolicyTest, NTPWebStoreIconShown) {
IN_PROC_BROWSER_TEST_F(PolicyTest, DISABLED_WebStoreIconHidden) { // This test is to verify that the web store icons is shown when no policy
// Verifies that the web store icons can be hidden from the new tab page. // applies. See WebStoreIconPolicyTest.NTPWebStoreIconHidden for verification
// when a policy is in effect.
// Open new tab page and look for the web store icons. // Open new tab page and look for the web store icons.
ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUINewTabURL)); content::WebContents* active_tab =
local_ntp_test_utils::OpenNewTab(browser(), GURL("about:blank"));
local_ntp_test_utils::NavigateToNTPAndWaitUntilLoaded(browser());
content::RenderFrameHost* iframe = GetMostVisitedIframe(active_tab);
// Look though all the tiles and see whether there is a webstore icon.
// Make sure that there is one web store icon.
EXPECT_TRUE(ContainsWebstoreTile(iframe));
}
IN_PROC_BROWSER_TEST_F(PolicyTest, AppsWebStoreIconHidden) {
// Verifies that the web store icon can be hidden from the chrome://apps
// page. A policy change takes immediate effect on the apps page for the
// current profile. Browser restart is not required.
// Open new tab page and look for the web store icons.
ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUIAppsURL));
content::WebContents* contents = content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents(); browser()->tab_strip_model()->GetActiveWebContents();
...@@ -2064,7 +2104,7 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, DISABLED_WebStoreIconHidden) { ...@@ -2064,7 +2104,7 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, DISABLED_WebStoreIconHidden) {
UpdateProviderPolicy(policies); UpdateProviderPolicy(policies);
// The web store icons should now be hidden. // The web store icons should now be hidden.
ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUINewTabURL)); ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUIAppsURL));
EXPECT_FALSE(ContainsVisibleElement(contents, EXPECT_FALSE(ContainsVisibleElement(contents,
"ahfgeienlihckogmohjhadlkjgocpleb")); "ahfgeienlihckogmohjhadlkjgocpleb"));
EXPECT_FALSE(ContainsVisibleElement(contents, "chrome-web-store-link")); EXPECT_FALSE(ContainsVisibleElement(contents, "chrome-web-store-link"));
...@@ -4062,6 +4102,43 @@ IN_PROC_BROWSER_TEST_F(PolicyStatisticsCollectorTest, Startup) { ...@@ -4062,6 +4102,43 @@ IN_PROC_BROWSER_TEST_F(PolicyStatisticsCollectorTest, Startup) {
EXPECT_GT(samples->GetCount(82), 0); EXPECT_GT(samples->GetCount(82), 0);
} }
// Similar to PolicyTest, but applies the HideWebStoreIcon policy before
// the browser is started. This is required because the list that includes the
// WebStoreIcon on the NTP is initialized at browser start.
class PolicyWebStoreIconTest : public PolicyTest {
public:
PolicyWebStoreIconTest() {}
~PolicyWebStoreIconTest() override {}
void SetUpInProcessBrowserTestFixture() override {
PolicyTest::SetUpInProcessBrowserTestFixture();
PolicyMap policies;
policies.Set(key::kHideWebStoreIcon, POLICY_LEVEL_MANDATORY,
POLICY_SCOPE_USER, POLICY_SOURCE_CLOUD,
std::make_unique<base::Value>(true), nullptr);
provider_.UpdateChromePolicy(policies);
}
private:
DISALLOW_COPY_AND_ASSIGN(PolicyWebStoreIconTest);
};
IN_PROC_BROWSER_TEST_F(PolicyWebStoreIconTest, NTPWebStoreIconHidden) {
// Verifies that the web store icon can be hidden from the new tab page. Check
// to see NTPWebStoreIconShown for behavior when the policy is not applied.
// Open new tab page and look for the web store icon
content::WebContents* active_tab =
local_ntp_test_utils::OpenNewTab(browser(), GURL("about:blank"));
local_ntp_test_utils::NavigateToNTPAndWaitUntilLoaded(browser());
content::RenderFrameHost* iframe = GetMostVisitedIframe(active_tab);
// Applying the policy before the browser started, the web store icon should
// now be hidden.
EXPECT_FALSE(ContainsWebstoreTile(iframe));
}
class MediaStreamDevicesControllerBrowserTest class MediaStreamDevicesControllerBrowserTest
: public PolicyTest, : public PolicyTest,
public testing::WithParamInterface<bool> { public testing::WithParamInterface<bool> {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment