From 75a804ecf604d7921e1ee6727196d7fc70c989a4 Mon Sep 17 00:00:00 2001 From: Marco Bonardo Date: Sun, 19 Nov 2017 21:58:14 +0100 Subject: [PATCH] Bug 1415908 - Intermittent failure (nsIAutoCompleteController.getCommentAt) in browser_ext_omnibox.js. r=adw Fixes a timing bug where in certain moments matchCount may not be in sync with the current search status, due to previous results not being cleared immediately. Still delays tree updates to avoid UI flickering. Fixes a theorical timing issue in unifiedComplete where a stopped search could notify a result. Removes the no more used OnUpdateSearchResult API. MozReview-Commit-ID: COoIN4oQT4v --HG-- extra : rebase_source : 1676a2ff0da45c2ec95ffda90154ab9c26bdf8fb --- .../autocomplete/nsAutoCompleteController.cpp | 55 ++++++++++--------- .../autocomplete/nsAutoCompleteController.h | 4 +- .../autocomplete/nsIAutoCompleteSearch.idl | 8 --- toolkit/components/places/UnifiedComplete.js | 2 + 4 files changed, 34 insertions(+), 35 deletions(-) diff --git a/toolkit/components/autocomplete/nsAutoCompleteController.cpp b/toolkit/components/autocomplete/nsAutoCompleteController.cpp index 42dc709cac89..3d4d70ab910b 100644 --- a/toolkit/components/autocomplete/nsAutoCompleteController.cpp +++ b/toolkit/components/autocomplete/nsAutoCompleteController.cpp @@ -71,7 +71,7 @@ nsAutoCompleteController::nsAutoCompleteController() : mRowCount(0), mSearchesOngoing(0), mSearchesFailed(0), - mFirstSearchResult(false), + mDelayedRowCountDelta(0), mImmediateSearchesCount(0), mCompletedSelectionIndex(-1) { @@ -216,6 +216,7 @@ nsAutoCompleteController::ResetInternalState() mProhibitAutoFill = false; mSearchStatus = nsIAutoCompleteController::STATUS_NONE; mRowCount = 0; + mDelayedRowCountDelta = 0; mCompletedSelectionIndex = -1; return NS_OK; @@ -871,26 +872,15 @@ nsAutoCompleteController::HandleSearchResult(nsIAutoCompleteSearch *aSearch, //////////////////////////////////////////////////////////////////////// //// nsIAutoCompleteObserver -NS_IMETHODIMP -nsAutoCompleteController::OnUpdateSearchResult(nsIAutoCompleteSearch *aSearch, nsIAutoCompleteResult* aResult) -{ - MOZ_ASSERT(mSearches.Contains(aSearch)); - - ClearResults(); - HandleSearchResult(aSearch, aResult); - return NS_OK; -} - NS_IMETHODIMP nsAutoCompleteController::OnSearchResult(nsIAutoCompleteSearch *aSearch, nsIAutoCompleteResult* aResult) { MOZ_ASSERT(mSearchesOngoing > 0 && mSearches.Contains(aSearch)); - // If this is the first search result we are processing - // we should clear out the previously cached results. - if (mFirstSearchResult) { - ClearResults(); - mFirstSearchResult = false; + // Update the tree if necessary. + if (mTree && mDelayedRowCountDelta != 0) { + mTree->RowCountChanged(0, mDelayedRowCountDelta); + mDelayedRowCountDelta = 0; } uint16_t result = 0; @@ -1220,16 +1210,18 @@ nsAutoCompleteController::BeforeSearches() mSearchStatus = nsIAutoCompleteController::STATUS_SEARCHING; mDefaultIndexCompleted = false; - // The first search result will clear mResults array, though we should pass - // the previous result to each search to allow them to reuse it. So we - // temporarily cache current results till AfterSearches(). + // ClearResults will clear the mResults array, but we should pass the previous + // result to each search to allow reusing it. So we temporarily cache the + // current results until AfterSearches(). if (!mResultCache.AppendObjects(mResults)) { return NS_ERROR_OUT_OF_MEMORY; } - + // The rowCountChanged notification is sent later, when the first result + // from the search arrives, to avoid flickering in the tree contents. + mDelayedRowCountDelta = 0; + ClearResults(true); mSearchesOngoing = mSearches.Length(); mSearchesFailed = 0; - mFirstSearchResult = true; // notify the input that the search is beginning mInput->OnSearchBegin(); @@ -1731,6 +1723,12 @@ nsAutoCompleteController::PostSearchCleanup() uint32_t minResults; input->GetMinResultsForPopup(&minResults); + // Apply a pending rowCountChanged. + if (mTree && mDelayedRowCountDelta != 0) { + mTree->RowCountChanged(0, mDelayedRowCountDelta); + // mDelayedRowCountDelta will be reset by the next search. + } + if (mRowCount || minResults == 0) { OpenPopup(); if (mRowCount) @@ -1749,15 +1747,22 @@ nsAutoCompleteController::PostSearchCleanup() } nsresult -nsAutoCompleteController::ClearResults() +nsAutoCompleteController::ClearResults(bool aIsSearching) { int32_t oldRowCount = mRowCount; mRowCount = 0; mResults.Clear(); if (oldRowCount != 0) { - if (mTree) - mTree->RowCountChanged(0, -oldRowCount); - else if (mInput) { + if (mTree) { + if (aIsSearching) { + // Delay the notification, so the tree provides a smoother transition to + // the new result. It will be handled as soon as we add the first result. + mDelayedRowCountDelta = -oldRowCount; + } else { + // Notify immediately. + mTree->RowCountChanged(0, -oldRowCount); + } + } else if (mInput) { nsCOMPtr popup; mInput->GetPopup(getter_AddRefs(popup)); NS_ENSURE_TRUE(popup != nullptr, NS_ERROR_FAILURE); diff --git a/toolkit/components/autocomplete/nsAutoCompleteController.h b/toolkit/components/autocomplete/nsAutoCompleteController.h index f43c63d3c3f4..0c2cc8b30c3d 100644 --- a/toolkit/components/autocomplete/nsAutoCompleteController.h +++ b/toolkit/components/autocomplete/nsAutoCompleteController.h @@ -117,7 +117,7 @@ protected: */ nsresult GetFinalDefaultCompleteValue(nsAString &_retval); - nsresult ClearResults(); + nsresult ClearResults(bool aIsSearching = false); nsresult RowIndexToSearch(int32_t aRowIndex, int32_t *aSearchIndex, int32_t *aItemIndex); @@ -164,7 +164,7 @@ protected: uint32_t mRowCount; uint32_t mSearchesOngoing; uint32_t mSearchesFailed; - bool mFirstSearchResult; + int32_t mDelayedRowCountDelta; uint32_t mImmediateSearchesCount; // The index of the match on the popup that was selected using the keyboard, // if the completeselectedindex attribute is set. diff --git a/toolkit/components/autocomplete/nsIAutoCompleteSearch.idl b/toolkit/components/autocomplete/nsIAutoCompleteSearch.idl index 188c333ac687..e7f828436f2a 100644 --- a/toolkit/components/autocomplete/nsIAutoCompleteSearch.idl +++ b/toolkit/components/autocomplete/nsIAutoCompleteSearch.idl @@ -40,14 +40,6 @@ interface nsIAutoCompleteObserver : nsISupports * @param result - The search result object */ void onSearchResult(in nsIAutoCompleteSearch search, in nsIAutoCompleteResult result); - - /* - * Called to update with new results - * - * @param search - The search object that processed this search - * @param result - The search result object - */ - void onUpdateSearchResult(in nsIAutoCompleteSearch search, in nsIAutoCompleteResult result); }; [scriptable, uuid(4c3e7462-fbfb-4310-8f4b-239238392b75)] diff --git a/toolkit/components/places/UnifiedComplete.js b/toolkit/components/places/UnifiedComplete.js index 188a3b054962..eae2fdc98803 100644 --- a/toolkit/components/places/UnifiedComplete.js +++ b/toolkit/components/places/UnifiedComplete.js @@ -2384,6 +2384,8 @@ Search.prototype = { _notifyDelaysCount: 0, notifyResult(searchOngoing, skipDelay = false) { let notify = () => { + if (!this.pending) + return; this._notifyDelaysCount = 0; let resultCode = this._currentMatchCount ? "RESULT_SUCCESS" : "RESULT_NOMATCH"; if (searchOngoing) {