Skip to content

GitLab

T
tangled
Commit bb7292fb authored by Reid Kleckner's avatar Reid Kleckner Committed by Commit Bot
Browse files
Options

Revert "[NTP] Update NTP to respect the HideWebStoreIcon policy" 

This reverts commit 9a40eecd.

Reason for revert: Test fails in official builds.

Original change's description:
> [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/1137432
> Reviewed-by: Scott Violet <sky@chromium.org>
> Commit-Queue: Weilun Shi <sweilun@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#580699}

TBR=sky@chromium.org,ramyan@chromium.org,sweilun@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 855603, 871166
Change-Id: If88116adf0fdd613ec5c2ede8abeb3c8b6b6207c
Reviewed-on: https://chromium-review.googlesource.com/1164205Reviewed-by: default avatarReid Kleckner <rnk@chromium.org>
Commit-Queue: Reid Kleckner <rnk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581064}
parent 787c45cf
Hide whitespace changes
Inline Side-by-side
Showing with 7 additions and 90 deletions
chrome/browser/history/top_sites_factory.cc
View file @ bb7292fb
......@@ -21,7 +21,6 @@
#include "chrome/browser/history/history_utils.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/common/chrome_features.h"
#include "chrome/common/pref_names.h"
#include "chrome/grit/chromium_strings.h"
#include "chrome/grit/generated_resources.h"
#include "chrome/grit/locale_settings.h"
......@@ -33,7 +32,6 @@
#include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/ntp_tiles/constants.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/pref_service.h"
#include "content/public/browser/browser_thread.h"
#include "ui/base/l10n/l10n_util.h"
#include "url/gurl.h"
......@@ -68,16 +66,12 @@ const RawPrepopulatedPage kRawPrepopulatedPages[] = {
#endif
void InitializePrepopulatedPageList(
PrefService* prefs,
history::PrepopulatedPageList* prepopulated_pages) {
#if !defined(OS_ANDROID)
DCHECK(prepopulated_pages);
bool hide_web_store_icon = prefs->GetBoolean(prefs::kHideWebStoreIcon);
prepopulated_pages->reserve(arraysize(kRawPrepopulatedPages));
for (size_t i = 0; i < arraysize(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(
GURL(l10n_util::GetStringUTF8(page.url_id)),
l10n_util::GetStringUTF16(page.title_id),
......@@ -122,6 +116,7 @@ scoped_refptr<history::TopSites> TopSitesFactory::BuildTopSites(
history::HistoryService* history_service =
HistoryServiceFactory::GetForProfile(profile,
ServiceAccessType::EXPLICIT_ACCESS);
scoped_refptr<history::TopSitesImpl> top_sites(new history::TopSitesImpl(
profile->GetPrefs(), history_service,
CreateTopSitesProvider(profile, history_service), prepopulated_page_list,
......@@ -147,8 +142,7 @@ TopSitesFactory::~TopSitesFactory() {
scoped_refptr<RefcountedKeyedService> TopSitesFactory::BuildServiceInstanceFor(
content::BrowserContext* context) const {
history::PrepopulatedPageList prepopulated_pages;
InitializePrepopulatedPageList(
Profile::FromBrowserContext(context)->GetPrefs(), &prepopulated_pages);
InitializePrepopulatedPageList(&prepopulated_pages);
return BuildTopSites(context, prepopulated_pages);
}
......
chrome/browser/policy/policy_browsertest.cc
View file @ bb7292fb
......@@ -98,8 +98,6 @@
#include "chrome/browser/ui/browser_window.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/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/toolbar/component_toolbar_actions_factory.h"
#include "chrome/browser/ui/toolbar/media_router_action_controller.h"
......@@ -116,9 +114,7 @@
#include "chrome/common/pref_names.h"
#include "chrome/common/url_constants.h"
#include "chrome/common/web_application_info.h"
#include "chrome/common/webui_url_constants.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/search_test_utils.h"
#include "chrome/test/base/testing_profile.h"
......@@ -358,14 +354,6 @@ void GetTestDataDirectory(base::FilePath* 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
// dir through URLRequestMockHTTPJobs.
void RedirectHostsToTestData(const char* const urls[], size_t size) {
......@@ -611,16 +599,6 @@ bool ContainsVisibleElement(content::WebContents* contents,
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)
class TestAudioObserver : public chromeos::CrasAudioHandler::AudioObserver {
public:
......@@ -2060,30 +2038,12 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, DeveloperToolsDisabledExtensionsDevMode) {
EXPECT_TRUE(is_toggle_dev_mode_checkbox_disabled);
}
IN_PROC_BROWSER_TEST_F(PolicyTest, NTPWebStoreIconShown) {
// This test is to verify that the web store icons is shown when no policy
// applies. See WebStoreIconPolicyTest.NTPWebStoreIconHidden for verification
// when a policy is in effect.
// Open new tab page and look for the web store icons.
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.
// TODO(samarth): remove along with rest of NTP4 code.
IN_PROC_BROWSER_TEST_F(PolicyTest, DISABLED_WebStoreIconHidden) {
// Verifies that the web store icons can be hidden from the new tab page.
// Open new tab page and look for the web store icons.
ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUIAppsURL));
ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUINewTabURL));
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
......@@ -2105,7 +2065,7 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, AppsWebStoreIconHidden) {
UpdateProviderPolicy(policies);
// The web store icons should now be hidden.
ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUIAppsURL));
ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUINewTabURL));
EXPECT_FALSE(ContainsVisibleElement(contents,
"ahfgeienlihckogmohjhadlkjgocpleb"));
EXPECT_FALSE(ContainsVisibleElement(contents, "chrome-web-store-link"));
......@@ -4103,43 +4063,6 @@ IN_PROC_BROWSER_TEST_F(PolicyStatisticsCollectorTest, Startup) {
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
: public PolicyTest,
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