723323 - Add the extensions.getAddons.getWithPerformance.url to mobile default prefs

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Blair McBride [:Unfocused] (UNAVAILABLE)]

To make it so we only send performance data to AMO when doing the background update check, bug 716736 added a new preference: extensions.getAddons.getWithPerformance.url

It's the same as extensions.getAddons.getWithPerformance.url, but with the performance query parameters added (those that were originally removed by bug 682356).

I'm assuming AMO tracks this data for mobile, but I don't know for sure.

Note that bug 682356 removed this in Fx11, and bug 716736 should hopefully be adding it back (for background update checks only) for Fx11. If we want no gap in the data for mobile (like desktop), this pref will need added for 11 (beta) for mobile too.

Comment 1

[Gregory Szorc [:gps]]

Attachment: Preference fix-up

Seems pretty straight-forward. Looks like we didn't remove all references to performance parameters when they were ripped out a few months ago. Whoops!

Comment 2

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 593639 [details] [diff] [review]
Preference fix-up

Review of attachment 593639 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/app/mobile.js
@@ +231,5 @@
>  pref("extensions.getAddons.search.browseURL", "https://addons.mozilla.org/%LOCALE%/mobile/search?q=%TERMS%");
>  pref("extensions.getAddons.search.url", "https://services.addons.mozilla.org/%LOCALE%/mobile/api/%API_VERSION%/search/%TERMS%/all/%MAX_RESULTS%/%OS%/%VERSION%/%COMPATIBILITY_MODE%");
>  pref("extensions.getAddons.browseAddons", "https://addons.mozilla.org/%LOCALE%/mobile/");
> +pref("extensions.getAddons.get.url", "https://services.addons.mozilla.org/%LOCALE%/mobile/api/%API_VERSION%/search/guid:%IDS%?src=mobile&appOS=%OS%&appVersion=%VERSION%");
> +pref("extensions.getAddons.getWithPerformance.url", "https://services.addons.mozilla.org/%LOCALE%/firefox/api/%API_VERSION%/search/guid:%IDS%?src=firefox&appOS=%OS%&appVersion=%VERSION%&tMain=%TIME_MAIN%&tFirstPaint=%TIME_FIRST_PAINT%&tSessionRestored=%TIME_SESSION_RESTORED%");

New pref is using /firefox/ instead of /mobile/, and src=firefox instead of src=mobile.

(Same goes for the other file.)

Comment 3

[Gregory Szorc [:gps]]

(In reply to Blair McBride (:Unfocused) from comment #2)
> New pref is using /firefox/ instead of /mobile/, and src=firefox instead of
> src=mobile.

*facepalm*

Comment 4

[Gregory Szorc [:gps]]

Attachment: Preference fix-up, v2

Removed clown shoes.

Comment 5

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 593691 [details] [diff] [review]
Preference fix-up, v2

Review of attachment 593691 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/app/mobile.js
@@ +231,5 @@
>  pref("extensions.getAddons.search.browseURL", "https://addons.mozilla.org/%LOCALE%/mobile/search?q=%TERMS%");
>  pref("extensions.getAddons.search.url", "https://services.addons.mozilla.org/%LOCALE%/mobile/api/%API_VERSION%/search/%TERMS%/all/%MAX_RESULTS%/%OS%/%VERSION%/%COMPATIBILITY_MODE%");
>  pref("extensions.getAddons.browseAddons", "https://addons.mozilla.org/%LOCALE%/mobile/");
> +pref("extensions.getAddons.get.url", "https://services.addons.mozilla.org/%LOCALE%/mobile/api/%API_VERSION%/search/guid:%IDS%?src=mobile&appOS=%OS%&appVersion=%VERSION%");
> +pref("extensions.getAddons.getWithPerformance.url", "https://services.addons.mozilla.org/%LOCALE%/mobile/api/%API_VERSION%/search/guid:%IDS%?src=firefox&appOS=%OS%&appVersion=%VERSION%&tMain=%TIME_MAIN%&tFirstPaint=%TIME_FIRST_PAINT%&tSessionRestored=%TIME_SESSION_RESTORED%");

Still one clownshoe on ;) src=firefox should be src=mobile

Comment 6

[Gregory Szorc [:gps]]

Attachment: Preference fix-up, v3

Remove other clown shoe.

Comment 7

[Gregory Szorc [:gps]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a9689287ce87

Comment 8

[Gregory Szorc [:gps]]

Comment on attachment 593695 [details] [diff] [review]
Preference fix-up, v3

[Approval Request Comment]

This patch goes hand in hand with bug 716736 and they should be treated as a unit. See approval comments on that bug.

Comment 9

[Alex Keybl [:akeybl]]

See https://bugzilla.mozilla.org/show_bug.cgi?id=716736#c16. Let's wait a couple of more days before approving.

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/a9689287ce87

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

Depends on bug716736 for uplift

Comment 12

[Mark Finkle (:mfinkle) (use needinfo?)]

Also, the URL for android uses "mobile" and this is changed in bug 715262

Comment 13

[Alex Keybl [:akeybl]]

Comment on attachment 593695 [details] [diff] [review]
Preference fix-up, v3

[Triage Comment]
Mobile only - approved for Aurora 12 and Beta 11.

Comment 14

[Gregory Szorc [:gps]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b577cc1e8d9b
https://hg.mozilla.org/releases/mozilla-beta/rev/6c764496e613

Patch did not apply cleanly on beta for some weird reason. The change was trivial, so I just copied the lines from tip. I have no clue why Mercurial failed to apply it. Who knows.