Closed Bug 1678618 Opened 1 year ago Closed 10 months ago

Evaluate merging onDeleteURI and onDeleteVisits

Categories

(Toolkit :: Places, task, P3)

task
Points:
3

Tracking

()

RESOLVED FIXED
87 Branch
Iteration:
86.3 - Jan 11 - Jan 24
Tracking Status
firefox87 --- fixed

People

(Reporter: mak, Assigned: daisuke)

References

(Blocks 2 open bugs)

Details

Attachments

(4 files, 2 obsolete files)

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?

Blocks: 1678628
Assignee: nobody → daisuke
Status: NEW → ASSIGNED
Iteration: --- → 86.3 - Jan 11 - Jan 24
Points: --- → 3

Depends on D101114

Depends on D101116

Depends on D101118

Blocks: 1685140
Attachment #9196005 - Attachment description: Bug 1678618: Imlement a mechanism to fire page-removed event instead of onDeleteURI. → Bug 1678618: Imlement a mechanism to fire page-removed event.
Attachment #9196007 - Attachment is obsolete: true
Attachment #9196006 - Attachment description: Bug 1678618: Apply page-removed event instead of onDeleteURI. → Bug 1678618: Apply page-removed event.
Attachment #9196008 - Attachment is obsolete: true
Blocks: 1692368
Attachment #9196009 - Attachment description: Bug 1678618: Remove nsINavHistoryObserver. → Bug 1678618: Remove onDeleteURI and onDeleteVisits from nsINavHistoryObserver.
Pushed by dakatsuka.birchill@mozilla.com:
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

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

Flags: needinfo?(daisuke)
Flags: needinfo?(daisuke)
Pushed by dakatsuka.birchill@mozilla.com:
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
You need to log in before you can comment on or make changes to this bug.