1414438 - Use `getBatched` instead of `get` to backfill records

Status: RESOLVED FIXED

(Firefox :: Sync, enhancement, P1)

Description

[Lina Butler [:lina]]

`_processIncoming` currently uses `get` to fetch payloads for backfilled records. I think we can replace this with `getBatched`, as we already do for new records.

Comment 1

[tfe]

Hi,

Maybe i could fix this bug?
This is not my first bug, but i am quite new with the firefox source code.
So the replace needs to replace get by getBatched on https://dxr.mozilla.org/mozilla-central/source/services/sync/modules/engines.js#1311, right?

How can this be tester? There is a scary comment on the beginning of the function saying it cannot be tested?

Comment 2

[Lina Butler [:lina]]

(In reply to tfe from comment #1)
> Maybe i could fix this bug?

Of course, please feel free! The scary function got a refactor in bug 1368209, and has a test now. Let's change https://searchfox.org/mozilla-central/rev/5a60492a53667fc61a62af1847d005a210b7a4f6/services/sync/modules/engines.js#1207 to use `getBatched`.

For testing, I think setting `engine.downloadBatchSize` in https://searchfox.org/mozilla-central/rev/7e090b227f7a0ec44d4ded604823d48823158c51/services/sync/tests/unit/test_history_engine.js#96, and verifying the test still passes, should be sufficient.

Thanks!

Comment 3

[tfe]

Attachment: Bug 1414438 - Use `getBatched` instead of `get` to backfill records

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

Comment 4

[Lina Butler [:lina]]

Comment on attachment 8925847 [details]
Bug 1414438 - Use `getBatched` instead of `get` to backfill records

https://reviewboard.mozilla.org/r/197050/#review202644

Thanks!

::: services/sync/modules/engines.js:1202
(Diff revision 1)
>      let backfilledItems = this.itemSource();
>  
>      backfilledItems.sort = "newest";
>      backfilledItems.full = true;
>  
>      // `get` includes the list of IDs as a query parameter, so we need to fetch

Please change `get` to `getBatched` in the comment, too.

Comment 5

[tfe]

Comment on attachment 8925847 [details]
Bug 1414438 - Use `getBatched` instead of `get` to backfill records

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/197050/diff/1-2/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e794bc60c010
Use `getBatched` instead of `get` to backfill records r=kitcambridge

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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