Skip to content

Commit

Permalink
Reload Local NTP on default search provider change.
Browse files Browse the repository at this point in the history 
Based on the default search provider setting, LocalNTPSource will set the local NTP JS state to render Google logo and fakebox in local NTP. We no longer append "?isGoogle" in local NTP url if the search provider is Google. Local NTP url will remain the same irrespective of the default search provider setting.

BUG=222183,235282
TEST=none

Review URL: https://chromiumcodereview.appspot.com/19054012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@212160 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
kmadhusu@chromium.org committed Jul 17, 2013
1 parent 380de76 commit 5df96ab
Show file tree
Hide file tree
Showing 17 changed files with 211 additions and 154 deletions.
2 changes: 1 addition & 1 deletion chrome/browser/resources/local_ntp/local_ntp.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
Use of this source code is governed by a BSD-style license that can be
found in the LICENSE file. -->
<head>
<script src="chrome-search://local-ntp/translated-strings.js"></script>
<script src="chrome-search://local-ntp/config.js"></script>
<script src="chrome-search://local-ntp/local-ntp.js"></script>
<link rel="stylesheet" href="chrome-search://local-ntp/local-ntp.css"></link>
<meta name="google" value="notranslate">
Expand Down
32 changes: 12 additions & 20 deletions chrome/browser/resources/local_ntp/local_ntp.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,9 @@

/**
* Controls rendering the new tab page for InstantExtended.
* @param {Object} location window.location or a mock.
* @return {Object} A limited interface for testing the local NTP.
*/
function LocalNTP(location) {
function LocalNTP() {
<include src="../../../../ui/webui/resources/js/assert.js">


Expand Down Expand Up @@ -616,7 +615,7 @@ function createTile(page, position) {
tileElement, 'div', CLASSES.BLACKLIST_BUTTON);
var blacklistFunction = generateBlacklistFunction(rid);
blacklistButton.addEventListener('click', blacklistFunction);
blacklistButton.title = templateData.removeThumbnailTooltip;
blacklistButton.title = configData.translatedStrings.removeThumbnailTooltip;

// When a tile is focused, have delete also blacklist the page.
registerKeyHandler(tileElement, KEYCODE.DELETE, blacklistFunction);
Expand Down Expand Up @@ -918,16 +917,6 @@ function getEmbeddedSearchApiHandle() {
return null;
}


/**
* @return {boolean} True if this is a Google page and not some other search
* provider. Used to determine whether to show the logo and fakebox.
*/
function isGooglePage() {
return location.href.indexOf('isGoogle') != -1;
}


/**
* Extract the desired navigation behavior from a click button.
* @param {number} button The Event#button property of a click event.
Expand Down Expand Up @@ -958,7 +947,7 @@ function init() {
tilesContainer.appendChild(row);
}

if (isGooglePage()) {
if (configData.isGooglePage) {
var logo = document.createElement('div');
logo.id = IDS.LOGO;

Expand All @@ -974,7 +963,7 @@ function init() {
document.body.classList.add(CLASSES.NON_GOOGLE_PAGE);
}

var recentTabsText = templateData.recentTabs;
var recentTabsText = configData.translatedStrings.recentTabs;
if (recentTabsText) {
var recentTabsLink = document.createElement('span');
recentTabsLink.id = IDS.RECENT_TABS;
Expand All @@ -989,16 +978,19 @@ function init() {
}

var notificationMessage = $(IDS.NOTIFICATION_MESSAGE);
notificationMessage.textContent = templateData.thumbnailRemovedNotification;
notificationMessage.textContent =
configData.translatedStrings.thumbnailRemovedNotification;
var undoLink = $(IDS.UNDO_LINK);
undoLink.addEventListener('click', onUndo);
registerKeyHandler(undoLink, KEYCODE.ENTER, onUndo);
undoLink.textContent = templateData.undoThumbnailRemove;
undoLink.textContent = configData.translatedStrings.undoThumbnailRemove;
var restoreAllLink = $(IDS.RESTORE_ALL_LINK);
restoreAllLink.addEventListener('click', onRestoreAll);
registerKeyHandler(restoreAllLink, KEYCODE.ENTER, onUndo);
restoreAllLink.textContent = templateData.restoreThumbnailsShort;
$(IDS.ATTRIBUTION_TEXT).textContent = templateData.attributionIntro;
restoreAllLink.textContent =
configData.translatedStrings.restoreThumbnailsShort;
$(IDS.ATTRIBUTION_TEXT).textContent =
configData.translatedStrings.attributionIntro;

var notificationCloseButton = $(IDS.NOTIFICATION_CLOSE_BUTTON);
notificationCloseButton.addEventListener('click', hideNotification);
Expand Down Expand Up @@ -1060,5 +1052,5 @@ return {
}

if (!window.localNTPUnitTest) {
LocalNTP(location).listen();
LocalNTP().listen();
}
2 changes: 1 addition & 1 deletion chrome/browser/search/instant_service.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ InstantService::InstantService(Profile* profile)
content::URLDataSource::Add(profile, new ThumbnailSource(profile));
content::URLDataSource::Add(profile, new FaviconSource(
profile, FaviconSource::FAVICON));
content::URLDataSource::Add(profile, new LocalNtpSource());
content::URLDataSource::Add(profile, new LocalNtpSource(profile));
content::URLDataSource::Add(profile, new MostVisitedIframeSource());
}

Expand Down
80 changes: 61 additions & 19 deletions chrome/browser/search/local_ntp_source.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,18 @@

#include "chrome/browser/search/local_ntp_source.h"

#include "base/json/json_string_value_serializer.h"
#include "base/logging.h"
#include "base/memory/ref_counted_memory.h"
#include "base/memory/scoped_ptr.h"
#include "base/strings/string_util.h"
#include "base/strings/stringprintf.h"
#include "base/values.h"
#include "chrome/browser/search/instant_io_context.h"
#include "chrome/browser/search/search.h"
#include "chrome/browser/search_engines/template_url_prepopulate_data.h"
#include "chrome/browser/search_engines/template_url_service.h"
#include "chrome/browser/search_engines/template_url_service_factory.h"
#include "chrome/common/url_constants.h"
#include "grit/browser_resources.h"
#include "grit/generated_resources.h"
Expand All @@ -26,7 +31,7 @@ namespace {
// Signifies a locally constructed resource, i.e. not from grit/.
const int kLocalResource = -1;

const char kTranslatedStringsFilename[] = "translated-strings.js";
const char kConfigDataFilename[] = "config.js";

const struct Resource{
const char* filename;
Expand All @@ -35,7 +40,7 @@ const struct Resource{
} kResources[] = {
{ "local-ntp.html", IDR_LOCAL_NTP_HTML, "text/html" },
{ "local-ntp.js", IDR_LOCAL_NTP_JS, "application/javascript" },
{ kTranslatedStringsFilename, kLocalResource, "application/javascript" },
{ kConfigDataFilename, kLocalResource, "application/javascript" },
{ "local-ntp.css", IDR_LOCAL_NTP_CSS, "text/css" },
{ "images/close_2.png", IDR_CLOSE_2, "image/png" },
{ "images/close_2_hover.png", IDR_CLOSE_2_H, "image/png" },
Expand All @@ -60,37 +65,74 @@ std::string StripParameters(const std::string& path) {
return path.substr(0, path.find("?"));
}

bool DefaultSearchProviderIsGoogle(Profile* profile) {
if (!profile)
return false;

TemplateURLService* template_url_service =
TemplateURLServiceFactory::GetForProfile(profile);
if (!template_url_service)
return false;

const TemplateURL* default_provider =
template_url_service->GetDefaultSearchProvider();
return default_provider &&
(TemplateURLPrepopulateData::GetEngineType(default_provider->url()) ==
SEARCH_ENGINE_GOOGLE);
}

// Adds a localized string keyed by resource id to the dictionary.
void AddString(base::DictionaryValue* dictionary,
const std::string& key,
int resource_id) {
dictionary->SetString(key, l10n_util::GetStringUTF16(resource_id));
}

// Returns a JS dictionary of translated strings for the local NTP.
std::string GetTranslatedStrings() {
base::DictionaryValue translated_strings;
// Populates |translated_strings| dictionary for the local NTP.
scoped_ptr<DictionaryValue> GetTranslatedStrings() {
scoped_ptr<base::DictionaryValue> translated_strings(
new base::DictionaryValue());

if (chrome::ShouldShowRecentTabsOnNTP())
AddString(&translated_strings, "recentTabs", IDS_RECENT_TABS_MENU);
AddString(&translated_strings, "thumbnailRemovedNotification",
AddString(translated_strings.get(), "recentTabs", IDS_RECENT_TABS_MENU);

AddString(translated_strings.get(), "thumbnailRemovedNotification",
IDS_NEW_TAB_THUMBNAIL_REMOVED_NOTIFICATION);
AddString(&translated_strings, "removeThumbnailTooltip",
AddString(translated_strings.get(), "removeThumbnailTooltip",
IDS_NEW_TAB_REMOVE_THUMBNAIL_TOOLTIP);
AddString(&translated_strings, "undoThumbnailRemove",
AddString(translated_strings.get(), "undoThumbnailRemove",
IDS_NEW_TAB_UNDO_THUMBNAIL_REMOVE);
AddString(&translated_strings, "restoreThumbnailsShort",
AddString(translated_strings.get(), "restoreThumbnailsShort",
IDS_NEW_TAB_RESTORE_THUMBNAILS_SHORT_LINK);
AddString(&translated_strings, "attributionIntro",
AddString(translated_strings.get(), "attributionIntro",
IDS_NEW_TAB_ATTRIBUTION_INTRO);
AddString(&translated_strings, "title", IDS_NEW_TAB_TITLE);
std::string translated_strings_js;
webui::AppendJsonJS(&translated_strings, &translated_strings_js);
return translated_strings_js;
AddString(translated_strings.get(), "title", IDS_NEW_TAB_TITLE);

return translated_strings.Pass();
}

// Returns a JS dictionary of configuration data for the local NTP.
std::string GetConfigData(Profile* profile) {
base::DictionaryValue config_data;
config_data.Set("translatedStrings", GetTranslatedStrings().release());
config_data.SetBoolean("isGooglePage",
DefaultSearchProviderIsGoogle(profile));

// Serialize the dictionary.
std::string js_text;
JSONStringValueSerializer serializer(&js_text);
serializer.Serialize(config_data);

std::string config_data_js;
config_data_js.append("var configData = ");
config_data_js.append(js_text);
config_data_js.append(";");
return config_data_js;
}

} // namespace

LocalNtpSource::LocalNtpSource() {
LocalNtpSource::LocalNtpSource(Profile* profile) : profile_(profile) {
}

LocalNtpSource::~LocalNtpSource() {
Expand All @@ -106,9 +148,9 @@ void LocalNtpSource::StartDataRequest(
int render_view_id,
const content::URLDataSource::GotDataCallback& callback) {
const std::string stripped_path = StripParameters(path);
if (stripped_path == kTranslatedStringsFilename) {
std::string translated_strings_js = GetTranslatedStrings();
callback.Run(base::RefCountedString::TakeString(&translated_strings_js));
if (stripped_path == kConfigDataFilename) {
std::string config_data_js = GetConfigData(profile_);
callback.Run(base::RefCountedString::TakeString(&config_data_js));
return;
}
for (size_t i = 0; i < arraysize(kResources); ++i) {
Expand Down
6 changes: 5 additions & 1 deletion chrome/browser/search/local_ntp_source.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@
#include "base/compiler_specific.h"
#include "content/public/browser/url_data_source.h"

class Profile;

// Serves HTML and resources for the local new tab page i.e.
// chrome-search://local-ntp/local-ntp.html
class LocalNtpSource : public content::URLDataSource {
public:
LocalNtpSource();
explicit LocalNtpSource(Profile* profile);

private:
virtual ~LocalNtpSource();
Expand All @@ -30,6 +32,8 @@ class LocalNtpSource : public content::URLDataSource {
const net::URLRequest* request) const OVERRIDE;
virtual std::string GetContentSecurityPolicyFrameSrc() const OVERRIDE;

Profile* profile_;

DISALLOW_COPY_AND_ASSIGN(LocalNtpSource);
};

Expand Down
9 changes: 0 additions & 9 deletions chrome/browser/search/search.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/search/instant_service.h"
#include "chrome/browser/search/instant_service_factory.h"
#include "chrome/browser/search_engines/template_url_prepopulate_data.h"
#include "chrome/browser/search_engines/template_url_service.h"
#include "chrome/browser/search_engines/template_url_service_factory.h"
#include "chrome/browser/ui/browser.h"
Expand Down Expand Up @@ -348,14 +347,6 @@ GURL GetInstantURL(Profile* profile, int start_margin) {
}

GURL GetLocalInstantURL(Profile* profile) {
const TemplateURL* default_provider =
GetDefaultSearchProviderTemplateURL(profile);

if (default_provider &&
(TemplateURLPrepopulateData::GetEngineType(default_provider->url()) ==
SEARCH_ENGINE_GOOGLE)) {
return GURL(chrome::kChromeSearchLocalGoogleNtpUrl);
}
return GURL(chrome::kChromeSearchLocalNtpUrl);
}

Expand Down
5 changes: 2 additions & 3 deletions chrome/browser/search/search.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,8 @@ bool NavEntryIsInstantNTP(const content::WebContents* contents,
// lead to an infinite recursion.
GURL GetInstantURL(Profile* profile, int start_margin);

// Returns the Local Instant URL of the default search engine. In particular,
// a Google search provider will include a special query parameter, indicating
// to the JS that Google-specific New Tab Page elements should be rendered.
// Returns the Local Instant URL of the New Tab Page.
// TODO(kmadhusu): Remove this function and update the call sites.
GURL GetLocalInstantURL(Profile* profile);

// Returns true if 'use_remote_ntp_on_startup' flag is enabled in field trials
Expand Down
28 changes: 9 additions & 19 deletions chrome/browser/search/search_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,23 +178,19 @@ class SearchTest : public BrowserWithTestWindowTest {
TemplateURLService* template_url_service =
TemplateURLServiceFactory::GetForProfile(profile());
ui_test_utils::WaitForTemplateURLServiceToLoad(template_url_service);
SetSearchProvider(false);
SetSearchProvider();
}

void SetSearchProvider(bool is_google) {
void SetSearchProvider() {
TemplateURLService* template_url_service =
TemplateURLServiceFactory::GetForProfile(profile());
TemplateURLData data;
if (is_google) {
data.SetURL("http://www.google.com/");
data.instant_url = "http://www.google.com/";
} else {
data.SetURL("http://foo.com/url?bar={searchTerms}");
data.instant_url = "http://foo.com/instant?"
"{google:omniboxStartMarginParameter}foo=foo#foo=foo&strk";
data.alternate_urls.push_back("http://foo.com/alt#quux={searchTerms}");
data.search_terms_replacement_key = "strk";
}
data.SetURL("http://foo.com/url?bar={searchTerms}");
data.instant_url = "http://foo.com/instant?"
"{google:omniboxStartMarginParameter}foo=foo#foo=foo&strk";
data.alternate_urls.push_back("http://foo.com/alt#quux={searchTerms}");
data.search_terms_replacement_key = "strk";

TemplateURL* template_url = new TemplateURL(profile(), data);
// Takes ownership of |template_url|.
template_url_service->Add(template_url);
Expand Down Expand Up @@ -271,7 +267,6 @@ TEST_F(SearchTest, ShouldAssignURLToInstantRendererExtendedEnabled) {

const SearchTestCase kTestCases[] = {
{chrome::kChromeSearchLocalNtpUrl, true, ""},
{chrome::kChromeSearchLocalGoogleNtpUrl, true, ""},
{"https://foo.com/instant?strk", true, ""},
{"https://foo.com/instant#strk", true, ""},
{"https://foo.com/instant?strk=0", true, ""},
Expand Down Expand Up @@ -308,7 +303,6 @@ const SearchTestCase kInstantNTPTestCases[] = {
{"chrome://blank/", false, "Chrome scheme"},
{"chrome-search://foo", false, "Chrome-search scheme"},
{chrome::kChromeSearchLocalNtpUrl, true, "Local new tab page"},
{chrome::kChromeSearchLocalGoogleNtpUrl, true, "Local new tab page"},
{"https://bar.com/instant?strk=1", false, "Random non-search page"},
};

Expand All @@ -318,7 +312,6 @@ TEST_F(SearchTest, InstantNTPExtendedEnabled) {
for (size_t i = 0; i < arraysize(kInstantNTPTestCases); ++i) {
const SearchTestCase& test = kInstantNTPTestCases[i];
NavigateAndCommitActiveTab(GURL(test.url));
SetSearchProvider(test.url == chrome::kChromeSearchLocalGoogleNtpUrl);
const content::WebContents* contents =
browser()->tab_strip_model()->GetWebContentsAt(0);
EXPECT_EQ(test.expected_result, IsInstantNTP(contents))
Expand All @@ -343,7 +336,6 @@ TEST_F(SearchTest, InstantNTPCustomNavigationEntry) {
for (size_t i = 0; i < arraysize(kInstantNTPTestCases); ++i) {
const SearchTestCase& test = kInstantNTPTestCases[i];
NavigateAndCommitActiveTab(GURL(test.url));
SetSearchProvider(test.url == chrome::kChromeSearchLocalGoogleNtpUrl);
content::WebContents* contents =
browser()->tab_strip_model()->GetWebContentsAt(0);
content::NavigationController& controller = contents->GetController();
Expand Down Expand Up @@ -408,8 +400,6 @@ TEST_F(SearchTest, StartMarginCGI) {
TEST_F(SearchTest, CommandLineOverrides) {
EnableInstantExtendedAPIForTesting();

// GetLocalInstantURL() should default to the non-Google local NTP.
SetSearchProvider(false);
GURL local_instant_url(GetLocalInstantURL(profile()));
EXPECT_EQ(GURL(chrome::kChromeSearchLocalNtpUrl), local_instant_url);

Expand Down Expand Up @@ -444,7 +434,7 @@ TEST_F(SearchTest, CommandLineOverrides) {
// to get the Google version of the local NTP, even though search provider's
// URL doesn't contain "google".
local_instant_url = GetLocalInstantURL(profile());
EXPECT_EQ(GURL(chrome::kChromeSearchLocalGoogleNtpUrl), local_instant_url);
EXPECT_EQ(GURL(chrome::kChromeSearchLocalNtpUrl), local_instant_url);

// If we specify extra search query params, they should be inserted into the
// query portion of the instant URL.
Expand Down
Loading

0 comments on commit 5df96ab

Please sign in to comment.