Closed Bug 1353783 Opened 3 years ago Closed 3 years ago
Places handle Completion doesn't return embed visits in its updated Count argument
59 bytes, text/x-review-board-request
See https://bugzilla.mozilla.org/show_bug.cgi?id=1343182#c28 UpdatePlaces has this updatedCount that returns in handleCompletion, this doesn't account for embed visits, so it looks like no visit was added, when instead some were added. And embed visits should not notify onError and onResult if those are ignored by the callback.
Comment on attachment 8855290 [details] Bug 1353783 - updatePlaces doesn't correctly notify and count embed visits. https://reviewboard.mozilla.org/r/127166/#review129936 Code LGTM but I don't know about all the android stuff... ::: toolkit/components/places/tests/favicons/xpcshell.ini (Diff revision 1) > -# Bug 676989: test fails consistently on Android > -fail-if = os == "android" As noted on IRC, not sure what these removals are about. Are these just dead test annotations?
Attachment #8855290 - Flags: review?(gijskruitbosch+bugs) → review+
(In reply to :Gijs from comment #2) > ::: toolkit/components/places/tests/favicons/xpcshell.ini > (Diff revision 1) > > -# Bug 676989: test fails consistently on Android > > -fail-if = os == "android" > > As noted on IRC, not sure what these removals are about. Are these just dead > test annotations? My fault, I should have annotated that in the commit message. Basically we can remove those for 2 reasons: 1. These are from xul fennec, the current Firefox for Android doesn't use Places at all. 2. in any case the same is repeated in the head part of the ini file, so no reason to have single annotations.
(fwiw, the current Android build should not even build the places/ folder at all)
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/64f302609496 updatePlaces doesn't correctly notify and count embed visits. r=Gijs
You need to log in before you can comment on or make changes to this bug.