Closed Bug 1677408 Opened 4 years ago Closed 4 years ago

Reduce history notifications traffic for bookmark views

Categories

(Toolkit :: Places, task, P2)

Product:
Toolkit
Component:
Places
Type:
task
Points:
5

Tracking

()

Status:
RESOLVED FIXED
Milestone:
85 Branch
Iteration:
85.1 - Nov 16 - Nov 29
Tracking Flags:
Tracking Status
firefox85 --- fixed

People

(Reporter: mak, Assigned: mak)

References

Details

Attachments

(7 files)

Bug 1677408 - Part 1 - Replace onPageChanged with a new favicon-changed notification. r=Standard8
(deleted), text/x-phabricator-request
Details
Bug 1677408 - Part 2 - Update consumers of onPageChanged. r=Standard8
(deleted), text/x-phabricator-request
Details
Bug 1677408 - Part 3 - Clean up bookmarks observer tracking in nsNavHistoryResult. r=Standard8
(deleted), text/x-phabricator-request
Details
Bug 1677408 - Part 4 - Replace onItemVisited with page-visited. r=Standard8
(deleted), text/x-phabricator-request
Details
Bug 1677408 - Part 5 - Remove no more used onBegin/EndUpdateBatch notifications forwarding. r=Standard8
(deleted), text/x-phabricator-request
Details
Bug 1677408 - Part 6 - Remove onItemVisited and the history observer in the bookmarks service. r=Standard8
(deleted), text/x-phabricator-request
Details
Bug 1677408 - Part 7 - Allow the view to opt-out of history details updates. r=standard8
(deleted), text/x-phabricator-request
Details

With some refactoring, we should be able to:

  1. remove OnPageChanged
  2. remove onItemVisited
  3. stop observing history from the bookmarks service
  4. Allow bookmark views that don't use nor show history details to not listen for new visits and not notify history details changes to the view

This should allow us to remove one history listener, and reduce the weight of history on bookmark views.

Use the new notifications system to notify about favicon changes, that is the only
part left of onPageChanged.

Depends on D97274

Rather than fetching bookmarks from the disk for each page-visited notification,
we just let the result to recursively find and update children in the memory
structure.

Depends on D97276

RunInBatchMode has been removed some time ago, the only thing still sending
these notifications is maintenance to force a UI update, and it can send both.

Depends on D97277

The result can directly listen for page-visited and recursively update children.
With onItemVisited gone, all the reasons for the bookmarks service to listen for
history notifications are gone, thus it can stop observing history.

Depends on D97278

In most cases bookmarks views don't need to be notified of history changes.
The only cases where they need that, is when history columns are visible in a
tree view, or the result is sorted by visits or frecency.
Thus, the menu and toolbar views can often save some work by not listening.

Depends on D97279

Blocks: 1473530
Pushed by mak77@bonardo.net: https://hg.mozilla.org/integration/autoland/rev/f7d6cc5e4457 Part 1 - Replace onPageChanged with a new favicon-changed notification. r=Standard8,emilio https://hg.mozilla.org/integration/autoland/rev/b0c23854e117 Part 2 - Update consumers of onPageChanged. r=Standard8 https://hg.mozilla.org/integration/autoland/rev/996cbd111957 Part 3 - Clean up bookmarks observer tracking in nsNavHistoryResult. r=Standard8 https://hg.mozilla.org/integration/autoland/rev/0f2d72d2ed5a Part 4 - Replace onItemVisited with page-visited. r=Standard8 https://hg.mozilla.org/integration/autoland/rev/ff93f264138b Part 5 - Remove no more used onBegin/EndUpdateBatch notifications forwarding. r=Standard8 https://hg.mozilla.org/integration/autoland/rev/4198bc35198c Part 6 - Remove onItemVisited and the history observer in the bookmarks service. r=Standard8 https://hg.mozilla.org/integration/autoland/rev/1e0dc9aaa979 Part 7 - Allow the view to opt-out of history details updates. r=Standard8

Thank you, I'm already verifying fixes on Try.

Flags: needinfo?(mak)
Pushed by mak77@bonardo.net: https://hg.mozilla.org/integration/autoland/rev/49f8c976469e Part 1 - Replace onPageChanged with a new favicon-changed notification. r=Standard8,emilio https://hg.mozilla.org/integration/autoland/rev/d739ef17da5f Part 2 - Update consumers of onPageChanged. r=Standard8 https://hg.mozilla.org/integration/autoland/rev/8684e7486a10 Part 3 - Clean up bookmarks observer tracking in nsNavHistoryResult. r=Standard8 https://hg.mozilla.org/integration/autoland/rev/f7c9fb064c37 Part 4 - Replace onItemVisited with page-visited. r=Standard8 https://hg.mozilla.org/integration/autoland/rev/51d608074a74 Part 5 - Remove no more used onBegin/EndUpdateBatch notifications forwarding. r=Standard8 https://hg.mozilla.org/integration/autoland/rev/bcaf47876119 Part 6 - Remove onItemVisited and the history observer in the bookmarks service. r=Standard8 https://hg.mozilla.org/integration/autoland/rev/9785de91c4fc Part 7 - Allow the view to opt-out of history details updates. r=Standard8
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: