Closed Bug 1388149 Opened 3 years ago Closed 3 years ago

PlacesUtils.history.insertMany ignores provided GUIDs

Categories

(Toolkit :: Places, defect, P1)

defect

Tracking

()

RESOLVED FIXED
Tracking Status
firefox56 --- unaffected
firefox57 --- fixed

People

(Reporter: tcsc, Assigned: tcsc)

References

Details

Attachments

(1 file)

This is causing TPS failures on stage. Trivial patch.
Comment on attachment 8894632 [details]
Bug 1388149 - Make PlacesUtils.history.insertMany respect provided GUIDs

https://reviewboard.mozilla.org/r/165792/#review170962

LGTM, although I think we should add a test to toolkit/components/places/tests/history/test_insert.js
Attachment #8894632 - Flags: review?(markh)
Comment on attachment 8894632 [details]
Bug 1388149 - Make PlacesUtils.history.insertMany respect provided GUIDs

https://reviewboard.mozilla.org/r/165792/#review171506

Looks great, thanks!

::: toolkit/components/places/tests/history/test_insertMany.js:166
(Diff revision 2)
> +      ]
> +    }
> +  ]);
> +
> +  Assert.ok(await PlacesUtils.history.fetch(guidA),
> +            "Record not inserted with correct guid");

We typically phrase these messages for the success case rather than the failure case - ie, "Record is inserted with correct GUID".

::: toolkit/components/places/tests/history/test_insertMany.js:193
(Diff revision 2)
> +    expectedGuids.delete(pageInfo.guid);
> +  });
> +  Assert.equal(expectedGuids.size, 0);
> +
> +
> +  Assert.ok(await PlacesUtils.history.fetch(guidB), "Record B not fetchable after insertMany");

ditto for these 2 lines.
Attachment #8894632 - Flags: review?(markh) → review+
Pushed by tchiovoloni@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3a1579259a05
Make PlacesUtils.history.insertMany respect provided GUIDs r=markh
https://hg.mozilla.org/mozilla-central/rev/3a1579259a05
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Since bug 1377944 was backed out I'm making this about places rather than Sync
Component: Sync → Places
Product: Firefox → Toolkit
Summary: Sync doesn't respect incoming history item guids → PlacesUtils.history.insertMany ignores provided GUIDs
Target Milestone: Firefox 57 → ---
You need to log in before you can comment on or make changes to this bug.