I forgot to calculate frecency stats as part of the migration in bug 1239708. So the URLs that are autofilled after migration are wrong. We correctly pick the most frecent URL to fill, but we don't properly retain or discard it when comparing it against the threshold because the threshold is based on frecency stats. Stats are updated every time a frecency in moz_places changes. So after migration, once you start visiting pages, the stats will start to be updated, but they'll be based only on those visited pages, not on pages that were already in moz_places at the time of migration. We'll need to add another migration to calculate the initial stats. Mark, CC'ing you since you're working on a migration too (bug 824502), so one of us will need to (re)base on the other.

Don't worry. I think we'll stop other schema migrations until the autofill dust settles down.

This is ready for review, but I'll wait to see that try is green before asking. https://treeherder.mozilla.org/#/jobs?repo=try&revision=be2d7684a44d1eab278478abe91f70da11c63dfb

On second thought I'll go ahead and ask since you might still be around and I don't expect significant changes with this version of the patch to fix failures on try. :-)

Comment on attachment 8975978 [details] Bug 1461753 - Add places.sqlite migration for calculating initial frecency stats. https://reviewboard.mozilla.org/r/244194/#review250140 ::: toolkit/components/places/Database.cpp:2381 (Diff revision 1) > + "SELECT id, frecency FROM moz_places WHERE frecency > 0; " > + ), getter_AddRefs(stmt)); > + NS_ENSURE_SUCCESS(rv, rv); > + > + bool hasMore = false; > + while (NS_SUCCEEDED(stmt->ExecuteStep(&hasMore)) && hasMore) { ugh, this is looping through some tens thousands entries in the best case, more than 100 thousands in the worst case... And it's all on the main thread...

So, first I'd suggest we update places from the most frecent to the least one, so we need an order by... second we should probably do this async and in chunks, or only update n more frecent entries...

We already do that in the v48 migration though (O(count(moz_places)), although there the looping is entirely in SQL and not with ExecuteStep(). But I can make another patch with batched migration like the moz_origins.frecency batching.

This does async batching. It's a little complex because of the MOZ_ASSERT(mFrecencyStatsCount > 0) and MOZ_ASSERT(mFrecencyStatsSum >= uOld) underflow assertions in UpdateFrecencyStats. We're in danger of hitting those assertions if the places *and their frecencies* during the reset/recalculation are different from after. i.e., if places are added or removed during recalculation, or frecencies change during recalculation. So what I do is queue up normal stats updates during the recalculation and then replay them on the main thread when the recalculation is done. If we were OK with removing those assertions (and simply returning early when they would have been hit), then queuing and replaying wouldn't be necessary, I think. Of course then it would be possible for the stats to be incorrect, but realistically probably only slightly. Also, I call UpdateFrecencyStats (its new name is UpdateFrecencyStatsCore) on the bg thread. The obvious alternative to doing that would be to accumulate the stats on the bg thread directly, in the runnable. But that would mean we would be duplicating the accumulation logic, which isn't great IMO, especially if we were to add more stats in the future. So I made sure that the runnable keeps a strong reference to nsNavHistory, and I made the individual stats member variables atomic. This also fixes the compiler errors on Windows that happened on try. (Apparently VS doesn't like the struct foo tmp = { .bar = x } initializer syntax?)

(In reply to Drew Willcoxon :adw (Away May 21–25) from comment #11) > We're in danger of hitting those assertions if the places *and their frecencies* Er, probably should say *or their frecencies*, not and.

Comment on attachment 8975978 [details] Bug 1461753 - Add places.sqlite migration for calculating initial frecency stats. https://reviewboard.mozilla.org/r/244194/#review250222 I'm sorry, there's a few more architectural problems to evaluate :( These are the most critical issues imo: 1. using prefs is not going to work, because users do all sort of things like moving places.sqlite across different profiles, and Reset! We can use the moz_meta table to store these prefs instead 2. all these calculations can get lost in case of shutdown/crash, we need a recovery mode. We can probably store a pref when we are doing recalculation and redo it at the next run, though some data may still get lost (see below), so we also need periodic recalculations Note that Sqlite can load extensions dinamically, and one of the extensions (https://www.sqlite.org/contrib/download/extension-functions.c?get=25) provides various utils like stdev, variance, mode, median, lower_quartile, upper_quartile. We don't have support for dinamically loading extensions yet, but if that could be useful we could add it. If we'd have a stddev function, could we offload all the calculation work to Sqlite, or are there any additional requirements? Fwiw, it looks like the mean is already achievable through SELECT avg(frecency) FROM moz_places, and it's quite efficient. And this (https://stackoverflow.com/a/18470454) stackoverflow answer provides a method to implement a stddev user function. So potentially, we could replace all of this with a one-shot SELECT statement on frecency, that is also an indexed column. It may be worth measuring the cost of that to do a full recalculation. it may be cheaper than we expect. ::: toolkit/components/places/nsNavHistory.h:671 (Diff revision 3) > int32_t mReloadVisitBonus; > > void DecayFrecencyCompleted(uint16_t reason); > uint32_t mDecayFrecencyPendingCount; > > - uint64_t mFrecencyStatsCount; > + mozilla::Atomic<uint64_t> mFrecencyStatsCount; It would be great to add documentation above this block on how these stats are being used and updated. ::: toolkit/components/places/nsNavHistory.cpp:659 (Diff revision 3) > + int32_t aOldFrecency, > + int32_t aNewFrecency); > +private: > + void Finish(); > + > + RefPtr<nsNavHistory> mNavHistory; This should be an nsMainThreadPtrHandle and should be initialized to a new nsMainThreadPtrHolder because we can't be sure which thread will destroy this runnable. ::: toolkit/components/places/nsNavHistory.cpp:671 (Diff revision 3) > + } QueuedUpdate; > + nsTArray<QueuedUpdate> mQueuedUpdates; > +}; > + > void > nsNavHistory::UpdateFrecencyStats(int64_t aPlaceId, it's strange that we pass placeId, that is only used for a MOZ_ASSERT at line 721. If that check is that important, it could be done at the calling point. But honestly, is not placeId pretty much guaranteed by the schema itself? (when you select a frecency, it must be on a row that surely has a rowid) ::: toolkit/components/places/nsNavHistory.cpp:685 (Diff revision 3) > + // either could end up hitting the underflow assertions in > + // UpdateFrecencyStatsCore, not necessarily now but possibly at a later time > + // for a later update. So queue the update and replay it later. > + nsCOMPtr<nsIRunnable> runnable = > + mRecalculateFrecencyStatsRunnable; > + if (runnable) { is this addrefing and if strictly necessary? mRecalculateFrecencyStatsRunnable is already an owning pointer and we just checked it. What about you just define it as RefPtr<PlacesRecalculateFrecencyStatsRunnable> mRecalculateFrecencyStatsRunnable; so you don't need casts and friends? Also, if it's in the mozilla::places:: namespace, you can drop the "Places" prefix from its name. ::: toolkit/components/places/nsNavHistory.cpp:705 (Diff revision 3) > + if (!mUpdateFrecencyStatsPrefsTimer) { > + Unused << NS_NewTimerWithFuncCallback( > + getter_AddRefs(mUpdateFrecencyStatsPrefsTimer), > + &UpdateFrecencyStatsPrefsTimerFired, > + this, > + 5000, // ms Now that I think about it, it's possible we shutdown before this goes through, and then stats will be wrong forever? Maybe we should store them before shutdown if a timer is pending. Fwiw, the same issue will happen in case of a crash and we can't prevent that. Note it's also possible the user moves places.sqlite to a different profile with no stats, or a different places.sqlite into this profile. Then stats will just be wrong. I can only see these solutions, so far: 1. we store these in the moz_meta table instead of prefs, and we ignore the crash problem 2. we recalculate stats from scratch once a day on idle, and force a recalculate if the user didn't hit a idle in 48 hours 3. we do 1, and also add a task to maintenance to recalculate stats weekly I think I propend for 3, where the maintenance part could be done even later (not super critical) ::: toolkit/components/places/nsNavHistory.cpp:718 (Diff revision 3) > +void > +nsNavHistory::UpdateFrecencyStatsCore(int64_t aPlaceId, > + int32_t aOldFrecency, > + int32_t aNewFrecency) > +{ > + // Note that this is called off the main thread during frecency stats "may be invoked on both main and helper threads" ::: toolkit/components/places/nsNavHistory.cpp:795 (Diff revision 3) > + ), getter_AddRefs(stmt)); > + NS_ENSURE_SUCCESS(rv, rv); > + > + rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("offset"), mOffset); > + NS_ENSURE_SUCCESS(rv, rv); > + mOffset += 200; I'd probably do more than 200 at a time. Considering a db with 100k entries, this would need 500 runnables. Maybe 1k at a time? ::: toolkit/components/places/nsNavHistory.cpp:812 (Diff revision 3) > + } > + > + if (count == 200) { > + // There are more frecencies to handle. Re-dispatch to the same thread for > + // the next chunk. > + return NS_DispatchToCurrentThread(this); So, this can be interruped by any crash/shutdown, and we'd break. Sounds like we may need a pref to annotate that stats are "wrong" and should be recalculated. ::: toolkit/components/places/nsNavHistory.cpp:825 (Diff revision 3) > +PlacesRecalculateFrecencyStatsRunnable::QueueUpdate(int64_t aPlaceId, > + int32_t aOldFrecency, > + int32_t aNewFrecency) > +{ > + QueuedUpdate update; > + update.placeId = aPlaceId; let's drop the placeId everywhere it is not necessary. ::: toolkit/components/places/nsNavHistory.cpp:835 (Diff revision 3) > + > +void > +PlacesRecalculateFrecencyStatsRunnable::Finish() > +{ > + // Replay all the queued updates. > + for (auto &update : mQueuedUpdates) { And in case of a crash/shutdown, these will be lost forever :(

(In reply to Marco Bonardo [::mak] from comment #13) > 1. using prefs is not going to work, because users do all sort of things > like moving places.sqlite across different profiles, and Reset! We can use > the moz_meta table to store these prefs instead Sounds good. This should have been obvious to me from the beginning (and bug 1462275 too). > 2. all these calculations can get lost in case of shutdown/crash, we need a > recovery mode. We can probably store a pref when we are doing recalculation > and redo it at the next run, though some data may still get lost (see > below), so we also need periodic recalculations Yeah, I agree with what you wrote later: > 3. we do 1, and also add a task to maintenance to recalculate stats weekly > > I think I propend for 3, where the maintenance part could be done even later > (not super critical) I'll add this to the maintenance task. > Note that Sqlite can load extensions dinamically, and one of the extensions > (https://www.sqlite.org/contrib/download/extension-functions.c?get=25) > provides various utils like stdev, variance, mode, median, lower_quartile, > upper_quartile. We don't have support for dinamically loading extensions > yet, but if that could be useful we could add it. > If we'd have a stddev function, could we offload all the calculation work to > Sqlite, or are there any additional requirements? No additional requirements. We could "offload" the calculations to Sqlite already by implementing our own Sqlite functions (which is why I put "offload" in quotes). IMO that's not such a big win. In fact it would be a loss because then we'd be doing an extra O(number of origins) work each time we autofill instead of the O(1) work we do now with an incrementally calculated threshold. So I don't think computing the threshold each time we autofill is a good idea. It could be a good idea for doing the initial calculations (what this bug is about). But I don't think we should block this bug on supporting Sqlite extensions. However, implementing our own Sqlite functions would be one way to solve this bug. > Fwiw, it looks like the mean is already achievable through SELECT > avg(frecency) FROM moz_places, and it's quite efficient. It must be O(n) though unless Sqlite keeps its own incrementally calculated stats on these columns (or we could ask it to do that for us), which seems unlikely. Sorry for not replying to your comment yesterday. It took a day for me to absorb it and start working on an implementation.

Summary: * Keep stats in the moz_meta table. As before, the stats that are persisted (now in this table) aren't the mean and stddev but values that can be used to incrementally calculate them. * That lets us update stats directly in SQL. I changed the triggers to do this. I added a couple #defines for the stats updates because they're kind of big chunks of SQL, and I think it's clearer to pull them out (instead of including them inline in the trigger SQL), even though they're each only used in one place. * That also lets us set the initial stats directly in the SQL. I changed the Recalculate function to do this and got rid of chunking. * That also lets us have an autofill_frecency_threshold Sqlite function we can call directly in the SQL instead of calling nsINavHistoryService and then plugging that value into the query. (But I still exposed the stats on nsINavHistoryService so that tests and whoever else is interested can easily get them.) * Recalculate frecencies as part of database maintenance.

Comment on attachment 8975978 [details] Bug 1461753 - Add places.sqlite migration for calculating initial frecency stats. https://reviewboard.mozilla.org/r/244194/#review250964 Nothing blocking, but I really think GetFrecencyStatsSync wants a cache, because it's being invoked continuously by unifiedcomplete. Either here or in a follow-up, it should be done. ::: toolkit/components/places/Database.cpp:1370 (Diff revision 4) > > + // Initial frecency stats. > + rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( > + "INSERT OR IGNORE INTO moz_meta (key, value) " \ > + "VALUES ('" MOZ_META_KEY_FRECENCY_COUNT "', 0); " > + )); I assume you do this so you can just select more easily, but actually you can still select easily with default values using a query like this: WITH keys(key) AS ( VALUES ('a'), ('b') ) SELECT key, IFNULL(value, 0.0) AS value FROM keys LEFT JOIN moz_meta USING (key); It seems safer than depending on the database having the values created as part of the schema. Otherwise, for coherency you should probably also run these inserts synchronously in the migration step, to ensure all dbs are coherent. (PS: not sure if you want these to be INTEGER or REAL, for Sqlite it doesn't matter, you can store whatever datatype in whatever column, regardless of how it's defined) ::: toolkit/components/places/PlacesDBUtils.jsm:76 (Diff revision 4) > this.invalidateCaches, > this.checkCoherence, > this.expire, > this.vacuum, > this.stats, > + this.frecencyStats, These stats look a bit more important, I'd probably put it before vacuum. ::: toolkit/components/places/nsINavHistoryService.idl:1181 (Diff revision 4) > * Creates a new options item with the same parameters of this one. > */ > nsINavHistoryQueryOptions clone(); > }; > > -[scriptable, uuid(20c974ff-ee16-4828-9326-1b7c9e036622)] > +[scriptable, uuid(12f42bae-7fca-4259-9ecd-c718655e5c5c)] nit: fwiw, bumping the uuid is no more necessary (it won't hurt, it's just not useful) ::: toolkit/components/places/nsNavHistory.h:476 (Diff revision 4) > bool IsFrecencyDecaying() const; > > /** > - * Updates frecencyMean and frecencyStandardDeviation given a change in > - * frecency of a particular moz_places row. > - * > + * On the calling thread, synchronously fetches the frecency stats from the > + * database. > + */ Since it may query, we don't want to invoke this on the main-thread, maybe better adding a note about that here and a MOZ_ASSERT in GetFrecencyStatsSync I also think we should rename it to GetFrecencyStatsInternal, I prefer to avoid abusing the Sync naming. ::: toolkit/components/places/nsNavHistory.h:640 (Diff revision 4) > int32_t mReloadVisitBonus; > > void DecayFrecencyCompleted(uint16_t reason); > uint32_t mDecayFrecencyPendingCount; > > - uint64_t mFrecencyStatsCount; > + nsresult RecalculateFrecencyStatsSync(); ditto for s/Sync/Internal ::: toolkit/components/places/nsNavHistory.cpp:664 (Diff revision 4) > - // There's actually a browser mochitest that makes sure preferences aren't > - // accessed too much, and it fails without throttling like this. > - if (!mUpdateFrecencyStatsPrefsTimer) { > - Unused << NS_NewTimerWithFuncCallback( > - getter_AddRefs(mUpdateFrecencyStatsPrefsTimer), > - &UpdateFrecencyStatsPrefs, > + "SELECT frecency * frecency AS frecency_squared " \ > + "FROM moz_places " \ > + "WHERE id >= 0 AND frecency > 0 " \ > + "); " > + )); > + NS_ENSURE_SUCCESS(rv, rv); Recent Sqlite versions introduced the possibility to insert multiple rows at once... INSERT INTO 'tablename' ('column1', 'column2') VALUES ('data1', 'data2'), ('data3', 'data4'), ... And then you could avoid the explicit transaction. ::: toolkit/components/places/nsNavHistory.cpp:757 (Diff revision 4) > - double sum = static_cast<double>(mFrecencyStatsSum); > - double count = static_cast<double>(mFrecencyStatsCount); > - *_retval = sqrt((squares - ((sum * sum) / count)) / count); > +nsresult > +nsNavHistory::GetFrecencyStatsSync(nsINavHistoryFrecencyStats **_retval) const > +{ > + NS_ENSURE_ARG_POINTER(_retval); > + > + nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement( Since we are the only ones reading and updating these values, I think the next step will be to have a cache for these stats, because querying every time looks expensive. Especially when this is being used as a SQL function by autofill, this query will run continuously, but the data changes more rarely. Perfect conditions for a cache.

Comments addressed, thanks. As I mentioned on IRC, this new revision inlines the frecency threshold calculation in the SQL, in UnifiedComplete. That should get rid of the concern about caching. (I had to add a sqrt() Sqlite function.) It also means we don't need nsINavHistory.getFrecencyStats(), so I removed it. (In reply to Marco Bonardo [::mak] from comment #18) > ::: toolkit/components/places/Database.cpp:1370 > (Diff revision 4) > > > > + // Initial frecency stats. > > + rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( > > + "INSERT OR IGNORE INTO moz_meta (key, value) " \ > > + "VALUES ('" MOZ_META_KEY_FRECENCY_COUNT "', 0); " > > + )); > > I assume you do this so you can just select more easily, but actually you > can still select easily with default values using a query like this: I did it so that the update triggers are easier to write -- UPDATE instead of INSERT OR REPLACE. But I took your suggestion and removed the initial INSERTS and changed the update triggers to INSERT OR REPLACE.

Small improvement to test_preventive_maintenance.js (INSERT VALUES (...), (...), (...) instead of separate INSERT statements)

Comment on attachment 8975978 [details] Bug 1461753 - Add places.sqlite migration for calculating initial frecency stats. This fixes the moz_places_afterupdate_frecency_trigger logic we talked about. I think it's best just to go back to the logic that we were using for a long time without problem -- I mean the frecency decay rate calculation specifically. I changed that too as part of the autofill bug. It's possible that's part of the spikes you mentioned (in addition to the faulty conjunctions/negation). I also moved the is_frecency_decaying() call last in case there's overhead in calling it.

Comment on attachment 8975978 [details] Bug 1461753 - Add places.sqlite migration for calculating initial frecency stats. https://reviewboard.mozilla.org/r/244194/#review251202 I don't see blockers, but there are a few try failures. rs=me if the changes to fix those are trivial.

Not sure what caused these new failures. A few different tries: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d4b142bd15361eb2fd2ceed73eb3a3ffcfa4f72c https://treeherder.mozilla.org/#/jobs?repo=try&revision=8517db440478b18056c42da6d7ae4781e59e1fa4 https://treeherder.mozilla.org/#/jobs?repo=try&revision=2cfb066d3342304c48647a0534e696cf55d59d0b

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3326d5af0718bad065486c2be195e1a7cefd928e https://treeherder.mozilla.org/#/jobs?repo=try&revision=41375a1adf4c17cf2f80d7f37e56d9a4fce89c5b

https://treeherder.mozilla.org/#/jobs?repo=try&revision=957c7ae239d72e66e6f311af102be90b3a7dbb44 https://treeherder.mozilla.org/#/jobs?repo=try&revision=64f19e2bdbb761c808bd05494632c2bb9e622fce https://treeherder.mozilla.org/#/jobs?repo=try&revision=490be06568478851ad6cdb136fc1921ff974195a

Pushed by dwillcoxon@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/94e22944d8ea Add places.sqlite migration for calculating initial frecency stats. r=mak

I noticed Bug 1461736 was marked as qe-verify+ before it was closed as a duplicate of this bug. Marco or Drew, can one of you confirm that this needs to be verified by me? If so, please let me know of any suggested STR to be able to verify going forward. Thanks.

(In reply to Grover Wimberly IV [:Grover-QA] from comment #35) > I noticed Bug 1461736 was marked as qe-verify+ before it was closed as a > duplicate of this bug. Marco or Drew, can one of you confirm that this needs > to be verified by me? If so, please let me know of any suggested STR to be > able to verify going forward. Thanks. It's not trivial, because this bug pretty much doesn't exist in a new profile, it exists if you upgrade an old profile. I think one thing you could do is creating a profile in a previous version (61 or 60 will do), build some frecency and autofill by typing urls in the Address Bar. Then update that profile to 62, and (after a minute, just in case, since we have a migration) type in the Address Bar to check if hosts are still autofilled. The caveat is that the algorithm changed, so we can't do a perfect 1:1 comparison of before/after, but in most cases the same hosts should be filled. When picking urls please avoid the ones redirecting to a different host, like maps.google.com => www.google.com/maps, since that case is covered by bug 1463132, that is not fixed yet. Both me and Drew are available to double check results.

Thanks for the clarification. Before I confirm that it's verified-fixed in the context of Comment 36, what would be an example of behavior you're not wanting to see? I need to confirm the old behavior we're trying to avoid seeing on a build of Nightly from before the patch landing.

A behavior we'd not want to see is that typing once a url starts immediately autofilling it the next time you start typing it... But this requires you first build some frecency by visiting multiple times some other pages, since otherwise, in a clean profile, that single typed page could still have an high enough value to pass the threshold.

Clearing NI, I think Marco answered, thanks Marco.

Here are some specific STR. NI'ing you, Grover, just to make sure you see this. build1 (before the original autofill bug 1239708 landed): http://ftp.mozilla.org/pub/firefox/nightly/2018/05/2018-05-14-22-01-26-mozilla-central/ build2 (after the original autofill bug landed): http://ftp.mozilla.org/pub/firefox/nightly/2018/05/2018-05-15-22-00-59-mozilla-central/ build3 (after this bug landed): http://ftp.mozilla.org/pub/firefox/nightly/2018/05/2018-05-19-22-00-58-mozilla-central/ STR 1. Start build1 with a new profile 2. In the urlbar, type nytimes.com and hit return 3. Quit 4. Start build2 with the same profile 5. In the urlbar, type example.com and hit return 6. Do the previous step again 7. In the urlbar, delete all text and start typing nytimes.com. It should *not* be autofilled. 8. In the urlbar, delete all text and start typing example.com. It *should* be autofilled. 9. Quit 10. Start build3 with the same profile 11. In the urlbar, delete all text and start typing nytimes.com. It *should* be autofilled. 12. In the urlbar, delete all text and start typing example.com. It *should* be autofilled. 13. Open about:config and set devtools.chrome.enabled=true 14. Open the browser console 15. Paste in the code snippet below and hit return. The output should look like this: > Promise { <state>: "pending" } frecency_count 16 frecency_sum 7370 frecency_sum_of_squares 20171800 Code snippet: (async function() { let db = await PlacesUtils.promiseDBConnection(); let rows = await db.execute(`SELECT key, value FROM moz_meta WHERE key IN ("frecency_count", "frecency_sum", "frecency_sum_of_squares") ORDER BY key ASC;`); for (let row of rows) { console.log(row.getResultByIndex(0), row.getResultByIndex(1)); } })();

Hi Drew/Marco, Thanks for the feedback. I was able to verify on Windows 10 x64 and Ubuntu 18.10x64 with the builds provided. However, when testing Mac OSX 10.11, I was getting the following values: > Promise { <state>: "pending" } frecency_count 16 frecency_sum 7289 frecency_sum_of_squares 19966961 The steps otherwise worked the same and I had no issues. If this is intended behavior, I will mark as Verified-Fixed.

Please try this: (1) Run the snippet from comment 40 again and check that it produces the same numbers, to verify that nothing's changed (2) Run this snippet and paste the output here in the bug: (async function() { let db = await PlacesUtils.promiseDBConnection(); let rows = await db.execute(`SELECT frecency, url FROM moz_places`); for (let row of rows) { console.log(row.getResultByIndex(0), row.getResultByIndex(1)); } })(); (3) Do step 1 again (the numbers should remain the same)

The results from Step 2 are below: Promise { <state>: "pending" } 137 https://blog.nightly.mozilla.org/ debugger eval code:5:5 137 https://bugzilla.mozilla.org/ debugger eval code:5:5 137 https://developer.mozilla.org/ debugger eval code:5:5 137 https://addons.mozilla.org/en-US/firefox/addon/nightly-tester-tools/ debugger eval code:5:5 137 about:crashes debugger eval code:5:5 137 https://mibbit.com/?server=irc.mozilla.org&channel=%23nightly debugger eval code:5:5 137 https://planet.mozilla.org/ debugger eval code:5:5 137 https://www.mozilla.org/en-US/contribute/ debugger eval code:5:5 0 place:sort=8&maxResults=10 debugger eval code:5:5 0 place:type=6&sort=14&maxResults=10 debugger eval code:5:5 24 https://www.mozilla.org/privacy/firefox/ debugger eval code:5:5 98 https://www.mozilla.org/en-US/privacy/firefox/ debugger eval code:5:5 24 https://www.mozilla.org/projects/firefox/62.0a1/firstrun/ debugger eval code:5:5 24 https://www.mozilla.org/firefox/nightly/firstrun/ debugger eval code:5:5 49 https://www.mozilla.org/en-US/firefox/nightly/firstrun/ debugger eval code:5:5 24 http://nytimes.com/ debugger eval code:5:5 1950 https://www.nytimes.com/ debugger eval code:5:5 4000 http://example.com/ debugger eval code:5:5 After running Step 3, the results were indeed the same as of that from Step 1.

Thank you Grover. Your numbers from comment 41 match up with the numbers in comment 43, so this is working as expected. I'm not sure why they're different from the Windows and Linux numbers, but that's not important. I'll go ahead and mark this verified given your comment 41.

Per Comment 44.

Based on comments (In reply to Drew Willcoxon :adw from comment #44) > Thank you Grover. Your numbers from comment 41 match up with the numbers in > comment 43, so this is working as expected. I'm not sure why they're > different from the Windows and Linux numbers, but that's not important. > I'll go ahead and mark this verified given your comment 41. Based on the comments listed, updating the tracking flags as well.