1262312 - A device update POST is made every time about:preferences:#sync is shown

Status: RESOLVED FIXED

(Firefox :: Sync, defect, P1)

Description

[Mark Hammond [:markh] [:mhammond]]

Every time soomething requests the name of the Sync device, a post is made to update the device name, even though the device name is not changed. This is due to a subtle implementation detail - the localName getter at https://dxr.mozilla.org/mozilla-central/rev/d9f50aa0a1aaf90499b85c31e0f329b762e80fdd/services/sync/modules/engines/clients.js#128 always calls the setter - even when the value is the same - and the setter causes us to update the device registration.

Comment 2

[Lina Butler (ex-Mozilla)]

Attachment: MozReview Request: Bug 1262312 - Don't update the device registration every time we open Sync preferences. r?markh

Review commit: https://reviewboard.mozilla.org/r/44461/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/44461/

Comment 3

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8738331 [details]
MozReview Request: Bug 1262312 - Don't update the device registration every time we open Sync preferences. r?markh

https://reviewboard.mozilla.org/r/44461/#review41201

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/43f067177f2d

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/43f067177f2d

Comment 6

[Lina Butler (ex-Mozilla)]

Comment on attachment 8738331 [details]
MozReview Request: Bug 1262312 - Don't update the device registration every time we open Sync preferences. r?markh

Approval Request Comment
[Feature/regressing bug #]: Bug 1227527.
[User impact if declined]: High load on the FxA servers every time the user syncs or opens Sync prefs.
[Describe test coverage new/current, TreeHerder]: Covered by existing unit tests.
[Risks and why]: Low risk. This change is limited in scope to the Sync clients engine, and rode the train to Aurora.
[String/UUID change made/needed]: None.

Comment 7

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

Marking this blocking for 46. We are holding back this morning from unthrottling 46. See bug 1268652.

Comment 8

[Lina Butler (ex-Mozilla)]

Comment on attachment 8738331 [details]
MozReview Request: Bug 1262312 - Don't update the device registration every time we open Sync preferences. r?markh

Requesting approval for release; same rationale as comment 6.

Comment 9

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

Comment on attachment 8738331 [details]
MozReview Request: Bug 1262312 - Don't update the device registration every time we open Sync preferences. r?markh

Needed to fix excessive server load on release. Please uplift to beta and m-r

Comment 10

[Lina Butler (ex-Mozilla)]

Uplifted. Thanks, Liz!

https://hg.mozilla.org/releases/mozilla-beta/rev/e6310162052b
https://hg.mozilla.org/releases/mozilla-release/rev/677179a9a050

Comment 11

[Mark Hammond [:markh] [:mhammond]]

Thanks Kit. I built from the release branch and verified it is working as expected. I can no longer reproduce unexpected requests to the device registration endpoint.

Comment 12

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

No one has mentioned esr. Does this affect 45.1.0 esr?

Comment 13

[Lina Butler (ex-Mozilla)]

I don't think so. Device registration is 46+, AFAIK.