1678618 - Evaluate merging onDeleteURI and onDeleteVisits

Status: RESOLVED FIXED

(Toolkit :: Places, task, P3)

Description

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

onDeleteURI indicates a page is removed from the DB, onDeleteVisits that visits to a page have been removed. The latter can also be a partial removal or a complete one.
If onDeleteURI is sent, onDeleteVisits is not, even if all the visits are removed.

This is a lot of complication and a bit confusing, we should analyze the consumers, and try to come up with a single notification, maybe a page-removed with removedFromStore and removedAllVisits boolean properties?

Comment 1

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1678618: Imlement a mechanism to fire page-removed event.

Depends on D100461

Comment 2

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1678618: Apply page-removed event.

Depends on D101114

Comment 3

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1678618: Extend page-removed event in order to replace onDeleteVisits.

Depends on D101115

Comment 4

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1678618: Apply page-removed event instead of onDeleteVisits.

Depends on D101116

Comment 5

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1678618: Remove onDeleteURI and onDeleteVisits from nsINavHistoryObserver.

Depends on D101117

Comment 6

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1678618: Avoid firing extra events in NewTabUtils.

Depends on D101118

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ec96259a3f81
Imlement a mechanism to fire page-removed event. r=mak
https://hg.mozilla.org/integration/autoland/rev/bef0ea72ded8
Apply page-removed event. r=mak
https://hg.mozilla.org/integration/autoland/rev/ae83fda183dd
Remove onDeleteURI and onDeleteVisits from nsINavHistoryObserver. r=mak
https://hg.mozilla.org/integration/autoland/rev/e714440f6c0d
Avoid firing extra events in NewTabUtils. r=mak

Comment 8

[Natalia Csoregi [:nataliaCs]]

Backed out for node failures

backout: https://hg.mozilla.org/integration/autoland/rev/ffed7ebd405ac9b6b0d7bf862190a63019ecb9f1

push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=HtxwRalQSXSuhn2Vtvw2FQ.0&revision=e714440f6c0d5eab905f2388bf89c4ca9fc47178&searchStr=linux%2C18.04%2Cx64%2Copt%2Cnode%2Ctests%2Csource-test-node-newtab-unit-tests%2Cnewtab

failure log: https://treeherder.mozilla.org/logviewer?job_id=329967416&repo=autoland&lineNumber=95

[task 2021-02-15T03:38:01.237Z] TEST START | karma
[task 2021-02-15T03:38:40.929Z] npm ERR! code ELIFECYCLE
[task 2021-02-15T03:38:40.929Z] npm ERR! errno 1
[task 2021-02-15T03:38:40.929Z] npm ERR! activity-streams@1.14.3 testmc:unit: karma start karma.mc.config.js
[task 2021-02-15T03:38:40.929Z] npm ERR! Exit status 1
[task 2021-02-15T03:38:40.929Z] npm ERR!
[task 2021-02-15T03:38:40.929Z] npm ERR! Failed at the activity-streams@1.14.3 testmc:unit script.
[task 2021-02-15T03:38:40.929Z] npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
[task 2021-02-15T03:38:40.929Z]
[task 2021-02-15T03:38:40.929Z] npm ERR! A complete log of this run can be found in:
[task 2021-02-15T03:38:40.929Z] npm ERR! /builds/worker/.npm/_logs/2021-02-15T03_38_40_886Z-debug.log
[task 2021-02-15T03:38:40.945Z] TEST-UNEXPECTED-FAIL karma | activity-stream:PlacesFeed:Custom dispatch should only dispatch 1 PLACES_LINKS_CHANGED action if any page-removed notifications happened at once: expected spy to be called once but was called 0 times
[task 2021-02-15T03:38:40.945Z] fail@http://localhost:9876/base/node_modules/sinon/pkg/sinon.js?fd64f3a13cddbb3783308d595307f8f6c1c10a4b:167:21
[task 2021-02-15T03:38:40.945Z] failAssertion@http://localhost:9876/base/node_modules/sinon/pkg/sinon.js?fd64f3a13cddbb3783308d595307f8f6c1c10a4b:126:16
[task 2021-02-15T03:38:40.945Z] mirrorPropAsAssertion/assert[name]@http://localhost:9876/base/node_modules/sinon/pkg/sinon.js?fd64f3a13cddbb3783308d595307f8f6c1c10a4b:152:26
[task 2021-02-15T03:38:40.945Z] @http://localhost:9876/base/test/unit/unit-entry.js?f81030d388b3cf5ce8bdeb6c829b185d7f001dad:154291:14
[task 2021-02-15T03:38:40.945Z]
[task 2021-02-15T03:38:40.945Z] TEST-UNEXPECTED-FAIL karma | activity-stream:PlacesFeed:PlacesObserver:#page-removed should dispatch a PLACES_LINK_DELETED action with the right url: expected spy to be called with arguments
[task 2021-02-15T03:38:40.945Z] fail@http://localhost:9876/base/node_modules/sinon/pkg/sinon.js?fd64f3a13cddbb3783308d595307f8f6c1c10a4b:167:21
[task 2021-02-15T03:38:40.945Z] failAssertion@http://localhost:9876/base/node_modules/sinon/pkg/sinon.js?fd64f3a13cddbb3783308d595307f8f6c1c10a4b:126:16
[task 2021-02-15T03:38:40.945Z] mirrorPropAsAssertion/assert[name]@http://localhost:9876/base/node_modules/sinon/pkg/sinon.js?fd64f3a13cddbb3783308d595307f8f6c1c10a4b:152:26
[task 2021-02-15T03:38:40.945Z] @http://localhost:9876/base/test/unit/unit-entry.js?f81030d388b3cf5ce8bdeb6c829b185d7f001dad:154319:16
[task 2021-02-15T03:38:40.945Z]
[task 2021-02-15T03:38:40.946Z] { checkBundles: true, karma: false }
[task 2021-02-15T03:38:40.947Z] CODE 1
[taskcluster 2021-02-15 03:38:41.248Z] === Task Finished ===
[taskcluster 2021-02-15 03:38:41.248Z] Unsuccessful task run with exit code: 1 completed in 72.762 seconds

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3ba9536a50cb
Imlement a mechanism to fire page-removed event. r=mak
https://hg.mozilla.org/integration/autoland/rev/d834ae135986
Apply page-removed event. r=mak
https://hg.mozilla.org/integration/autoland/rev/3e51a8ea19c3
Remove onDeleteURI and onDeleteVisits from nsINavHistoryObserver. r=mak
https://hg.mozilla.org/integration/autoland/rev/6b16c2d4f144
Avoid firing extra events in NewTabUtils. r=mak

Comment 10

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/3ba9536a50cb
https://hg.mozilla.org/mozilla-central/rev/d834ae135986
https://hg.mozilla.org/mozilla-central/rev/3e51a8ea19c3
https://hg.mozilla.org/mozilla-central/rev/6b16c2d4f144