Closed Bug 1590816 Opened 5 years ago Closed 5 years ago

Unify link coloring code in history implementations.

Categories

(Core :: General, task)

task
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox72 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

Details

Attachments

(16 files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

We have a lot of duplicated code in places' history and GeckoView's history. It is a pain to change.

Most of the code I'm interested in should be unified.

In preparation to unify the link coloring code with GeckoViewHistory, which
uses these hashtables for the equivalent of mObservers, it'd be good if we'd be
using the same.

In preparation for moving all the link coloring code.

GetHistoryService should work everywhere.

Interestingly the android implementation had a potentially serious bug (was
missing a script blocker).

Now without a duplicate version on another file :)

GeckoView does this link clearing stuff (which is somewhat dubious), but always
used to return NS_OK.

The error handling case in Link.cpp was pretty broken anyway (it'd leave the
link marked as registered), so make that infallible, given we fatally assert in
the non-android case.

NotifyVisited is also infallible so make that infallible too.

There's no need to use NS_IMETHOD gunk when not using scriptable XPCOM, and
these functions are not called from script, so make them regular pure-virtual
functions.

Depends on D50478.

See Also: → 1591132

Otherwise a page which appends links with different hrefs fast enough could
never see the visited state updated, for example.

Depends on D50480

And make the assertions consistent with places.

This was copied verbatim from places, but the only thing the code can do is
schedule runnables, which outlive the function, so there's no script that can
run.

Depends on D50502.

Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ae141ea79cb8 Improve assertions in history service. r=mak https://hg.mozilla.org/integration/autoland/rev/6cab6abf119e Marginally cleanup an error case in the history service. r=mak https://hg.mozilla.org/integration/autoland/rev/c0ca50bb81ce Use a more modern hash table for mRecentlyVisitedURI. r=mak https://hg.mozilla.org/integration/autoland/rev/c0b1c51a4ef3 Use the same data structures for GeckoViewHistory and places History. r=mak https://hg.mozilla.org/integration/autoland/rev/c340ffe9aa3b Move the tracked URIs hash table to IHistory. r=mak https://hg.mozilla.org/integration/autoland/rev/60601417a731 Simplify a rather weird ifdef set. r=mak https://hg.mozilla.org/integration/autoland/rev/d5c1df050f9f Introduce mozilla::BaseHistory, and move tracked URIs and a simple function there. r=mak https://hg.mozilla.org/integration/autoland/rev/f18c7c05ffd0 Move History::DispatchNotifyVisited and related code to BaseHistory. r=mak,lina https://hg.mozilla.org/integration/autoland/rev/f9c5be57dacb Move Register and UnregisterVisitedCallback to BaseHistory. r=mak,lina https://hg.mozilla.org/integration/autoland/rev/b93b7fb6f7f7 Move NotifyVisited to BaseHistory. r=mak,lina https://hg.mozilla.org/integration/autoland/rev/ceeff631a71b Make GetLinkDocument a static function again. r=mak https://hg.mozilla.org/integration/autoland/rev/10c5d5a32f62 Various naming and comment cleanups, as suggested in review comments. r=mak https://hg.mozilla.org/integration/autoland/rev/bdbaba80d7a0 Cleanup some link coloring APIs to make them infallible when appropriate. r=mak https://hg.mozilla.org/integration/autoland/rev/a4947b14d81f Don't re-initialize GeckoViewHistory's visited timer over and over. r=lina https://hg.mozilla.org/integration/autoland/rev/2279b41eef85 Don't remove tracked links from GeckoViewHistory. r=lina https://hg.mozilla.org/integration/autoland/rev/f915da42169e Remove a useless script blocker and comment from BaseHistory. r=mak
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: