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
This commit is contained in:
Marco Bonardo
2017-11-19 21:58:14 +01:00
parent edf443e7fb
commit c6b922c0b7
4 changed files with 34 additions and 35 deletions

View File

@@ -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<nsIAutoCompletePopup> popup;
mInput->GetPopup(getter_AddRefs(popup));
NS_ENSURE_TRUE(popup != nullptr, NS_ERROR_FAILURE);