Bug 1075620 - Switch to GET for fetch to allow caching of links data from redirect [r=ttaubert]
Use GET instead of POST and update the fetch endpoint to use %LOCALE%.
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -1547,17 +1547,17 @@ pref("browser.newtabpage.enabled", true)
// number of rows of newtab grid
pref("browser.newtabpage.rows", 3);
// number of columns of newtab grid
pref("browser.newtabpage.columns", 5);
// directory tiles download URL
-pref("browser.newtabpage.directory.source", "https://tiles.services.mozilla.com/v2/links/fetch");
+pref("browser.newtabpage.directory.source", "https://tiles.services.mozilla.com/v2/links/fetch/%LOCALE%");
// endpoint to send newtab click and view pings
pref("browser.newtabpage.directory.ping", "https://tiles.services.mozilla.com/v2/links/");
// Enable the DOM fullscreen API.
pref("full-screen-api.enabled", true);
// True if the fullscreen API requires approval upon a domain entering fullscreen.
--- a/browser/modules/DirectoryLinksProvider.jsm
+++ b/browser/modules/DirectoryLinksProvider.jsm
@@ -178,16 +178,19 @@ let DirectoryLinksProvider = {
_removePrefsObserver: function DirectoryLinksProvider_removeObserver() {
for (let pref in this._observedPrefs) {
let prefName = this._observedPrefs[pref];
Services.prefs.removeObserver(prefName, this);
}
},
_fetchAndCacheLinks: function DirectoryLinksProvider_fetchAndCacheLinks(uri) {
+ // Replace with the same display locale used for selecting links data
+ uri = uri.replace("%LOCALE%", this.locale);
+
let deferred = Promise.defer();
let xmlHttp = new XMLHttpRequest();
let self = this;
xmlHttp.onload = function(aResponse) {
let json = this.responseText;
if (this.status && this.status != 200) {
json = "{}";
@@ -201,24 +204,22 @@ let DirectoryLinksProvider = {
});
};
xmlHttp.onerror = function(e) {
deferred.reject("Fetching " + uri + " results in error code: " + e.target.status);
};
try {
- xmlHttp.open('POST', uri);
+ xmlHttp.open("GET", uri);
// Override the type so XHR doesn't complain about not well-formed XML
xmlHttp.overrideMimeType(DIRECTORY_LINKS_TYPE);
// Set the appropriate request type for servers that require correct types
xmlHttp.setRequestHeader("Content-Type", DIRECTORY_LINKS_TYPE);
- xmlHttp.send(JSON.stringify({
- locale: this.locale,
- }));
+ xmlHttp.send();
} catch (e) {
deferred.reject("Error fetching " + uri);
Cu.reportError(e);
}
return deferred.promise;
},
/**
--- a/browser/modules/test/xpcshell/test_DirectoryLinksProvider.js
+++ b/browser/modules/test/xpcshell/test_DirectoryLinksProvider.js
@@ -48,32 +48,29 @@ const kPingUrl = kBaseUrl + kPingPath;
Services.prefs.setCharPref(kLocalePref, "en-US");
Services.prefs.setCharPref(kSourceUrlPref, kTestURL);
Services.prefs.setCharPref(kPingUrlPref, kPingUrl);
Services.prefs.setBoolPref(kNewtabEnhancedPref, true);
const kHttpHandlerData = {};
kHttpHandlerData[kExamplePath] = {"en-US": [{"url":"http://example.com","title":"RemoteSource"}]};
-const expectedBodyObject = {locale: DirectoryLinksProvider.locale};
const BinaryInputStream = CC("@mozilla.org/binaryinputstream;1",
"nsIBinaryInputStream",
"setInputStream");
+let gLastRequestPath;
function getHttpHandler(path) {
let code = 200;
let body = JSON.stringify(kHttpHandlerData[path]);
if (path == kFailPath) {
code = 204;
}
return function(aRequest, aResponse) {
- let bodyStream = new BinaryInputStream(aRequest.bodyInputStream);
- let bodyObject = JSON.parse(NetUtil.readInputStreamToString(bodyStream, bodyStream.available()));
- isIdentical(bodyObject, expectedBodyObject);
-
+ gLastRequestPath = aRequest.path;
aResponse.setStatusLine(null, code);
aResponse.setHeader("Content-Type", "application/json");
aResponse.write(body);
};
}
function isIdentical(actual, expected) {
if (expected == null) {
@@ -126,17 +123,19 @@ function promiseDirectoryDownloadOnPrefC
let oldValue = Services.prefs.getCharPref(pref);
if (oldValue != newValue) {
// if the preference value is already equal to newValue
// the pref service will not call our observer and we
// deadlock. Hence only setup observer if values differ
let observer = new LinksChangeObserver();
DirectoryLinksProvider.addObserver(observer);
Services.prefs.setCharPref(pref, newValue);
- return observer.deferred.promise;
+ return observer.deferred.promise.then(() => {
+ DirectoryLinksProvider.removeObserver(observer);
+ });
}
return Promise.resolve();
}
function promiseSetupDirectoryLinksProvider(options = {}) {
return Task.spawn(function() {
let linksURL = options.linksURL || kTestURL;
yield DirectoryLinksProvider.init();
@@ -291,17 +290,18 @@ add_task(function test_fetchAndCacheLink
let data = yield readJsonFile();
isIdentical(data, kURLData);
});
add_task(function test_fetchAndCacheLinks_remote() {
yield DirectoryLinksProvider.init();
yield cleanJsonFile();
// this must trigger directory links json download and save it to cache file
- yield DirectoryLinksProvider._fetchAndCacheLinks(kExampleURL);
+ yield promiseDirectoryDownloadOnPrefChange(kSourceUrlPref, kExampleURL + "%LOCALE%");
+ do_check_eq(gLastRequestPath, kExamplePath + "en-US");
let data = yield readJsonFile();
isIdentical(data, kHttpHandlerData[kExamplePath]);
});
add_task(function test_fetchAndCacheLinks_malformedURI() {
yield DirectoryLinksProvider.init();
yield cleanJsonFile();
let someJunk = "some junk";
@@ -331,17 +331,18 @@ add_task(function test_fetchAndCacheLink
// File should be empty.
let data = yield readJsonFile();
isIdentical(data, "");
});
add_task(function test_fetchAndCacheLinks_non200Status() {
yield DirectoryLinksProvider.init();
yield cleanJsonFile();
- yield DirectoryLinksProvider._fetchAndCacheLinks(kFailURL);
+ yield promiseDirectoryDownloadOnPrefChange(kSourceUrlPref, kFailURL);
+ do_check_eq(gLastRequestPath, kFailPath);
let data = yield readJsonFile();
isIdentical(data, {});
});
// To test onManyLinksChanged observer, trigger a fetch
add_task(function test_DirectoryLinksProvider__linkObservers() {
yield DirectoryLinksProvider.init();
@@ -503,16 +504,17 @@ add_task(function test_DirectoryLinksPro
yield cleanJsonFile();
// ensure that provider does not think it needs to download
do_check_false(DirectoryLinksProvider._needsDownload);
// change the source URL, which should force directory download
yield promiseDirectoryDownloadOnPrefChange(kSourceUrlPref, kExampleURL);
// then wait for testObserver to fire and test that json is downloaded
yield testObserver.deferred.promise;
+ do_check_eq(gLastRequestPath, kExamplePath);
let data = yield readJsonFile();
isIdentical(data, kHttpHandlerData[kExamplePath]);
yield promiseCleanDirectoryLinksProvider();
});
add_task(function test_DirectoryLinksProvider_fetchDirectoryOnShow() {
yield promiseSetupDirectoryLinksProvider();