We'll want to update both click and links: browser.newtabpage.directory.source https://tiles.up.mozillalabs.com/v1/links/fetch browser.newtabpage.directory.reportClickEndPoint https://tiles.up.mozillalabs.com/v1/links/click

adw, do you know what's a good way to turn off this pref for tests? There's hooks that prevent external connections: PROCESS-CRASH | automation.py | application crashed [@ nsSocketTransport::InitiateSocket()]

I see there's bug 1022785 but then also bug 1036028. It seems the pattern is to have the test harness set the appropriate prefs to prevent remote connections.

The only tests that trigger code paths that use those prefs are tiles-related, right? If so, you can just set the prefs in the relevant tests or head files by using SpecialPowers.pushPrefEnv [1] in mochitest variants or the usual pref services APIs in both mochitests and xpcshell tests. The mochitest framework sets a bunch of prefs when it starts if I recall correctly, so we could set the prefs there as a more heavyweight solution, if unrelated tests end up triggering these paths. Like how bug 892875 turned off background thumbnailing for all mochitests. For xpcshell tests, I don't think there's any batch of prefs that the framework sets when it starts, but xpcshell tests are generally more isolated than mochitests, so it seems like only tiles-related tests should be triggering these paths, so you can simply use the usual pref service APIs when the relevant tests start. Does that help? [1] http://mxr.mozilla.org/mozilla-central/source/testing/specialpowers/content/specialpowersAPI.js

(In reply to Drew Willcoxon :adw from comment #3) > The only tests that trigger code paths that use those prefs are > tiles-related, right? Not quite. Because the new tab gets preloaded, the directory links endpoint will be fetched even if the test doesn't require it directly. So it seems that we'll need to update the various test harnesses to change the default prefs ?

How could I forget. Sounds fine to me.

oyiptong, the current urls I have in patches: https://tiles.up.mozillalabs.com/v1/links/fetch https://tiles.up.mozillalabs.com/v1/links/view https://tiles.up.mozillalabs.com/v1/links/click You mentioned bumping them to v2 elsewhere. Should I do that and keep the other parts of the urls the same?

oyiptong, also in bug 1042214, I remove the directoryCount object from the fetch, so the fetch's payload is just {"locale":"en-US"} which arguably looks like the view/click payload with "tiles" being optional.

I note comment 7 as currently v1 returns 400 bad request for those without directoryCount.

wip pending link confirmation from oyiptong

Notes: could also remove toolkit/content/directoryLinks.json

The new urls will be the same, save for the version denominator. They will be: https://tiles.up.mozillalabs.com/v2/links/fetch https://tiles.up.mozillalabs.com/v2/links/view https://tiles.up.mozillalabs.com/v2/links/click

Comment on attachment 8473730 [details] [diff] [review] v1 Review of attachment 8473730 [details] [diff] [review]: ----------------------------------------------------------------- Yay remote links! It looks like the only thing that still uses directoryLinks.json is test_DirectoryLinksProvider.js, so shouldn't that file be removed along with this patch? It can be moved to the test directory if the test still really needs it.

Oh duh. I even left comment 10 to remind myself.

Bug 1042214 has changes to turn off remote connections for tests.

test_DirectoryLinksProvider ran fine without the file.

Comment on attachment 8474024 [details] [diff] [review] v2 Review of attachment 8474024 [details] [diff] [review]: ----------------------------------------------------------------- Oh I see, directoryLinks.json is also the name of the file in the profile directory where we cache remote links, not only the directory.source file packaged at that chrome URI, and that's what the test is concerned with.

testing/profiles/prefs_general.js wasn't enough

Tracking because enhanced tile is a new feature.

Comment on attachment 8477782 [details] [diff] [review] reftest fix instead? r=froydnj https://hg.mozilla.org/integration/fx-team/rev/d5cb280f36bf

https://hg.mozilla.org/mozilla-central/rev/d76f7a57af61 https://hg.mozilla.org/mozilla-central/rev/d5cb280f36bf