1530816 - addonRepository returns emtpy array for rtamo

Status: VERIFIED FIXED

(WebExtensions :: General, defect)

Description

[Shane Caraveo (:mixedpuppy)]

It turns out the kill switch id we use results in an empty array returned from AddonRepository due to the id we send being different from what we get back[1].

Alternate fixes:

• Wait for 67: switch back to Fetch
  • but make sure there is one central implementation
  • make sure there are no hard coded urls as before
• Get it into 66: AMO returns the id it received

In the second case this could be handled outside firefox. Looking over the code, nothing in AddonRepository (result from getAddonsByIDs) uses the id beyond the filter mentioned. ActivityStream also does not use the id[2].

[1] https://searchfox.org/mozilla-central/rev/dbddac86aadf1d4871fb350bbe66db43728a9f81/toolkit/mozapps/extensions/internal/AddonRepository.jsm#469
[2] https://searchfox.org/mozilla-central/rev/dbddac86aadf1d4871fb350bbe66db43728a9f81/browser/components/newtab/lib/OnboardingMessageProvider.jsm#207

Comment 1

[Shane Caraveo (:mixedpuppy)]

We could also (if delaying till 67) fix AddonRepository to recognize the rta id.

Comment 2

[David Durst [:ddurst]]

CCing Stuart and Mat for visibility on the AMO side.

Comment 3

[Andrew Swan [:aswan]]

(In reply to Shane Caraveo (:mixedpuppy) from comment #1)

We could also (if delaying till 67) fix AddonRepository to recognize the rta id.

If this is the only blocker and rtamo is otherwise ready for 66, this would probably be a small enough change to uplift to beta.

Comment 4

[Mathieu Pillard [:mat]]

FWIW I would largely prefer not adding a hack to AMO for this.

Comment 5

[Stuart Colville [:muffinresearch]]

(In reply to Mathieu Pillard [:mat] from comment #4)

FWIW I would largely prefer not adding a hack to AMO for this.

I second this, and if an uplift for 66 is possible that sounds like the best solution to me.

Comment 6

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1530816 support RTA IDs in AddonRepository

AddonRepository validates IDs that are returned from AMO. We
need to handle the case where we are using rta prefix for ReturnToAMO.

Comment 7

[Shane Caraveo (:mixedpuppy)]

I've also manually verified the fix against a download on windows, about:welcome shows me tabby cat!

Comment 8

[Shane Caraveo (:mixedpuppy)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=29e53508f1f27bc4c3e9b1c231b803298469e52e

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4d4fca2edcb5
support RTA IDs in AddonRepository r=aswan

Comment 10

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 9047102 [details]
Bug 1530816 support RTA IDs in AddonRepository

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: None
• User impact if declined: addon recommendations cannot be provided.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: QE has test plans for the entire RTAMO flow.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Limited to AMO api use. Tests coverage is good on this.
• String changes made/needed: none

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9047102 [details]
Bug 1530816 support RTA IDs in AddonRepository

Support for testing return to AMO.
OK for uplift for beta 12.

Comment 12

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/4d4fca2edcb5

Comment 13

[Vlad Jiman]

Verified using the above build for Fx67 using multiple white listed extensions including tabby cat, everything appears fine using this version. Tests were run using Windows 10 x64.

Comment 14

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/releases/mozilla-beta/rev/420d4410f71f

Comment 15

[Vlad Jiman]

Verified using Firefox 66.0 (20190307095232) on Windows 10 x64 bit. Testing has been done in the same way as on Fx67, no issues have occurred with either white listed or non white listed addons.