commit 81045bb958ec759eddab3ebd67841cb036b22282
parent fb001b1e1e967fac9fe1fb359fb94ecbab2453be
Author: Mike Conley <mconley@mozilla.com>
Date: Thu, 11 Dec 2025 17:44:19 +0000
Bug 2005544 - Send the pre-flight request unconditionally when communicating with MARS over OHTTP. r=home-newtab-reviewers,nbarrett
This configuration was originally controlled by a trainhopConfig flag, but it seems that
during initial startup, trainhopConfig isn't always fully populated, meaning that for the
first request to MARS over OHTTP, the preflight headers are not sent. That's not great.
This patch makes it so that if we're configured to communicate with MARS over OHTTP, we
always do the pre-flight part.
Differential Revision: https://phabricator.services.mozilla.com/D276010
Diffstat:
4 files changed, 43 insertions(+), 15 deletions(-)
diff --git a/browser/extensions/newtab/lib/DiscoveryStreamFeed.sys.mjs b/browser/extensions/newtab/lib/DiscoveryStreamFeed.sys.mjs
@@ -1200,6 +1200,8 @@ export class DiscoveryStreamFeed {
...(placements.length ? { placements } : {}),
};
+ const marsOhttpEnabled = state.Prefs.values[PREF_UNIFIED_ADS_OHTTP];
+
// Bug 1964715: Remove this logic when AdsFeed is 100% enabled
if (unifiedAdsEnabled && !adsFeedEnabled) {
const endpointBaseUrl = state.Prefs.values[PREF_UNIFIED_ADS_ENDPOINT];
@@ -1207,13 +1209,11 @@ export class DiscoveryStreamFeed {
unifiedAdsPlacements = this.getAdsPlacements();
const blockedSponsors =
state.Prefs.values[PREF_UNIFIED_ADS_BLOCKED_LIST];
- const preFlightConfig =
- state.Prefs.values?.trainhopConfig?.marsPreFlight || {};
// We need some basic data that we can pass along to the ohttp request.
// We purposefully don't use ohttp on this request. We also expect to
// mostly hit the HTTP cache rather than the network with these requests.
- if (preFlightConfig.enabled) {
+ if (marsOhttpEnabled) {
const preFlight = await this.fetchFromEndpoint(
`${endpointBaseUrl}v1/ads-preflight`,
{
@@ -1239,8 +1239,6 @@ export class DiscoveryStreamFeed {
};
}
- const marsOhttpEnabled = state.Prefs.values[PREF_UNIFIED_ADS_OHTTP];
-
let spocsResponse;
// Logic decision point: Query ads servers in this file or utilize AdsFeed method
if (adsFeedEnabled) {
diff --git a/browser/extensions/newtab/lib/TopSitesFeed.sys.mjs b/browser/extensions/newtab/lib/TopSitesFeed.sys.mjs
@@ -650,13 +650,10 @@ export class ContileIntegration {
const endpointBaseUrl = state.Prefs.values[PREF_UNIFIED_ADS_ENDPOINT];
- const preFlightConfig =
- state.Prefs.values?.trainhopConfig?.marsPreFlight || {};
-
// We need some basic data that we can pass along to the ohttp request.
// We purposefully don't use ohttp on this request. We also expect to
// mostly hit the HTTP cache rather than the network with these requests.
- if (preFlightConfig.enabled) {
+ if (marsOhttpEnabled) {
const preflightResponse = await this._topSitesFeed.fetch(
`${endpointBaseUrl}v1/ads-preflight`,
{
diff --git a/browser/extensions/newtab/test/unit/lib/DiscoveryStreamFeed.test.js b/browser/extensions/newtab/test/unit/lib/DiscoveryStreamFeed.test.js
@@ -1037,9 +1037,7 @@ describe("DiscoveryStreamFeed", () => {
"unifiedAds.spocs.enabled": true,
"discoverystream.placements.spocs": "newtab_stories_1",
"discoverystream.placements.spocs.counts": "1",
- trainhopConfig: {
- marsPreFlight: { enabled: true },
- },
+ "unifiedAds.ohttp.enabled": true,
},
},
});
diff --git a/browser/extensions/newtab/test/xpcshell/test_TopSitesFeed.js b/browser/extensions/newtab/test/xpcshell/test_TopSitesFeed.js
@@ -3400,7 +3400,7 @@ add_task(async function test_ContileIntegration() {
info(
"TopSitesFeed._fetchSites should cast headers from a Headers object to JS object when using OHTTP"
);
- let { feed } = prepFeed(getTopSitesFeedForTest(sandbox));
+ let { feed, fetchStub } = prepFeed(getTopSitesFeedForTest(sandbox));
Services.prefs.setStringPref(
"browser.newtabpage.activity-stream.discoverystream.ohttp.relayURL",
@@ -3423,6 +3423,21 @@ add_task(async function test_ContileIntegration() {
"1";
feed.store.state.Prefs.values["unifiedAds.blockedAds"] = "";
+ const TEST_PREFLIGHT_UA_STRING = "Some test UA";
+ const TEST_PREFLIGHT_GEONAME_ID = "Some geo name";
+ const TEST_PREFLIGHT_GEO_LOCATION = "Some geo location";
+
+ fetchStub.resolves({
+ ok: true,
+ status: 200,
+ json: () =>
+ Promise.resolve({
+ normalized_ua: TEST_PREFLIGHT_UA_STRING,
+ geoname_id: TEST_PREFLIGHT_GEONAME_ID,
+ geo_location: TEST_PREFLIGHT_GEO_LOCATION,
+ }),
+ });
+
const fakeOhttpConfig = { config: "config" };
sandbox.stub(ObliviousHTTP, "getOHTTPConfig").resolves(fakeOhttpConfig);
@@ -3453,6 +3468,8 @@ add_task(async function test_ContileIntegration() {
let fetched = await feed._contile._fetchSites();
+ Assert.ok(fetchStub.calledOnce, "The preflight request was made.");
+
Assert.ok(fetched);
Assert.ok(
ohttpRequestStub.calledOnce,
@@ -3465,8 +3482,10 @@ add_task(async function test_ContileIntegration() {
fakeOhttpConfig,
"config should be passed through"
);
+
+ const sentHeaders = callArgs[3].headers;
Assert.equal(
- typeof callArgs[3].headers,
+ typeof sentHeaders,
"object",
"headers should be a plain object"
);
@@ -3474,10 +3493,26 @@ add_task(async function test_ContileIntegration() {
// We use instanceof here since isInstance isn't available for
// Headers, it seems.
// eslint-disable-next-line mozilla/use-isInstance
- !(callArgs[3].headers instanceof Headers),
+ !(sentHeaders instanceof Headers),
"headers should not be a Headers instance"
);
+ Assert.equal(
+ sentHeaders["x-user-agent"],
+ TEST_PREFLIGHT_UA_STRING,
+ "Sent the x-user-agent header from preflight"
+ );
+ Assert.equal(
+ sentHeaders["x-geoname-id"],
+ TEST_PREFLIGHT_GEONAME_ID,
+ "Sent the x-geoname-id header from preflight"
+ );
+ Assert.equal(
+ sentHeaders["x-geo-location"],
+ TEST_PREFLIGHT_GEO_LOCATION,
+ "Sent the x-geo-location header from preflight"
+ );
+
Services.prefs.clearUserPref(
"browser.newtabpage.activity-stream.discoverystream.ohttp.relayURL"
);
