Bug 1689275: Avoid notifying empty events. r=mak

Differential Revision: https://phabricator.services.mozilla.com/D103268
This commit is contained in:
Daisuke Akatsuka 2021-02-01 01:41:46 +00:00
parent 10d7e8d01b
commit f172e68f63
3 changed files with 54 additions and 29 deletions

View File

@ -276,39 +276,44 @@ void PlacesObservers::NotifyListeners(
void PlacesObservers::NotifyListeners(
const Sequence<OwningNonNull<PlacesEvent>>& aEvents) {
MOZ_ASSERT(aEvents.Length() > 0, "Must pass a populated array of events");
MOZ_RELEASE_ASSERT(!gCallingListeners);
gCallingListeners = true;
uint32_t flags = GetFlagsForEvents(aEvents);
CallListeners<RefPtr<PlacesEventCallback>, RefPtr<PlacesEventCallback>>(
flags, *JSListeners::GetListeners(), aEvents, [](auto& cb) { return cb; },
// MOZ_CAN_RUN_SCRIPT_BOUNDARY because on Windows this gets called from
// some internals of the std::function implementation that we can't
// annotate. We handle this by annotating CallListeners and making sure
// it holds a strong ref to the callback.
[&](auto& cb, const auto& events)
MOZ_CAN_RUN_SCRIPT_BOUNDARY { MOZ_KnownLive(cb)->Call(events); });
if (aEvents.Length() > 0) {
uint32_t flags = GetFlagsForEvents(aEvents);
CallListeners<WeakPtr<places::INativePlacesEventCallback>,
RefPtr<places::INativePlacesEventCallback>>(
flags, *WeakNativeListeners::GetListeners(), aEvents,
[](auto& cb) { return cb.get(); },
[&](auto& cb, const Sequence<OwningNonNull<PlacesEvent>>& events) {
cb->HandlePlacesEvent(events);
});
CallListeners<RefPtr<PlacesEventCallback>, RefPtr<PlacesEventCallback>>(
flags, *JSListeners::GetListeners(), aEvents,
[](auto& cb) { return cb; },
// MOZ_CAN_RUN_SCRIPT_BOUNDARY because on Windows this gets called from
// some internals of the std::function implementation that we can't
// annotate. We handle this by annotating CallListeners and making sure
// it holds a strong ref to the callback.
[&](auto& cb, const auto& events)
MOZ_CAN_RUN_SCRIPT_BOUNDARY { MOZ_KnownLive(cb)->Call(events); });
CallListeners<WeakPtr<PlacesWeakCallbackWrapper>,
RefPtr<PlacesWeakCallbackWrapper>>(
flags, *WeakJSListeners::GetListeners(), aEvents,
[](auto& cb) { return cb.get(); },
// MOZ_CAN_RUN_SCRIPT_BOUNDARY because on Windows this gets called from
// some internals of the std::function implementation that we can't
// annotate. We handle this by annotating CallListeners and making sure
// it holds a strong ref to the callback.
[&](auto& cb, const auto& events) MOZ_CAN_RUN_SCRIPT_BOUNDARY {
RefPtr<PlacesEventCallback> callback(cb->mCallback);
callback->Call(events);
});
CallListeners<WeakPtr<places::INativePlacesEventCallback>,
RefPtr<places::INativePlacesEventCallback>>(
flags, *WeakNativeListeners::GetListeners(), aEvents,
[](auto& cb) { return cb.get(); },
[&](auto& cb, const Sequence<OwningNonNull<PlacesEvent>>& events) {
cb->HandlePlacesEvent(events);
});
CallListeners<WeakPtr<PlacesWeakCallbackWrapper>,
RefPtr<PlacesWeakCallbackWrapper>>(
flags, *WeakJSListeners::GetListeners(), aEvents,
[](auto& cb) { return cb.get(); },
// MOZ_CAN_RUN_SCRIPT_BOUNDARY because on Windows this gets called from
// some internals of the std::function implementation that we can't
// annotate. We handle this by annotating CallListeners and making sure
// it holds a strong ref to the callback.
[&](auto& cb, const auto& events) MOZ_CAN_RUN_SCRIPT_BOUNDARY {
RefPtr<PlacesEventCallback> callback(cb->mCallback);
callback->Call(events);
});
}
auto& listenersToRemove = *JSListeners::GetListenersToRemove();
if (listenersToRemove.Length() > 0) {

View File

@ -2430,7 +2430,11 @@ class BookmarkObserverRecorder {
"Interrupted before notifying observers for new items"
);
}
PlacesObservers.notifyListeners(this.placesEvents);
if (this.placesEvents.length) {
PlacesObservers.notifyListeners(this.placesEvents);
}
await Async.yieldingForEach(
this.itemMovedArgs,
args => {

View File

@ -6,6 +6,7 @@
// * Listener: listen to single/multi event(s)
// * Event: fire single/multi type of event(s)
// * Timing: fire event(s) at same time/separately
// And also test notifying empty events.
add_task(async () => {
info("Test for listening to single event and firing single event");
@ -321,6 +322,21 @@ add_task(async () => {
await PlacesUtils.bookmarks.remove(bookmark.guid);
});
add_task(async function test_empty_notifications_array() {
info("Test whether listener does not receive empty events");
if (AppConstants.DEBUG) {
info(
"Ignore this test since we added a MOZ_ASSERT for empty events in debug build"
);
return;
}
const observer = startObservation(["page-visited"]);
PlacesObservers.notifyListeners([]);
Assert.equal(observer.firedEvents.length, 0, "Listener does not receive any");
});
function startObservation(targets) {
const observer = {
firedEvents: [],