1476674 - Crash in nsNavHistory::RecalculateOriginFrecencyStats

Status: RESOLVED FIXED

(Toolkit :: Places, defect, P1)

Description

[[:philipp]]

This bug was filed from the Socorro interface and is
report bp-d5647c12-1610-4fba-98fa-c09780180717.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll nsNavHistory::RecalculateOriginFrecencyStats toolkit/components/places/nsNavHistory.cpp:617
1 xul.dll static nsresult mozilla::places::`anonymous namespace'::MigrateV52OriginFrecenciesRunnable::Run toolkit/components/places/Database.cpp:2599
2 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1051
3 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:519
4 xul.dll static bool mozilla::SpinEventLoopUntil<1, <lambda_c1e220acbaa4d185d2d1800aa1daa0b2> > xpcom/threads/nsThreadUtils.h:324
5 xul.dll mozilla::storage::Connection::SpinningSynchronousClose storage/mozStorageConnection.cpp:1385
6 xul.dll mozilla::places::Database::BackupAndReplaceDatabaseFile toolkit/components/places/Database.cpp:874
7 xul.dll mozilla::places::Database::EnsureConnection toolkit/components/places/Database.cpp:665
8 xul.dll mozilla::places::History::GetDBConn toolkit/components/places/History.cpp:2692
9 xul.dll mozilla::places::History::VisitURI toolkit/components/places/History.cpp:2872

=============================================================

this signature is newly appearing on 63.0a1 and 62.0b9 after uplifting bug 1467627. crashes appear to be happening during browser startup according to one comment and the uptime field in crash reports.

Comment 1

[Marco Bonardo [:mak]]

Looks like the eventTarget is not there, that means the connection has been shutdown or has not ever been there. We're not null checking the conn, nor the eventTarget.

Comment 2

[Drew Willcoxon :adw]

Attachment: Bug 1476674 - Fix crash in nsNavHistory::RecalculateOriginFrecencyStats

Comment 3

[Drew Willcoxon :adw]

This changes the MOZ_ASSERT(target) to NS_ENSURE_STATE(target).

While I'm here, I also simplified the target assignment slightly by getting the connection and calling do_GetInterface() in one line instead of two.  I didn't know whether passing null to do_GetInterface() was OK, so I tried hardcoding passing nullptr to it (instead of mDB->MainConn()) and as expected the NS_ENSURE_STATE failed and there wasn't a crash.

Comment 4

[Phabricator Automation]

Comment on attachment 8993156 [details]
Bug 1476674 - Fix crash in nsNavHistory::RecalculateOriginFrecencyStats

Marco Bonardo [::mak] has approved the revision.

https://phabricator.services.mozilla.com/D2232

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/522e7b9b6899
Fix crash in nsNavHistory::RecalculateOriginFrecencyStats r=mak

Comment 6

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/522e7b9b6899

Comment 7

[Drew Willcoxon :adw]

Attachment: Beta/62 patch

Approval Request Comment
[Feature/Bug causing the regression]:
Autofill improvements in bug 1467627

[User impact if declined]:
Possible crash on startup

[Is this code covered by automated tests?]:
Yes, but obviously it didn't catch this bug (hard to reproduce)

[Has the fix been verified in Nightly?]:
No

[Needs manual test from QE? If yes, steps to reproduce]:
No

[List of other uplifts needed for the feature/fix]:
None

[Is the change risky?]:
No

[Why is the change risky/not risky?]:
Small simple fix that simply returns from a method early if a pointer is null

[String changes made/needed]:
None

Comment 8

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8993544 [details] [diff] [review]
Beta/62 patch

Crash fix, let's uplift for beta 11.

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/releases/mozilla-beta/rev/b2360af9510e