From 5828eecda1490657aff4bb4e31ccefd994828fb3 Mon Sep 17 00:00:00 2001 From: Phil Ringnalda Date: Sun, 30 Mar 2014 12:42:09 -0700 Subject: [PATCH] Back out ab12037022ef:81f65b2f3d07 (bug 911307) for intermittent Win8 debug failures in its browser_newtab_update.js CLOSED TREE --- browser/base/content/newtab/page.js | 13 +- browser/base/content/test/newtab/browser.ini | 1 - .../test/newtab/browser_newtab_update.js | 52 --- browser/base/content/test/newtab/head.js | 34 +- .../downloads/nsDownloadManager.cpp | 16 - toolkit/components/places/Database.cpp | 2 - toolkit/components/places/History.cpp | 23 +- toolkit/components/places/SQLFunctions.cpp | 58 --- toolkit/components/places/SQLFunctions.h | 37 -- .../places/nsINavHistoryService.idl | 33 +- toolkit/components/places/nsNavBookmarks.cpp | 18 - toolkit/components/places/nsNavHistory.cpp | 196 +------- toolkit/components/places/nsNavHistory.h | 23 - .../components/places/nsNavHistoryResult.cpp | 36 -- .../components/places/nsNavHistoryResult.h | 4 - .../tests/unit/test_frecency_observers.js | 85 ---- .../components/places/tests/unit/xpcshell.ini | 1 - toolkit/modules/BinarySearch.jsm | 74 --- toolkit/modules/NewTabUtils.jsm | 423 ++---------------- toolkit/modules/moz.build | 1 - .../tests/xpcshell/test_BinarySearch.js | 81 ---- .../tests/xpcshell/test_NewTabUtils.js | 176 -------- toolkit/modules/tests/xpcshell/xpcshell.ini | 2 - 23 files changed, 61 insertions(+), 1328 deletions(-) delete mode 100644 browser/base/content/test/newtab/browser_newtab_update.js delete mode 100644 toolkit/components/places/tests/unit/test_frecency_observers.js delete mode 100644 toolkit/modules/BinarySearch.jsm delete mode 100644 toolkit/modules/tests/xpcshell/test_BinarySearch.js delete mode 100644 toolkit/modules/tests/xpcshell/test_NewTabUtils.js diff --git a/browser/base/content/newtab/page.js b/browser/base/content/newtab/page.js index fa0686246733..112f7b8e9ea4 100644 --- a/browser/base/content/newtab/page.js +++ b/browser/base/content/newtab/page.js @@ -36,11 +36,7 @@ let gPage = { * thumbnail service. */ get allowBackgroundCaptures() { - // The preloader is bypassed altogether for private browsing windows, and - // therefore allow-background-captures will not be set. In that case, the - // page is not preloaded and so it's visible, so allow background captures. - return inPrivateBrowsingMode() || - document.documentElement.getAttribute("allow-background-captures") == + return document.documentElement.getAttribute("allow-background-captures") == "true"; }, @@ -69,13 +65,10 @@ let gPage = { /** * Updates the whole page and the grid when the storage has changed. - * @param aOnlyIfHidden If true, the page is updated only if it's hidden in - * the preloader. */ - update: function Page_update(aOnlyIfHidden=false) { - let skipUpdate = aOnlyIfHidden && this.allowBackgroundCaptures; + update: function Page_update() { // The grid might not be ready yet as we initialize it asynchronously. - if (gGrid.ready && !skipUpdate) { + if (gGrid.ready) { gGrid.refresh(); } }, diff --git a/browser/base/content/test/newtab/browser.ini b/browser/base/content/test/newtab/browser.ini index 01ec5ab570da..c7b1c7f9449d 100644 --- a/browser/base/content/test/newtab/browser.ini +++ b/browser/base/content/test/newtab/browser.ini @@ -24,4 +24,3 @@ skip-if = os == "mac" # Intermittent failures, bug 898317 [browser_newtab_tabsync.js] [browser_newtab_undo.js] [browser_newtab_unpin.js] -[browser_newtab_update.js] diff --git a/browser/base/content/test/newtab/browser_newtab_update.js b/browser/base/content/test/newtab/browser_newtab_update.js deleted file mode 100644 index c9c36dfa1354..000000000000 --- a/browser/base/content/test/newtab/browser_newtab_update.js +++ /dev/null @@ -1,52 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - http://creativecommons.org/publicdomain/zero/1.0/ */ - -/** - * Checks that newtab is updated as its links change. - */ - -function runTests() { - if (NewTabUtils.allPages.updateScheduledForHiddenPages) { - // Wait for dynamic updates triggered by the previous test to finish. - yield whenPagesUpdated(null, true); - } - - // First, start with an empty page. setLinks will trigger a hidden page - // update because it calls clearHistory. We need to wait for that update to - // happen so that the next time we wait for a page update below, we catch the - // right update and not the one triggered by setLinks. - // - // Why this weird way of yielding? First, these two functions don't return - // promises, they call TestRunner.next when done. Second, the point at which - // setLinks is done is independent of when the page update will happen, so - // calling whenPagesUpdated cannot wait until that time. - setLinks([]); - whenPagesUpdated(null, true); - yield null; - yield null; - - // Strategy: Add some visits, open a new page, check the grid, repeat. - fillHistory([link(1)]); - yield whenPagesUpdated(null, true); - yield addNewTabPageTab(); - checkGrid("1,,,,,,,,"); - - fillHistory([link(2)]); - yield whenPagesUpdated(null, true); - yield addNewTabPageTab(); - checkGrid("2,1,,,,,,,"); - - fillHistory([link(1)]); - yield whenPagesUpdated(null, true); - yield addNewTabPageTab(); - checkGrid("1,2,,,,,,,"); - - fillHistory([link(2), link(3), link(4)]); - yield whenPagesUpdated(null, true); - yield addNewTabPageTab(); - checkGrid("2,1,3,4,,,,,"); -} - -function link(id) { - return { url: "http://example.com/#" + id, title: "site#" + id }; -} diff --git a/browser/base/content/test/newtab/head.js b/browser/base/content/test/newtab/head.js index fa298d506873..6df4bff9c620 100644 --- a/browser/base/content/test/newtab/head.js +++ b/browser/base/content/test/newtab/head.js @@ -159,34 +159,20 @@ function clearHistory(aCallback) { function fillHistory(aLinks, aCallback) { let numLinks = aLinks.length; - if (!numLinks) { - if (aCallback) - executeSoon(aCallback); - return; - } - let transitionLink = Ci.nsINavHistoryService.TRANSITION_LINK; - // Important: To avoid test failures due to clock jitter on Windows XP, call - // Date.now() once here, not each time through the loop. - let now = Date.now() * 1000; - - for (let i = 0; i < aLinks.length; i++) { - let link = aLinks[i]; + for (let link of aLinks.reverse()) { let place = { uri: makeURI(link.url), title: link.title, - // Links are secondarily sorted by visit date descending, so decrease the - // visit date as we progress through the array so that links appear in the - // grid in the order they're present in the array. - visits: [{visitDate: now - i, transitionType: transitionLink}] + visits: [{visitDate: Date.now() * 1000, transitionType: transitionLink}] }; PlacesUtils.asyncHistory.updatePlaces(place, { handleError: function () ok(false, "couldn't add visit to history"), handleResult: function () {}, handleCompletion: function () { - if (--numLinks == 0 && aCallback) + if (--numLinks == 0) aCallback(); } }); @@ -517,18 +503,12 @@ function createDragEvent(aEventType, aData) { /** * Resumes testing when all pages have been updated. - * @param aCallback Called when done. If not specified, TestRunner.next is used. - * @param aOnlyIfHidden If true, this resumes testing only when an update that - * applies to pre-loaded, hidden pages is observed. If - * false, this resumes testing when any update is observed. */ -function whenPagesUpdated(aCallback, aOnlyIfHidden=false) { +function whenPagesUpdated(aCallback) { let page = { - update: function (onlyIfHidden=false) { - if (onlyIfHidden == aOnlyIfHidden) { - NewTabUtils.allPages.unregister(this); - executeSoon(aCallback || TestRunner.next); - } + update: function () { + NewTabUtils.allPages.unregister(this); + executeSoon(aCallback || TestRunner.next); } }; diff --git a/toolkit/components/downloads/nsDownloadManager.cpp b/toolkit/components/downloads/nsDownloadManager.cpp index 13508c7402ff..c9c85db5c508 100644 --- a/toolkit/components/downloads/nsDownloadManager.cpp +++ b/toolkit/components/downloads/nsDownloadManager.cpp @@ -2338,22 +2338,6 @@ nsDownloadManager::OnTitleChanged(nsIURI *aURI, return NS_OK; } -NS_IMETHODIMP -nsDownloadManager::OnFrecencyChanged(nsIURI* aURI, - int32_t aNewFrecency, - const nsACString& aGUID, - bool aHidden, - PRTime aLastVisitDate) -{ - return NS_OK; -} - -NS_IMETHODIMP -nsDownloadManager::OnManyFrecenciesChanged() -{ - return NS_OK; -} - NS_IMETHODIMP nsDownloadManager::OnDeleteURI(nsIURI *aURI, const nsACString& aGUID, diff --git a/toolkit/components/places/Database.cpp b/toolkit/components/places/Database.cpp index 0177b3ed7113..b75d97e43b80 100644 --- a/toolkit/components/places/Database.cpp +++ b/toolkit/components/places/Database.cpp @@ -941,8 +941,6 @@ Database::InitFunctions() NS_ENSURE_SUCCESS(rv, rv); rv = FixupURLFunction::create(mMainConn); NS_ENSURE_SUCCESS(rv, rv); - rv = FrecencyNotificationFunction::create(mMainConn); - NS_ENSURE_SUCCESS(rv, rv); return NS_OK; } diff --git a/toolkit/components/places/History.cpp b/toolkit/components/places/History.cpp index 3e9d9bf1022a..33ea3bde3ce9 100644 --- a/toolkit/components/places/History.cpp +++ b/toolkit/components/places/History.cpp @@ -1185,10 +1185,7 @@ private: if (aPlace.placeId) { stmt = mHistory->GetStatement( "UPDATE moz_places " - "SET frecency = NOTIFY_FRECENCY(" - "CALCULATE_FRECENCY(:page_id), " - "url, guid, hidden, last_visit_date" - ") " + "SET frecency = CALCULATE_FRECENCY(:page_id) " "WHERE id = :page_id" ); NS_ENSURE_STATE(stmt); @@ -1198,9 +1195,7 @@ private: else { stmt = mHistory->GetStatement( "UPDATE moz_places " - "SET frecency = NOTIFY_FRECENCY(" - "CALCULATE_FRECENCY(id), url, guid, hidden, last_visit_date" - ") " + "SET frecency = CALCULATE_FRECENCY(id) " "WHERE url = :page_url" ); NS_ENSURE_STATE(stmt); @@ -2042,14 +2037,13 @@ History::InsertPlace(const VisitData& aPlace) NS_ENSURE_SUCCESS(rv, rv); rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("url"), aPlace.spec); NS_ENSURE_SUCCESS(rv, rv); - nsString title = aPlace.title; // Empty strings should have no title, just like nsNavHistory::SetPageTitle. - if (title.IsEmpty()) { + if (aPlace.title.IsEmpty()) { rv = stmt->BindNullByName(NS_LITERAL_CSTRING("title")); } else { - title.Assign(StringHead(aPlace.title, TITLE_LENGTH_MAX)); - rv = stmt->BindStringByName(NS_LITERAL_CSTRING("title"), title); + rv = stmt->BindStringByName(NS_LITERAL_CSTRING("title"), + StringHead(aPlace.title, TITLE_LENGTH_MAX)); } NS_ENSURE_SUCCESS(rv, rv); rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("typed"), aPlace.typed); @@ -2071,13 +2065,6 @@ History::InsertPlace(const VisitData& aPlace) rv = stmt->Execute(); NS_ENSURE_SUCCESS(rv, rv); - // Post an onFrecencyChanged observer notification. - const nsNavHistory* navHistory = nsNavHistory::GetConstHistoryService(); - NS_ENSURE_STATE(navHistory); - navHistory->DispatchFrecencyChangedNotification(aPlace.spec, frecency, guid, - aPlace.hidden, - aPlace.visitTime); - return NS_OK; } diff --git a/toolkit/components/places/SQLFunctions.cpp b/toolkit/components/places/SQLFunctions.cpp index 62ea36c130f7..f8fd439db9b1 100644 --- a/toolkit/components/places/SQLFunctions.cpp +++ b/toolkit/components/places/SQLFunctions.cpp @@ -749,63 +749,5 @@ namespace places { return NS_OK; } -//////////////////////////////////////////////////////////////////////////////// -//// Frecency Changed Notification Function - - /* static */ - nsresult - FrecencyNotificationFunction::create(mozIStorageConnection *aDBConn) - { - nsRefPtr function = - new FrecencyNotificationFunction(); - nsresult rv = aDBConn->CreateFunction( - NS_LITERAL_CSTRING("notify_frecency"), 5, function - ); - NS_ENSURE_SUCCESS(rv, rv); - - return NS_OK; - } - - NS_IMPL_ISUPPORTS1( - FrecencyNotificationFunction, - mozIStorageFunction - ) - - NS_IMETHODIMP - FrecencyNotificationFunction::OnFunctionCall(mozIStorageValueArray *aArgs, - nsIVariant **_result) - { - uint32_t numArgs; - nsresult rv = aArgs->GetNumEntries(&numArgs); - NS_ENSURE_SUCCESS(rv, rv); - MOZ_ASSERT(numArgs == 5); - - int32_t newFrecency = aArgs->AsInt32(0); - - nsAutoCString spec; - rv = aArgs->GetUTF8String(1, spec); - NS_ENSURE_SUCCESS(rv, rv); - - nsAutoCString guid; - rv = aArgs->GetUTF8String(2, guid); - NS_ENSURE_SUCCESS(rv, rv); - - bool hidden = static_cast(aArgs->AsInt32(3)); - PRTime lastVisitDate = static_cast(aArgs->AsInt64(4)); - - const nsNavHistory* navHistory = nsNavHistory::GetConstHistoryService(); - NS_ENSURE_STATE(navHistory); - navHistory->DispatchFrecencyChangedNotification(spec, newFrecency, guid, - hidden, lastVisitDate); - - nsCOMPtr result = - do_CreateInstance("@mozilla.org/variant;1"); - NS_ENSURE_STATE(result); - rv = result->SetAsInt32(newFrecency); - NS_ENSURE_SUCCESS(rv, rv); - NS_ADDREF(*_result = result); - return NS_OK; - } - } // namespace places } // namespace mozilla diff --git a/toolkit/components/places/SQLFunctions.h b/toolkit/components/places/SQLFunctions.h index 315ce2f4b89c..79f6218b1f24 100644 --- a/toolkit/components/places/SQLFunctions.h +++ b/toolkit/components/places/SQLFunctions.h @@ -280,43 +280,6 @@ public: static nsresult create(mozIStorageConnection *aDBConn); }; - -//////////////////////////////////////////////////////////////////////////////// -//// Frecency Changed Notification Function - -/** - * For a given place, posts a runnable to the main thread that calls - * onFrecencyChanged on nsNavHistory's nsINavHistoryObservers. The passed-in - * newFrecency value is returned unchanged. - * - * @param newFrecency - * The place's new frecency. - * @param url - * The place's URL. - * @param guid - * The place's GUID. - * @param hidden - * The place's hidden boolean. - * @param lastVisitDate - * The place's last visit date. - * @return newFrecency - */ -class FrecencyNotificationFunction MOZ_FINAL : public mozIStorageFunction -{ -public: - NS_DECL_THREADSAFE_ISUPPORTS - NS_DECL_MOZISTORAGEFUNCTION - - /** - * Registers the function with the specified database connection. - * - * @param aDBConn - * The database connection to register with. - */ - static nsresult create(mozIStorageConnection *aDBConn); -}; - - } // namespace places } // namespace storage diff --git a/toolkit/components/places/nsINavHistoryService.idl b/toolkit/components/places/nsINavHistoryService.idl index 7e375114b94e..1a584b8c45a1 100644 --- a/toolkit/components/places/nsINavHistoryService.idl +++ b/toolkit/components/places/nsINavHistoryService.idl @@ -610,7 +610,7 @@ interface nsINavHistoryResult : nsISupports * DANGER! If you are in the middle of a batch transaction, there may be a * database transaction active. You can still access the DB, but be careful. */ -[scriptable, uuid(0f0f45b0-13a1-44ae-a0ab-c6046ec6d4da)] +[scriptable, uuid(45e2970b-9b00-4473-9938-39d6beaf4248)] interface nsINavHistoryObserver : nsISupports { /** @@ -675,37 +675,6 @@ interface nsINavHistoryObserver : nsISupports in AString aPageTitle, in ACString aGUID); - /** - * Called when an individual page's frecency has changed. - * - * This is not called for pages whose frecencies change as the result of some - * large operation where some large or unknown number of frecencies change at - * once. Use onManyFrecenciesChanged to detect such changes. - * - * @param aURI - * The page's URI. - * @param aNewFrecency - * The page's new frecency. - * @param aGUID - * The page's GUID. - * @param aHidden - * True if the page is marked as hidden. - * @param aVisitDate - * The page's last visit date. - */ - void onFrecencyChanged(in nsIURI aURI, - in long aNewFrecency, - in ACString aGUID, - in boolean aHidden, - in PRTime aVisitDate); - - /** - * Called when the frecencies of many pages have changed at once. - * - * onFrecencyChanged is not called for each of those pages. - */ - void onManyFrecenciesChanged(); - /** * Removed by the user. */ diff --git a/toolkit/components/places/nsNavBookmarks.cpp b/toolkit/components/places/nsNavBookmarks.cpp index 7d4f2191d4b0..640483dd2672 100644 --- a/toolkit/components/places/nsNavBookmarks.cpp +++ b/toolkit/components/places/nsNavBookmarks.cpp @@ -2841,24 +2841,6 @@ nsNavBookmarks::OnTitleChanged(nsIURI* aURI, } -NS_IMETHODIMP -nsNavBookmarks::OnFrecencyChanged(nsIURI* aURI, - int32_t aNewFrecency, - const nsACString& aGUID, - bool aHidden, - PRTime aLastVisitDate) -{ - return NS_OK; -} - - -NS_IMETHODIMP -nsNavBookmarks::OnManyFrecenciesChanged() -{ - return NS_OK; -} - - NS_IMETHODIMP nsNavBookmarks::OnPageChanged(nsIURI* aURI, uint32_t aChangedAttribute, diff --git a/toolkit/components/places/nsNavHistory.cpp b/toolkit/components/places/nsNavHistory.cpp index f2d773032a3c..c48da35912d1 100644 --- a/toolkit/components/places/nsNavHistory.cpp +++ b/toolkit/components/places/nsNavHistory.cpp @@ -535,82 +535,6 @@ nsNavHistory::NotifyTitleChange(nsIURI* aURI, nsINavHistoryObserver, OnTitleChanged(aURI, aTitle, aGUID)); } -void -nsNavHistory::NotifyFrecencyChanged(nsIURI* aURI, - int32_t aNewFrecency, - const nsACString& aGUID, - bool aHidden, - PRTime aLastVisitDate) -{ - MOZ_ASSERT(!aGUID.IsEmpty()); - NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers, - nsINavHistoryObserver, - OnFrecencyChanged(aURI, aNewFrecency, aGUID, aHidden, - aLastVisitDate)); -} - -void -nsNavHistory::NotifyManyFrecenciesChanged() -{ - NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers, - nsINavHistoryObserver, - OnManyFrecenciesChanged()); -} - -namespace { - -class FrecencyNotification : public nsRunnable -{ -public: - FrecencyNotification(const nsACString& aSpec, - int32_t aNewFrecency, - const nsACString& aGUID, - bool aHidden, - PRTime aLastVisitDate) - : mSpec(aSpec) - , mNewFrecency(aNewFrecency) - , mGUID(aGUID) - , mHidden(aHidden) - , mLastVisitDate(aLastVisitDate) - { - } - - NS_IMETHOD Run() - { - MOZ_ASSERT(NS_IsMainThread(), "Must be called on the main thread"); - nsNavHistory* navHistory = nsNavHistory::GetHistoryService(); - if (navHistory) { - nsCOMPtr uri; - (void)NS_NewURI(getter_AddRefs(uri), mSpec); - navHistory->NotifyFrecencyChanged(uri, mNewFrecency, mGUID, mHidden, - mLastVisitDate); - } - return NS_OK; - } - -private: - nsCString mSpec; - int32_t mNewFrecency; - nsCString mGUID; - bool mHidden; - PRTime mLastVisitDate; -}; - -} // anonymous namespace - -void -nsNavHistory::DispatchFrecencyChangedNotification(const nsACString& aSpec, - int32_t aNewFrecency, - const nsACString& aGUID, - bool aHidden, - PRTime aLastVisitDate) const -{ - nsCOMPtr notif = new FrecencyNotification(aSpec, aNewFrecency, - aGUID, aHidden, - aLastVisitDate); - (void)NS_DispatchToMainThread(notif); -} - int32_t nsNavHistory::GetDaysOfHistory() { MOZ_ASSERT(NS_IsMainThread(), "This can only be called on the main thread"); @@ -1004,68 +928,32 @@ nsNavHistory::GetHasHistoryEntries(bool* aHasEntries) } -namespace { - -class InvalidateAllFrecenciesCallback : public AsyncStatementCallback -{ -public: - InvalidateAllFrecenciesCallback() - { - } - - NS_IMETHOD HandleCompletion(uint16_t aReason) - { - if (aReason == REASON_FINISHED) { - nsNavHistory *navHistory = nsNavHistory::GetHistoryService(); - NS_ENSURE_STATE(navHistory); - navHistory->NotifyManyFrecenciesChanged(); - } - return NS_OK; - } -}; - -} // anonymous namespace - nsresult nsNavHistory::invalidateFrecencies(const nsCString& aPlaceIdsQueryString) { // Exclude place: queries by setting their frecency to zero. - nsCString invalidFrecenciesSQLFragment( - "UPDATE moz_places SET frecency = " - ); - if (!aPlaceIdsQueryString.IsEmpty()) - invalidFrecenciesSQLFragment.AppendLiteral("NOTIFY_FRECENCY("); - invalidFrecenciesSQLFragment.AppendLiteral( - "(CASE " - "WHEN url BETWEEN 'place:' AND 'place;' " - "THEN 0 " - "ELSE -1 " - "END) " - ); - if (!aPlaceIdsQueryString.IsEmpty()) { - invalidFrecenciesSQLFragment.AppendLiteral( - ", url, guid, hidden, last_visit_date) " - ); - } - invalidFrecenciesSQLFragment.AppendLiteral( + nsAutoCString invalideFrecenciesSQLFragment( + "UPDATE moz_places SET frecency = (CASE " + "WHEN url BETWEEN 'place:' AND 'place;' " + "THEN 0 " + "ELSE -1 " + "END) " "WHERE frecency > 0 " ); + if (!aPlaceIdsQueryString.IsEmpty()) { - invalidFrecenciesSQLFragment.AppendLiteral("AND id IN("); - invalidFrecenciesSQLFragment.Append(aPlaceIdsQueryString); - invalidFrecenciesSQLFragment.AppendLiteral(")"); + invalideFrecenciesSQLFragment.AppendLiteral("AND id IN("); + invalideFrecenciesSQLFragment.Append(aPlaceIdsQueryString); + invalideFrecenciesSQLFragment.AppendLiteral(")"); } - nsRefPtr cb = - aPlaceIdsQueryString.IsEmpty() ? new InvalidateAllFrecenciesCallback() - : nullptr; nsCOMPtr stmt = mDB->GetAsyncStatement( - invalidFrecenciesSQLFragment + invalideFrecenciesSQLFragment ); NS_ENSURE_STATE(stmt); nsCOMPtr ps; - nsresult rv = stmt->ExecuteAsync(cb, getter_AddRefs(ps)); + nsresult rv = stmt->ExecuteAsync(nullptr, getter_AddRefs(ps)); NS_ENSURE_SUCCESS(rv, rv); return NS_OK; @@ -3190,30 +3078,6 @@ nsNavHistory::Observe(nsISupports *aSubject, const char *aTopic, } -namespace { - -class DecayFrecencyCallback : public AsyncStatementTelemetryTimer -{ -public: - DecayFrecencyCallback() - : AsyncStatementTelemetryTimer(Telemetry::PLACES_IDLE_FRECENCY_DECAY_TIME_MS) - { - } - - NS_IMETHOD HandleCompletion(uint16_t aReason) - { - (void)AsyncStatementTelemetryTimer::HandleCompletion(aReason); - if (aReason == REASON_FINISHED) { - nsNavHistory *navHistory = nsNavHistory::GetHistoryService(); - NS_ENSURE_STATE(navHistory); - navHistory->NotifyManyFrecenciesChanged(); - } - return NS_OK; - } -}; - -} // anonymous namespace - nsresult nsNavHistory::DecayFrecency() { @@ -3251,7 +3115,8 @@ nsNavHistory::DecayFrecency() deleteAdaptive.get() }; nsCOMPtr ps; - nsRefPtr cb = new DecayFrecencyCallback(); + nsRefPtr cb = + new AsyncStatementTelemetryTimer(Telemetry::PLACES_IDLE_FRECENCY_DECAY_TIME_MS); rv = mDB->MainConn()->ExecuteAsync(stmts, ArrayLength(stmts), cb, getter_AddRefs(ps)); NS_ENSURE_SUCCESS(rv, rv); @@ -4447,9 +4312,7 @@ nsNavHistory::UpdateFrecency(int64_t aPlaceId) { nsCOMPtr updateFrecencyStmt = mDB->GetAsyncStatement( "UPDATE moz_places " - "SET frecency = NOTIFY_FRECENCY(" - "CALCULATE_FRECENCY(:page_id), url, guid, hidden, last_visit_date" - ") " + "SET frecency = CALCULATE_FRECENCY(:page_id) " "WHERE id = :page_id" ); NS_ENSURE_STATE(updateFrecencyStmt); @@ -4482,31 +4345,6 @@ nsNavHistory::UpdateFrecency(int64_t aPlaceId) } -namespace { - -class FixInvalidFrecenciesCallback : public AsyncStatementCallbackNotifier -{ -public: - FixInvalidFrecenciesCallback() - : AsyncStatementCallbackNotifier(TOPIC_FRECENCY_UPDATED) - { - } - - NS_IMETHOD HandleCompletion(uint16_t aReason) - { - nsresult rv = AsyncStatementCallbackNotifier::HandleCompletion(aReason); - NS_ENSURE_SUCCESS(rv, rv); - if (aReason == REASON_FINISHED) { - nsNavHistory *navHistory = nsNavHistory::GetHistoryService(); - NS_ENSURE_STATE(navHistory); - navHistory->NotifyManyFrecenciesChanged(); - } - return NS_OK; - } -}; - -} // anonymous namespace - nsresult nsNavHistory::FixInvalidFrecencies() { @@ -4517,8 +4355,8 @@ nsNavHistory::FixInvalidFrecencies() ); NS_ENSURE_STATE(stmt); - nsRefPtr callback = - new FixInvalidFrecenciesCallback(); + nsRefPtr callback = + new AsyncStatementCallbackNotifier(TOPIC_FRECENCY_UPDATED); nsCOMPtr ps; (void)stmt->ExecuteAsync(callback, getter_AddRefs(ps)); diff --git a/toolkit/components/places/nsNavHistory.h b/toolkit/components/places/nsNavHistory.h index b2ebcec72b6a..149ce1cc282d 100644 --- a/toolkit/components/places/nsNavHistory.h +++ b/toolkit/components/places/nsNavHistory.h @@ -418,29 +418,6 @@ public: const nsString& title, const nsACString& aGUID); - /** - * Fires onFrecencyChanged event to nsINavHistoryService observers - */ - void NotifyFrecencyChanged(nsIURI* aURI, - int32_t aNewFrecency, - const nsACString& aGUID, - bool aHidden, - PRTime aLastVisitDate); - - /** - * Fires onManyFrecenciesChanged event to nsINavHistoryService observers - */ - void NotifyManyFrecenciesChanged(); - - /** - * Posts a runnable to the main thread that calls NotifyFrecencyChanged. - */ - void DispatchFrecencyChangedNotification(const nsACString& aSpec, - int32_t aNewFrecency, - const nsACString& aGUID, - bool aHidden, - PRTime aLastVisitDate) const; - bool isBatching() { return mBatchLevel > 0; } diff --git a/toolkit/components/places/nsNavHistoryResult.cpp b/toolkit/components/places/nsNavHistoryResult.cpp index 817aa90d0cb9..aa7b9d48ee5e 100644 --- a/toolkit/components/places/nsNavHistoryResult.cpp +++ b/toolkit/components/places/nsNavHistoryResult.cpp @@ -2614,24 +2614,6 @@ nsNavHistoryQueryResultNode::OnTitleChanged(nsIURI* aURI, } -NS_IMETHODIMP -nsNavHistoryQueryResultNode::OnFrecencyChanged(nsIURI* aURI, - int32_t aNewFrecency, - const nsACString& aGUID, - bool aHidden, - PRTime aLastVisitDate) -{ - return NS_OK; -} - - -NS_IMETHODIMP -nsNavHistoryQueryResultNode::OnManyFrecenciesChanged() -{ - return NS_OK; -} - - /** * Here, we can always live update by just deleting all occurrences of * the given URI. @@ -4680,24 +4662,6 @@ nsNavHistoryResult::OnTitleChanged(nsIURI* aURI, } -NS_IMETHODIMP -nsNavHistoryResult::OnFrecencyChanged(nsIURI* aURI, - int32_t aNewFrecency, - const nsACString& aGUID, - bool aHidden, - PRTime aLastVisitDate) -{ - return NS_OK; -} - - -NS_IMETHODIMP -nsNavHistoryResult::OnManyFrecenciesChanged() -{ - return NS_OK; -} - - NS_IMETHODIMP nsNavHistoryResult::OnDeleteURI(nsIURI *aURI, const nsACString& aGUID, diff --git a/toolkit/components/places/nsNavHistoryResult.h b/toolkit/components/places/nsNavHistoryResult.h index bac24871bb41..21aff6528363 100644 --- a/toolkit/components/places/nsNavHistoryResult.h +++ b/toolkit/components/places/nsNavHistoryResult.h @@ -64,10 +64,6 @@ private: NS_DECL_NSINAVBOOKMARKOBSERVER \ NS_IMETHOD OnTitleChanged(nsIURI* aURI, const nsAString& aPageTitle, \ const nsACString& aGUID); \ - NS_IMETHOD OnFrecencyChanged(nsIURI* aURI, int32_t aNewFrecency, \ - const nsACString& aGUID, bool aHidden, \ - PRTime aLastVisitDate); \ - NS_IMETHOD OnManyFrecenciesChanged(); \ NS_IMETHOD OnDeleteURI(nsIURI *aURI, const nsACString& aGUID, \ uint16_t aReason); \ NS_IMETHOD OnClearHistory(); \ diff --git a/toolkit/components/places/tests/unit/test_frecency_observers.js b/toolkit/components/places/tests/unit/test_frecency_observers.js deleted file mode 100644 index f027e98e57cc..000000000000 --- a/toolkit/components/places/tests/unit/test_frecency_observers.js +++ /dev/null @@ -1,85 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - http://creativecommons.org/publicdomain/zero/1.0/ */ - -function run_test() { - run_next_test(); -} - -// Each of these tests a path that triggers a frecency update. Together they -// hit all sites that update a frecency. - -// InsertVisitedURIs::UpdateFrecency and History::InsertPlace -add_task(function test_InsertVisitedURIs_UpdateFrecency_and_History_InsertPlace() { - // InsertPlace is at the end of a path that UpdateFrecency is also on, so kill - // two birds with one stone and expect two notifications. Trigger the path by - // adding a download. - let uri = NetUtil.newURI("http://example.com/a"); - Cc["@mozilla.org/browser/download-history;1"]. - getService(Ci.nsIDownloadHistory). - addDownload(uri); - yield Promise.all([onFrecencyChanged(uri), onFrecencyChanged(uri)]); -}); - -// nsNavHistory::UpdateFrecency -add_task(function test_nsNavHistory_UpdateFrecency() { - let bm = PlacesUtils.bookmarks; - let uri = NetUtil.newURI("http://example.com/b"); - bm.insertBookmark(bm.unfiledBookmarksFolder, uri, - Ci.nsINavBookmarksService.DEFAULT_INDEX, "test"); - yield onFrecencyChanged(uri); -}); - -// nsNavHistory::invalidateFrecencies for particular pages -add_task(function test_nsNavHistory_invalidateFrecencies_somePages() { - let uri = NetUtil.newURI("http://test-nsNavHistory-invalidateFrecencies-somePages.com/"); - // Bookmarking the URI is enough to add it to moz_places, and importantly, it - // means that removePagesFromHost doesn't remove it from moz_places, so its - // frecency is able to be changed. - let bm = PlacesUtils.bookmarks; - bm.insertBookmark(bm.unfiledBookmarksFolder, uri, - Ci.nsINavBookmarksService.DEFAULT_INDEX, "test"); - PlacesUtils.history.removePagesFromHost(uri.host, false); - yield onFrecencyChanged(uri); -}); - -// nsNavHistory::invalidateFrecencies for all pages -add_task(function test_nsNavHistory_invalidateFrecencies_allPages() { - PlacesUtils.history.removeAllPages(); - yield onManyFrecenciesChanged(); -}); - -// nsNavHistory::DecayFrecency and nsNavHistory::FixInvalidFrecencies -add_task(function test_nsNavHistory_DecayFrecency_and_nsNavHistory_FixInvalidFrecencies() { - // FixInvalidFrecencies is at the end of a path that DecayFrecency is also on, - // so expect two notifications. Trigger the path by making nsNavHistory - // observe the idle-daily notification. - PlacesUtils.history.QueryInterface(Ci.nsIObserver). - observe(null, "idle-daily", ""); - yield Promise.all([onManyFrecenciesChanged(), onManyFrecenciesChanged()]); -}); - -function onFrecencyChanged(expectedURI) { - let deferred = Promise.defer(); - let obs = new NavHistoryObserver(); - obs.onFrecencyChanged = - (uri, newFrecency, guid, hidden, visitDate) => { - PlacesUtils.history.removeObserver(obs); - do_check_true(!!uri); - do_check_true(uri.equals(expectedURI)); - deferred.resolve(); - }; - PlacesUtils.history.addObserver(obs, false); - return deferred.promise; -} - -function onManyFrecenciesChanged() { - let deferred = Promise.defer(); - let obs = new NavHistoryObserver(); - obs.onManyFrecenciesChanged = () => { - PlacesUtils.history.removeObserver(obs); - do_check_true(true); - deferred.resolve(); - }; - PlacesUtils.history.addObserver(obs, false); - return deferred.promise; -} diff --git a/toolkit/components/places/tests/unit/xpcshell.ini b/toolkit/components/places/tests/unit/xpcshell.ini index 3165288b9b1c..2389aff76b49 100644 --- a/toolkit/components/places/tests/unit/xpcshell.ini +++ b/toolkit/components/places/tests/unit/xpcshell.ini @@ -113,7 +113,6 @@ skip-if = true [test_null_interfaces.js] [test_onItemChanged_tags.js] [test_pageGuid_bookmarkGuid.js] -[test_frecency_observers.js] [test_placeURIs.js] [test_PlacesUtils_asyncGetBookmarkIds.js] [test_PlacesUtils_lazyobservers.js] diff --git a/toolkit/modules/BinarySearch.jsm b/toolkit/modules/BinarySearch.jsm deleted file mode 100644 index b07879b78eab..000000000000 --- a/toolkit/modules/BinarySearch.jsm +++ /dev/null @@ -1,74 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this file, - * You can obtain one at http://mozilla.org/MPL/2.0/. */ - -"use strict"; - -this.EXPORTED_SYMBOLS = [ - "BinarySearch", -]; - -this.BinarySearch = Object.freeze({ - - /** - * Returns the index of the given target in the given array or -1 if the - * target is not found. - * - * See search() for a description of this function's parameters. - * - * @return The index of `target` in `array` or -1 if `target` is not found. - */ - indexOf: function (array, target, comparator) { - let [found, idx] = this.search(array, target, comparator); - return found ? idx : -1; - }, - - /** - * Returns the index within the given array where the given target may be - * inserted to keep the array ordered. - * - * See search() for a description of this function's parameters. - * - * @return The index in `array` where `target` may be inserted to keep `array` - * ordered. - */ - insertionIndexOf: function (array, target, comparator) { - return this.search(array, target, comparator)[1]; - }, - - /** - * Searches for the given target in the given array. - * - * @param array - * An array whose elements are ordered by `comparator`. - * @param target - * The value to search for. - * @param comparator - * A function that takes two arguments and compares them, returning a - * negative number if the first should be ordered before the second, - * zero if the first and second have the same ordering, or a positive - * number if the second should be ordered before the first. The first - * argument is always `target`, and the second argument is a value - * from the array. - * @return An array with two elements. If `target` is found, the first - * element is true, and the second element is its index in the array. - * If `target` is not found, the first element is false, and the - * second element is the index where it may be inserted to keep the - * array ordered. - */ - search: function (array, target, comparator) { - let low = 0; - let high = array.length - 1; - while (low <= high) { - let mid = Math.floor((low + high) / 2); - let cmp = comparator(target, array[mid]); - if (cmp == 0) - return [true, mid]; - if (cmp < 0) - high = mid - 1; - else - low = mid + 1; - } - return [false, low]; - }, -}); diff --git a/toolkit/modules/NewTabUtils.jsm b/toolkit/modules/NewTabUtils.jsm index 5fbc3119aa92..45a077db70db 100644 --- a/toolkit/modules/NewTabUtils.jsm +++ b/toolkit/modules/NewTabUtils.jsm @@ -19,13 +19,6 @@ XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils", XPCOMUtils.defineLazyModuleGetter(this, "PageThumbs", "resource://gre/modules/PageThumbs.jsm"); -XPCOMUtils.defineLazyModuleGetter(this, "BinarySearch", - "resource://gre/modules/BinarySearch.jsm"); - -XPCOMUtils.defineLazyGetter(this, "Timer", () => { - return Cu.import("resource://gre/modules/Timer.jsm", {}); -}); - XPCOMUtils.defineLazyGetter(this, "gPrincipal", function () { let uri = Services.io.newURI("about:newtab", null, null); return Services.scriptSecurityManager.getNoAppCodebasePrincipal(uri); @@ -51,18 +44,12 @@ const PREF_NEWTAB_ROWS = "browser.newtabpage.rows"; // The preference that tells the number of columns of the newtab grid. const PREF_NEWTAB_COLUMNS = "browser.newtabpage.columns"; -// The maximum number of results PlacesProvider retrieves from history. +// The maximum number of results we want to retrieve from history. const HISTORY_RESULTS_LIMIT = 100; -// The maximum number of links Links.getLinks will return. -const LINKS_GET_LINKS_LIMIT = 100; - // The gather telemetry topic. const TOPIC_GATHER_TELEMETRY = "gather-telemetry"; -// The amount of time we wait while coalescing updates for hidden pages. -const SCHEDULE_UPDATE_TIMEOUT_MS = 1000; - /** * Calculate the MD5 hash for a string. * @param aValue @@ -257,34 +244,14 @@ let AllPages = { /** * Updates all currently active pages but the given one. * @param aExceptPage The page to exclude from updating. - * @param aHiddenPagesOnly If true, only pages hidden in the preloader are - * updated. */ - update: function AllPages_update(aExceptPage, aHiddenPagesOnly=false) { + update: function AllPages_update(aExceptPage) { this._pages.forEach(function (aPage) { if (aExceptPage != aPage) - aPage.update(aHiddenPagesOnly); + aPage.update(); }); }, - /** - * Many individual link changes may happen in a small amount of time over - * multiple turns of the event loop. This method coalesces updates by waiting - * a small amount of time before updating hidden pages. - */ - scheduleUpdateForHiddenPages: function AllPages_scheduleUpdateForHiddenPages() { - if (!this._scheduleUpdateTimeout) { - this._scheduleUpdateTimeout = Timer.setTimeout(() => { - delete this._scheduleUpdateTimeout; - this.update(null, true); - }, SCHEDULE_UPDATE_TIMEOUT_MS); - } - }, - - get updateScheduledForHiddenPages() { - return !!this._scheduleUpdateTimeout; - }, - /** * Implements the nsIObserver interface to get notified when the preference * value changes or when a new copy of a page thumbnail is available. @@ -537,25 +504,13 @@ let BlockedLinks = { * the history to retrieve the most frequently visited sites. */ let PlacesProvider = { - /** - * Set this to change the maximum number of links the provider will provide. - */ - maxNumLinks: HISTORY_RESULTS_LIMIT, - - /** - * Must be called before the provider is used. - */ - init: function PlacesProvider_init() { - PlacesUtils.history.addObserver(this, true); - }, - /** * Gets the current set of links delivered by this provider. * @param aCallback The function that the array of links is passed to. */ getLinks: function PlacesProvider_getLinks(aCallback) { let options = PlacesUtils.history.getNewQueryOptions(); - options.maxResults = this.maxNumLinks; + options.maxResults = HISTORY_RESULTS_LIMIT; // Sort by frecency, descending. options.sortingMode = Ci.nsINavHistoryQueryOptions.SORT_BY_FRECENCY_DESCENDING @@ -570,14 +525,7 @@ let PlacesProvider = { let url = row.getResultByIndex(1); if (LinkChecker.checkLoadURI(url)) { let title = row.getResultByIndex(2); - let frecency = row.getResultByIndex(12); - let lastVisitDate = row.getResultByIndex(5); - links.push({ - url: url, - title: title, - frecency: frecency, - lastVisitDate: lastVisitDate, - }); + links.push({url: url, title: title}); } } }, @@ -588,26 +536,6 @@ let PlacesProvider = { }, handleCompletion: function (aReason) { - // The Places query breaks ties in frecency by place ID descending, but - // that's different from how Links.compareLinks breaks ties, because - // compareLinks doesn't have access to place IDs. It's very important - // that the initial list of links is sorted in the same order imposed by - // compareLinks, because Links uses compareLinks to perform binary - // searches on the list. So, ensure the list is so ordered. - let i = 1; - let outOfOrder = []; - while (i < links.length) { - if (Links.compareLinks(links[i - 1], links[i]) > 0) - outOfOrder.push(links.splice(i, 1)[0]); - else - i++; - } - for (let link of outOfOrder) { - i = BinarySearch.insertionIndexOf(links, link, - Links.compareLinks.bind(Links)); - links.splice(i, 0, link); - } - aCallback(links); } }; @@ -616,116 +544,28 @@ let PlacesProvider = { let query = PlacesUtils.history.getNewQuery(); let db = PlacesUtils.history.QueryInterface(Ci.nsPIPlacesDatabase); db.asyncExecuteLegacyQueries([query], 1, options, callback); - }, - - /** - * Registers an object that will be notified when the provider's links change. - * @param aObserver An object with the following optional properties: - * * onLinkChanged: A function that's called when a single link - * changes. It's passed the provider and the link object. Only the - * link's `url` property is guaranteed to be present. If its `title` - * property is present, then its title has changed, and the - * property's value is the new title. If any sort properties are - * present, then its position within the provider's list of links may - * have changed, and the properties' values are the new sort-related - * values. Note that this link may not necessarily have been present - * in the lists returned from any previous calls to getLinks. - * * onManyLinksChanged: A function that's called when many links - * change at once. It's passed the provider. You should call - * getLinks to get the provider's new list of links. - */ - addObserver: function PlacesProvider_addObserver(aObserver) { - this._observers.push(aObserver); - }, - - _observers: [], - - /** - * Called by the history service. - */ - onFrecencyChanged: function PlacesProvider_onFrecencyChanged(aURI, aNewFrecency, aGUID, aHidden, aLastVisitDate) { - // The implementation of the query in getLinks excludes hidden and - // unvisited pages, so it's important to exclude them here, too. - if (!aHidden && aLastVisitDate) { - this._callObservers("onLinkChanged", { - url: aURI.spec, - frecency: aNewFrecency, - lastVisitDate: aLastVisitDate, - }); - } - }, - - /** - * Called by the history service. - */ - onManyFrecenciesChanged: function PlacesProvider_onManyFrecenciesChanged() { - this._callObservers("onManyLinksChanged"); - }, - - /** - * Called by the history service. - */ - onTitleChanged: function PlacesProvider_onTitleChanged(aURI, aNewTitle, aGUID) { - this._callObservers("onLinkChanged", { - url: aURI.spec, - title: aNewTitle - }); - }, - - _callObservers: function PlacesProvider__callObservers(aMethodName, aArg) { - for (let obs of this._observers) { - if (obs[aMethodName]) { - try { - obs[aMethodName](this, aArg); - } catch (err) { - Cu.reportError(err); - } - } - } - }, - - QueryInterface: XPCOMUtils.generateQI([Ci.nsINavHistoryObserver, - Ci.nsISupportsWeakReference]), + } }; /** * Singleton that provides access to all links contained in the grid (including - * the ones that don't fit on the grid). A link is a plain object that looks - * like this: + * the ones that don't fit on the grid). A link is a plain object with title + * and url properties. * - * { - * url: "http://www.mozilla.org/", - * title: "Mozilla", - * frecency: 1337, - * lastVisitDate: 1394678824766431, - * } + * Example: + * + * {url: "http://www.mozilla.org/", title: "Mozilla"} */ let Links = { /** - * The maximum number of links returned by getLinks. + * The links cache. */ - maxNumLinks: LINKS_GET_LINKS_LIMIT, + _links: null, /** - * The link providers. + * The default provider for links. */ - _providers: new Set(), - - /** - * A mapping from each provider to an object { sortedLinks, linkMap }. - * sortedLinks is the cached, sorted array of links for the provider. linkMap - * is a Map from link URLs to link objects. - */ - _providerLinks: new Map(), - - /** - * The properties of link objects used to sort them. - */ - _sortProperties: [ - "frecency", - "lastVisitDate", - "url", - ], + _provider: PlacesProvider, /** * List of callbacks waiting for the cache to be populated. @@ -733,26 +573,7 @@ let Links = { _populateCallbacks: [], /** - * Adds a link provider. - * @param aProvider The link provider. - */ - addProvider: function Links_addProvider(aProvider) { - this._providers.add(aProvider); - aProvider.addObserver(this); - }, - - /** - * Removes a link provider. - * @param aProvider The link provider. - */ - removeProvider: function Links_removeProvider(aProvider) { - if (!this._providers.delete(aProvider)) - throw new Error("Unknown provider"); - this._providerLinks.delete(aProvider); - }, - - /** - * Populates the cache with fresh links from the providers. + * Populates the cache with fresh links from the current provider. * @param aCallback The callback to call when finished (optional). * @param aForce When true, populates the cache even when it's already filled. */ @@ -780,15 +601,16 @@ let Links = { } } - let numProvidersRemaining = this._providers.size; - for (let provider of this._providers) { - this._populateProviderCache(provider, () => { - if (--numProvidersRemaining == 0) - executeCallbacks(); - }, aForce); - } + if (this._links && !aForce) { + executeCallbacks(); + } else { + this._provider.getLinks(function (aLinks) { + this._links = aLinks; + executeCallbacks(); + }.bind(this)); - this._addObserver(); + this._addObserver(); + } }, /** @@ -797,10 +619,9 @@ let Links = { */ getLinks: function Links_getLinks() { let pinnedLinks = Array.slice(PinnedLinks.links); - let links = this._getMergedProviderLinks(); // Filter blocked and pinned links. - links = links.filter(function (link) { + let links = (this._links || []).filter(function (link) { return !BlockedLinks.isBlocked(link) && !PinnedLinks.isPinned(link); }); @@ -820,186 +641,7 @@ let Links = { * Resets the links cache. */ resetCache: function Links_resetCache() { - this._providerLinks.clear(); - }, - - /** - * Compares two links. - * @param aLink1 The first link. - * @param aLink2 The second link. - * @return A negative number if aLink1 is ordered before aLink2, zero if - * aLink1 and aLink2 have the same ordering, or a positive number if - * aLink1 is ordered after aLink2. - */ - compareLinks: function Links_compareLinks(aLink1, aLink2) { - for (let prop of this._sortProperties) { - if (!(prop in aLink1) || !(prop in aLink2)) - throw new Error("Comparable link missing required property: " + prop); - } - return aLink2.frecency - aLink1.frecency || - aLink2.lastVisitDate - aLink1.lastVisitDate || - aLink1.url.localeCompare(aLink2.url); - }, - - /** - * Calls getLinks on the given provider and populates our cache for it. - * @param aProvider The provider whose cache will be populated. - * @param aCallback The callback to call when finished. - * @param aForce When true, populates the provider's cache even when it's - * already filled. - */ - _populateProviderCache: function Links_populateProviderCache(aProvider, aCallback, aForce) { - if (this._providerLinks.has(aProvider) && !aForce) { - aCallback(); - } else { - aProvider.getLinks(links => { - // Filter out null and undefined links so we don't have to deal with - // them in getLinks when merging links from providers. - links = links.filter((link) => !!link); - this._providerLinks.set(aProvider, { - sortedLinks: links, - linkMap: links.reduce((map, link) => { - map.set(link.url, link); - return map; - }, new Map()), - }); - aCallback(); - }); - } - }, - - /** - * Merges the cached lists of links from all providers whose lists are cached. - * @return The merged list. - */ - _getMergedProviderLinks: function Links__getMergedProviderLinks() { - // Build a list containing a copy of each provider's sortedLinks list. - let linkLists = []; - for (let links of this._providerLinks.values()) { - linkLists.push(links.sortedLinks.slice()); - } - - function getNextLink() { - let minLinks = null; - for (let links of linkLists) { - if (links.length && - (!minLinks || Links.compareLinks(links[0], minLinks[0]) < 0)) - minLinks = links; - } - return minLinks ? minLinks.shift() : null; - } - - let finalLinks = []; - for (let nextLink = getNextLink(); - nextLink && finalLinks.length < this.maxNumLinks; - nextLink = getNextLink()) { - finalLinks.push(nextLink); - } - - return finalLinks; - }, - - /** - * Called by a provider to notify us when a single link changes. - * @param aProvider The provider whose link changed. - * @param aLink The link that changed. If the link is new, it must have all - * of the _sortProperties. Otherwise, it may have as few or as - * many as is convenient. - */ - onLinkChanged: function Links_onLinkChanged(aProvider, aLink) { - if (!("url" in aLink)) - throw new Error("Changed links must have a url property"); - - let links = this._providerLinks.get(aProvider); - if (!links) - // This is not an error, it just means that between the time the provider - // was added and the future time we call getLinks on it, it notified us of - // a change. - return; - - let { sortedLinks, linkMap } = links; - - // Nothing to do if the list is full and the link isn't in it and shouldn't - // be in it. - if (!linkMap.has(aLink.url) && - sortedLinks.length && - sortedLinks.length == aProvider.maxNumLinks) { - let lastLink = sortedLinks[sortedLinks.length - 1]; - if (this.compareLinks(lastLink, aLink) < 0) - return; - } - - let updatePages = false; - - // Update the title in O(1). - if ("title" in aLink) { - let link = linkMap.get(aLink.url); - if (link && link.title != aLink.title) { - link.title = aLink.title; - updatePages = true; - } - } - - // Update the link's position in O(lg n). - if (this._sortProperties.some((prop) => prop in aLink)) { - let link = linkMap.get(aLink.url); - if (link) { - // The link is already in the list. - let idx = this._indexOf(sortedLinks, link); - if (idx < 0) - throw new Error("Link should be in _sortedLinks if in _linkMap"); - sortedLinks.splice(idx, 1); - for (let prop of this._sortProperties) { - if (prop in aLink) - link[prop] = aLink[prop]; - } - } - else { - // The link is new. - for (let prop of this._sortProperties) { - if (!(prop in aLink)) - throw new Error("New link missing required sort property: " + prop); - } - // Copy the link object so that if the caller changes it, it doesn't - // screw up our bookkeeping. - link = {}; - for (let [prop, val] of Iterator(aLink)) { - link[prop] = val; - } - linkMap.set(link.url, link); - } - let idx = this._insertionIndexOf(sortedLinks, link); - sortedLinks.splice(idx, 0, link); - if (sortedLinks.length > aProvider.maxNumLinks) { - let lastLink = sortedLinks.pop(); - linkMap.delete(lastLink.url); - } - updatePages = true; - } - - if (updatePages) - AllPages.scheduleUpdateForHiddenPages(); - }, - - /** - * Called by a provider to notify us when many links change. - */ - onManyLinksChanged: function Links_onManyLinksChanged(aProvider) { - this._populateProviderCache(aProvider, () => { - AllPages.scheduleUpdateForHiddenPages(); - }, true); - }, - - _indexOf: function Links__indexOf(aArray, aLink) { - return this._binsearch(aArray, aLink, "indexOf"); - }, - - _insertionIndexOf: function Links__insertionIndexOf(aArray, aLink) { - return this._binsearch(aArray, aLink, "insertionIndexOf"); - }, - - _binsearch: function Links__binsearch(aArray, aLink, aMethod) { - return BinarySearch[aMethod](aArray, aLink, this.compareLinks.bind(this)); + this._links = null; }, /** @@ -1012,7 +654,7 @@ let Links = { if (AllPages.length && AllPages.enabled) this.populateCache(function () { AllPages.update() }, true); else - this.resetCache(); + this._links = null; }, /** @@ -1132,20 +774,11 @@ this.NewTabUtils = { _initialized: false, init: function NewTabUtils_init() { - if (this.initWithoutProviders()) { - PlacesProvider.init(); - Links.addProvider(PlacesProvider); - } - }, - - initWithoutProviders: function NewTabUtils_initWithoutProviders() { if (!this._initialized) { this._initialized = true; ExpirationFilter.init(); Telemetry.init(); - return true; } - return false; }, /** diff --git a/toolkit/modules/moz.build b/toolkit/modules/moz.build index bd3028e51fe9..ce51fa079139 100644 --- a/toolkit/modules/moz.build +++ b/toolkit/modules/moz.build @@ -11,7 +11,6 @@ MOCHITEST_CHROME_MANIFESTS += ['tests/chrome/chrome.ini'] EXTRA_JS_MODULES += [ 'AsyncShutdown.jsm', - 'BinarySearch.jsm', 'BrowserUtils.jsm', 'CharsetMenu.jsm', 'debug.js', diff --git a/toolkit/modules/tests/xpcshell/test_BinarySearch.js b/toolkit/modules/tests/xpcshell/test_BinarySearch.js deleted file mode 100644 index 1601bffb194e..000000000000 --- a/toolkit/modules/tests/xpcshell/test_BinarySearch.js +++ /dev/null @@ -1,81 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - * http://creativecommons.org/publicdomain/zero/1.0/ */ - -Components.utils.import("resource://gre/modules/BinarySearch.jsm"); - -function run_test() { - // empty array - ok([], 1, false, 0); - - // one-element array - ok([2], 2, true, 0); - ok([2], 1, false, 0); - ok([2], 3, false, 1); - - // two-element array - ok([2, 4], 2, true, 0); - ok([2, 4], 4, true, 1); - ok([2, 4], 1, false, 0); - ok([2, 4], 3, false, 1); - ok([2, 4], 5, false, 2); - - // three-element array - ok([2, 4, 6], 2, true, 0); - ok([2, 4, 6], 4, true, 1); - ok([2, 4, 6], 6, true, 2); - ok([2, 4, 6], 1, false, 0); - ok([2, 4, 6], 3, false, 1); - ok([2, 4, 6], 5, false, 2); - ok([2, 4, 6], 7, false, 3); - - // duplicates - ok([2, 2], 2, true, 0); - ok([2, 2], 1, false, 0); - ok([2, 2], 3, false, 2); - - // duplicates on the left - ok([2, 2, 4], 2, true, 1); - ok([2, 2, 4], 4, true, 2); - ok([2, 2, 4], 1, false, 0); - ok([2, 2, 4], 3, false, 2); - ok([2, 2, 4], 5, false, 3); - - // duplicates on the right - ok([2, 4, 4], 2, true, 0); - ok([2, 4, 4], 4, true, 1); - ok([2, 4, 4], 1, false, 0); - ok([2, 4, 4], 3, false, 1); - ok([2, 4, 4], 5, false, 3); - - // duplicates in the middle - ok([2, 4, 4, 6], 2, true, 0); - ok([2, 4, 4, 6], 4, true, 1); - ok([2, 4, 4, 6], 6, true, 3); - ok([2, 4, 4, 6], 1, false, 0); - ok([2, 4, 4, 6], 3, false, 1); - ok([2, 4, 4, 6], 5, false, 3); - ok([2, 4, 4, 6], 7, false, 4); - - // duplicates all around - ok([2, 2, 4, 4, 6, 6], 2, true, 0); - ok([2, 2, 4, 4, 6, 6], 4, true, 2); - ok([2, 2, 4, 4, 6, 6], 6, true, 4); - ok([2, 2, 4, 4, 6, 6], 1, false, 0); - ok([2, 2, 4, 4, 6, 6], 3, false, 2); - ok([2, 2, 4, 4, 6, 6], 5, false, 4); - ok([2, 2, 4, 4, 6, 6], 7, false, 6); -} - -function ok(array, target, expectedFound, expectedIdx) { - let [found, idx] = BinarySearch.search(array, target, cmp); - do_check_eq(found, expectedFound); - do_check_eq(idx, expectedIdx); - - idx = expectedFound ? expectedIdx : -1; - do_check_eq(BinarySearch.indexOf(array, target, cmp), idx); - do_check_eq(BinarySearch.insertionIndexOf(array, target, cmp), expectedIdx); -} - -function cmp(num1, num2) { - return num1 - num2; -} diff --git a/toolkit/modules/tests/xpcshell/test_NewTabUtils.js b/toolkit/modules/tests/xpcshell/test_NewTabUtils.js deleted file mode 100644 index b8c6856c0ec4..000000000000 --- a/toolkit/modules/tests/xpcshell/test_NewTabUtils.js +++ /dev/null @@ -1,176 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - http://creativecommons.org/publicdomain/zero/1.0/ */ - -// See also browser/base/content/test/newtab/. - -const { classes: Cc, interfaces: Ci, results: Cr, utils: Cu } = Components; -Cu.import("resource://gre/modules/NewTabUtils.jsm"); -Cu.import("resource://gre/modules/Promise.jsm"); - -function run_test() { - run_next_test(); -} - -add_test(function multipleProviders() { - // Make each provider generate NewTabUtils.links.maxNumLinks links to check - // that no more than maxNumLinks are actually returned in the merged list. - let evenLinks = makeLinks(0, 2 * NewTabUtils.links.maxNumLinks, 2); - let evenProvider = new TestProvider(done => done(evenLinks)); - let oddLinks = makeLinks(0, 2 * NewTabUtils.links.maxNumLinks - 1, 2); - let oddProvider = new TestProvider(done => done(oddLinks)); - - NewTabUtils.initWithoutProviders(); - NewTabUtils.links.addProvider(evenProvider); - NewTabUtils.links.addProvider(oddProvider); - - // This is sync since the providers' getLinks are sync. - NewTabUtils.links.populateCache(function () {}, false); - - let links = NewTabUtils.links.getLinks(); - let expectedLinks = makeLinks(NewTabUtils.links.maxNumLinks, - 2 * NewTabUtils.links.maxNumLinks, - 1); - do_check_eq(links.length, NewTabUtils.links.maxNumLinks); - do_check_links(links, expectedLinks); - - NewTabUtils.links.removeProvider(evenProvider); - NewTabUtils.links.removeProvider(oddProvider); - run_next_test(); -}); - -add_test(function changeLinks() { - let expectedLinks = makeLinks(0, 20, 2); - let provider = new TestProvider(done => done(expectedLinks)); - - NewTabUtils.initWithoutProviders(); - NewTabUtils.links.addProvider(provider); - - // This is sync since the provider's getLinks is sync. - NewTabUtils.links.populateCache(function () {}, false); - - do_check_links(NewTabUtils.links.getLinks(), expectedLinks); - - // Notify of a new link. - let newLink = { - url: "http://example.com/19", - title: "My frecency is 19", - frecency: 19, - lastVisitDate: 0, - }; - expectedLinks.splice(1, 0, newLink); - provider.notifyLinkChanged(newLink); - do_check_links(NewTabUtils.links.getLinks(), expectedLinks); - - // Notify of a link that's changed sort criteria. - newLink.frecency = 17; - expectedLinks.splice(1, 1); - expectedLinks.splice(2, 0, newLink); - provider.notifyLinkChanged({ - url: newLink.url, - frecency: 17, - }); - do_check_links(NewTabUtils.links.getLinks(), expectedLinks); - - // Notify of a link that's changed title. - newLink.title = "My frecency is now 17"; - provider.notifyLinkChanged({ - url: newLink.url, - title: newLink.title, - }); - do_check_links(NewTabUtils.links.getLinks(), expectedLinks); - - // Notify of a new link again, but this time make it overflow maxNumLinks. - provider.maxNumLinks = expectedLinks.length; - newLink = { - url: "http://example.com/21", - frecency: 21, - lastVisitDate: 0, - }; - expectedLinks.unshift(newLink); - expectedLinks.pop(); - do_check_eq(expectedLinks.length, provider.maxNumLinks); // Sanity check. - provider.notifyLinkChanged(newLink); - do_check_links(NewTabUtils.links.getLinks(), expectedLinks); - - // Notify of many links changed. - expectedLinks = makeLinks(0, 3, 1); - provider.notifyManyLinksChanged(); - // NewTabUtils.links will now repopulate its cache, which is sync since - // the provider's getLinks is sync. - do_check_links(NewTabUtils.links.getLinks(), expectedLinks); - - NewTabUtils.links.removeProvider(provider); - run_next_test(); -}); - -add_task(function oneProviderAlreadyCached() { - let links1 = makeLinks(0, 10, 1); - let provider1 = new TestProvider(done => done(links1)); - - NewTabUtils.initWithoutProviders(); - NewTabUtils.links.addProvider(provider1); - - // This is sync since the provider's getLinks is sync. - NewTabUtils.links.populateCache(function () {}, false); - do_check_links(NewTabUtils.links.getLinks(), links1); - - let links2 = makeLinks(10, 20, 1); - let provider2 = new TestProvider(done => done(links2)); - NewTabUtils.links.addProvider(provider2); - - NewTabUtils.links.populateCache(function () {}, false); - do_check_links(NewTabUtils.links.getLinks(), links2.concat(links1)); - - NewTabUtils.links.removeProvider(provider1); - NewTabUtils.links.removeProvider(provider2); -}); - -function TestProvider(getLinksFn) { - this.getLinks = getLinksFn; - this._observers = new Set(); -} - -TestProvider.prototype = { - addObserver: function (observer) { - this._observers.add(observer); - }, - notifyLinkChanged: function (link) { - this._notifyObservers("onLinkChanged", link); - }, - notifyManyLinksChanged: function () { - this._notifyObservers("onManyLinksChanged"); - }, - _notifyObservers: function (observerMethodName, arg) { - for (let obs of this._observers) { - if (obs[observerMethodName]) - obs[observerMethodName](this, arg); - } - }, -}; - -function do_check_links(actualLinks, expectedLinks) { - do_check_true(Array.isArray(actualLinks)); - do_check_eq(actualLinks.length, expectedLinks.length); - for (let i = 0; i < expectedLinks.length; i++) { - let expected = expectedLinks[i]; - let actual = actualLinks[i]; - do_check_eq(actual.url, expected.url); - do_check_eq(actual.title, expected.title); - do_check_eq(actual.frecency, expected.frecency); - do_check_eq(actual.lastVisitDate, expected.lastVisitDate); - } -} - -function makeLinks(frecRangeStart, frecRangeEnd, step) { - let links = []; - // Remember, links are ordered by frecency descending. - for (let i = frecRangeEnd; i > frecRangeStart; i -= step) { - links.push({ - url: "http://example.com/" + i, - title: "My frecency is " + i, - frecency: i, - lastVisitDate: 0, - }); - } - return links; -} diff --git a/toolkit/modules/tests/xpcshell/xpcshell.ini b/toolkit/modules/tests/xpcshell/xpcshell.ini index 7c85f29a1622..c4bd72cf2b1e 100644 --- a/toolkit/modules/tests/xpcshell/xpcshell.ini +++ b/toolkit/modules/tests/xpcshell/xpcshell.ini @@ -8,14 +8,12 @@ support-files = zips/zen.zip [test_AsyncShutdown.js] -[test_BinarySearch.js] [test_DeferredTask.js] [test_dict.js] [test_DirectoryLinksProvider.js] [test_FileUtils.js] [test_Http.js] [test_Log.js] -[test_NewTabUtils.js] [test_PermissionsUtils.js] [test_Preferences.js] [test_Promise.js]