Bug 1538968 Comment 0 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

Unfortunately for us, HTML has several things that qualify as "links".

There are (at least?) `<a>`, `<area>` and `<link>` tags, which all inherit from [Link](https://searchfox.org/mozilla-central/source/dom/base/Link.h#34) in our codebase.

Fluent uses `<link rel="localization">` to keep track of localization files.

Places uses link state callbacks to keep track of history state:

```
#01: mozilla::places::History::GetIsVisitedStatement(mozIStorageCompletionCallback*)[dist/Nightly.app/Contents/MacOS/XUL +0x39d8273]
#02: mozilla::places::(anonymous namespace)::VisitedQuery::Start(nsIURI*, mozIVisitedStatusCallback*)[dist/Nightly.app/Contents/MacOS/XUL +0x39dafd3]
#03: mozilla::places::History::RegisterVisitedCallback(nsIURI*, mozilla::dom::Link*)[dist/Nightly.app/Contents/MacOS/XUL +0x39dac50]
#04: mozilla::dom::Link::LinkState() const[dist/Nightly.app/Contents/MacOS/XUL +0x1135d29]
#05: mozilla::dom::Document::FlushPendingLinkUpdates()[dist/Nightly.app/Contents/MacOS/XUL +0x10ff6b2]
#06: mozilla::detail::RunnableMethodImpl<mozilla::dom::Document*, void (mozilla::dom::Document::*)(), true, (mozilla::RunnableKind)0>::Run()[dist/Nightly.app/Contents/MacOS/XUL +0x1130837]
#07: IdleRunnableWrapper::Run()[dist/Nightly.app/Contents/MacOS/XUL +0xf50c7]
#08: nsThread::ProcessNextEvent(bool, bool*)[dist/Nightly.app/Contents/MacOS/XUL +0xeadc3]
#09: nsThreadManager::SpinEventLoopUntilInternal(nsINestedEventLoopCondition*, bool)[dist/Nightly.app/Contents/MacOS/XUL +0xecfc7]
```

In bug 1518234 we're trying to use fluent in migration.xul, which runs prior to profile setup if invoked using the `-migration` commandline flag, which also happens as part of Firefox Reset/Refresh.

Unfortunately, that breaks things. Rather badly, in fact. Because places initializes and [assumes that nobody is stupid enough to initialize it before there is a profile](https://searchfox.org/mozilla-central/rev/2c912888e3b7ae4baf161d98d7a01434f31830aa/toolkit/components/places/Database.cpp#549-552). If people *do* try and do so, things break. More specifically, the history component will just assume that everything is broken and fail all database connections for the rest of Firefox's uptime.

This manifests, if you apply the patch from bug 1518234, by the migration never completing. This is because it tries to do some initial bookmark imports and the initialize places, right after it forces profile startup (which would make the above profile directory available etc.). Of course, at this point it's too late and the database is corrupt. But the code will [bail out and never send a "places initialization is finished" notification](https://searchfox.org/mozilla-central/rev/2c912888e3b7ae4baf161d98d7a01434f31830aa/browser/components/BrowserGlue.jsm#2002-2011) which means that [the migration code waits forever for one of those notifications](https://searchfox.org/mozilla-central/rev/56705678f5fc363be5e0237e1686f619b0d23009/browser/components/migration/MigrationUtils.jsm#379-390), and so the dialog hangs forever.


So there's a few different things to consider in terms of fixing this... pile of unfortunate coincidences:

1. fix HTML to not have so many things called "link", because it's clearly a liability/design flaw. I'm told this is not an option.
2. make places' initialization code return early if called before we have a profile, and just no-op somehow (allowing us to retry later).
3. make fluent's <link>s not trigger history visited state thingies. I dunno what trips this, but it seems like a waste of time either way.

I think we probably need both 2/3, though I guess for the moment fixing either one should be sufficient to unblock 1518234. Marco, is there anything that would stop us from doing (2)? Gandalf, same question for (3) ?
Unfortunately for us, HTML has several things that qualify as "links".

There are (at least?) `<a>`, `<area>` and `<link>` tags, which all inherit from [Link](https://searchfox.org/mozilla-central/source/dom/base/Link.h#34) in our codebase.

Fluent uses `<link rel="localization">` to keep track of localization files.

Places uses link state callbacks to keep track of history state:

```
#01: mozilla::places::History::GetIsVisitedStatement(mozIStorageCompletionCallback*)[dist/Nightly.app/Contents/MacOS/XUL +0x39d8273]
#02: mozilla::places::(anonymous namespace)::VisitedQuery::Start(nsIURI*, mozIVisitedStatusCallback*)[dist/Nightly.app/Contents/MacOS/XUL +0x39dafd3]
#03: mozilla::places::History::RegisterVisitedCallback(nsIURI*, mozilla::dom::Link*)[dist/Nightly.app/Contents/MacOS/XUL +0x39dac50]
#04: mozilla::dom::Link::LinkState() const[dist/Nightly.app/Contents/MacOS/XUL +0x1135d29]
#05: mozilla::dom::Document::FlushPendingLinkUpdates()[dist/Nightly.app/Contents/MacOS/XUL +0x10ff6b2]
#06: mozilla::detail::RunnableMethodImpl<mozilla::dom::Document*, void (mozilla::dom::Document::*)(), true, (mozilla::RunnableKind)0>::Run()[dist/Nightly.app/Contents/MacOS/XUL +0x1130837]
#07: IdleRunnableWrapper::Run()[dist/Nightly.app/Contents/MacOS/XUL +0xf50c7]
#08: nsThread::ProcessNextEvent(bool, bool*)[dist/Nightly.app/Contents/MacOS/XUL +0xeadc3]
#09: nsThreadManager::SpinEventLoopUntilInternal(nsINestedEventLoopCondition*, bool)[dist/Nightly.app/Contents/MacOS/XUL +0xecfc7]
```

In bug 1518234 we're trying to use fluent in migration.xul, which runs prior to profile setup if invoked using the `-migration` commandline flag, which also happens as part of Firefox Reset/Refresh.

Unfortunately, that breaks things. Rather badly, in fact. Because places initializes and [assumes that nobody is stupid enough to initialize it before there is a profile](https://searchfox.org/mozilla-central/rev/2c912888e3b7ae4baf161d98d7a01434f31830aa/toolkit/components/places/Database.cpp#549-552). If people *do* try and do so, things break. More specifically, the history component will just assume that everything is broken and fail all database connections for the rest of Firefox's uptime.

This manifests, if you apply the patch from bug 1518234, by the migration never completing. This is because it tries to do some initial bookmark imports and then initialize places, right after it forces profile startup (which would make the above profile directory available etc.). Of course, at this point it's too late and the database is corrupt. But the code will [bail out and never send a "places initialization is finished" notification](https://searchfox.org/mozilla-central/rev/2c912888e3b7ae4baf161d98d7a01434f31830aa/browser/components/BrowserGlue.jsm#2002-2011) which means that [the migration code waits forever for one of those notifications](https://searchfox.org/mozilla-central/rev/56705678f5fc363be5e0237e1686f619b0d23009/browser/components/migration/MigrationUtils.jsm#379-390), and so the dialog hangs forever.


So there's a few different things to consider in terms of fixing this... pile of unfortunate coincidences:

1. fix HTML to not have so many things called "link", because it's clearly a liability/design flaw. I'm told this is not an option.
2. make places' initialization code return early if called before we have a profile, and just no-op somehow (allowing us to retry later).
3. make fluent's <link>s not trigger history visited state thingies. I dunno what trips this, but it seems like a waste of time either way.

I think we probably need both 2/3, though I guess for the moment fixing either one should be sufficient to unblock 1518234. Marco, is there anything that would stop us from doing (2)? Gandalf, same question for (3) ?

Back to Bug 1538968 Comment 0