diff --git a/toolkit/components/places/nsNavHistoryResult.cpp b/toolkit/components/places/nsNavHistoryResult.cpp index e4d9ef6962ef..aa7b9d48ee5e 100644 --- a/toolkit/components/places/nsNavHistoryResult.cpp +++ b/toolkit/components/places/nsNavHistoryResult.cpp @@ -54,21 +54,9 @@ return NS_OK; \ } else -// This should be used whenever the result has to hand out up-to-date contents -// to the caller. During batches the contents are not updated until a Refresh() -// is executed. This ensures a Refresh() is executed before proceeding, if a -// batch is ongoing. -// Note that there's no point in using this in node getters, since after a -// batch the node would be replaced by a new one, so it would hand out outdated -// information regardless. -#define END_RESULT_BATCH_AND_REFRESH_CONTENTS() \ - PR_BEGIN_MACRO \ - nsNavHistoryResult* result = GetResult(); \ - NS_WARN_IF_FALSE(result, "Working with a non-live-updating Places container"); \ - if (result && result->mBatchInProgress) { \ - result->EndBatch(); \ - } \ - PR_END_MACRO +// Number of changes to handle separately in a batch. If more changes are +// requested the node will switch to full refresh mode. +#define MAX_BATCH_CHANGES_BEFORE_REFRESH 5 // Emulate string comparison (used for sorting) for PRTime and int. inline int32_t ComparePRTime(PRTime a, PRTime b) @@ -1662,8 +1650,6 @@ nsNavHistoryContainerResultNode::ChangeTitles(nsIURI* aURI, NS_IMETHODIMP nsNavHistoryContainerResultNode::GetHasChildren(bool *aHasChildren) { - END_RESULT_BATCH_AND_REFRESH_CONTENTS(); - *aHasChildren = (mChildren.Count() > 0); return NS_OK; } @@ -1677,9 +1663,6 @@ nsNavHistoryContainerResultNode::GetChildCount(uint32_t* aChildCount) { if (!mExpanded) return NS_ERROR_NOT_AVAILABLE; - - END_RESULT_BATCH_AND_REFRESH_CONTENTS(); - *aChildCount = mChildren.Count(); return NS_OK; } @@ -1691,9 +1674,6 @@ nsNavHistoryContainerResultNode::GetChild(uint32_t aIndex, { if (!mExpanded) return NS_ERROR_NOT_AVAILABLE; - - END_RESULT_BATCH_AND_REFRESH_CONTENTS(); - if (aIndex >= uint32_t(mChildren.Count())) return NS_ERROR_INVALID_ARG; NS_ADDREF(*_retval = mChildren[aIndex]); @@ -1708,8 +1688,6 @@ nsNavHistoryContainerResultNode::GetChildIndex(nsINavHistoryResultNode* aNode, if (!mExpanded) return NS_ERROR_NOT_AVAILABLE; - END_RESULT_BATCH_AND_REFRESH_CONTENTS(); - int32_t nodeIndex = FindChild(static_cast(aNode)); if (nodeIndex == -1) return NS_ERROR_INVALID_ARG; @@ -1728,8 +1706,6 @@ nsNavHistoryContainerResultNode::FindNodeByDetails(const nsACString& aURIString, if (!mExpanded) return NS_ERROR_NOT_AVAILABLE; - END_RESULT_BATCH_AND_REFRESH_CONTENTS(); - *_retval = nullptr; for (int32_t i = 0; i < mChildren.Count(); ++i) { if (mChildren[i]->mURI.Equals(aURIString) && @@ -1797,7 +1773,8 @@ nsNavHistoryQueryResultNode::nsNavHistoryQueryResultNode( true, nullptr), mLiveUpdate(QUERYUPDATE_COMPLEX_WITH_BOOKMARKS), mHasSearchTerms(false), - mContentsValid(false) + mContentsValid(false), + mBatchChanges(0) { } @@ -1810,6 +1787,7 @@ nsNavHistoryQueryResultNode::nsNavHistoryQueryResultNode( true, aOptions), mQueries(aQueries), mContentsValid(false), + mBatchChanges(0), mTransitions(mQueries[0]->Transitions()) { NS_ASSERTION(aQueries.Count() > 0, "Must have at least one query"); @@ -1842,6 +1820,7 @@ nsNavHistoryQueryResultNode::nsNavHistoryQueryResultNode( true, aOptions), mQueries(aQueries), mContentsValid(false), + mBatchChanges(0), mTransitions(mQueries[0]->Transitions()) { NS_ASSERTION(aQueries.Count() > 0, "Must have at least one query"); @@ -1987,8 +1966,6 @@ nsNavHistoryQueryResultNode::GetHasChildren(bool* aHasChildren) return NS_OK; } - END_RESULT_BATCH_AND_REFRESH_CONTENTS(); - uint16_t resultType = mOptions->ResultType(); // Tags are always populated, otherwise they are removed. @@ -2392,6 +2369,7 @@ nsNavHistoryQueryResultNode::OnEndUpdateBatch() NS_ENSURE_SUCCESS(rv, rv); } + mBatchChanges = 0; return NS_OK; } @@ -2430,7 +2408,8 @@ nsNavHistoryQueryResultNode::OnVisit(nsIURI* aURI, int64_t aVisitId, nsNavHistoryResult* result = GetResult(); NS_ENSURE_STATE(result); - if (result->mBatchInProgress) { + if (result->mBatchInProgress && + ++mBatchChanges > MAX_BATCH_CHANGES_BEFORE_REFRESH) { nsresult rv = Refresh(); NS_ENSURE_SUCCESS(rv, rv); return NS_OK; @@ -2575,7 +2554,8 @@ nsNavHistoryQueryResultNode::OnTitleChanged(nsIURI* aURI, nsNavHistoryResult* result = GetResult(); NS_ENSURE_STATE(result); - if (result->mBatchInProgress) { + if (result->mBatchInProgress && + ++mBatchChanges > MAX_BATCH_CHANGES_BEFORE_REFRESH) { nsresult rv = Refresh(); NS_ENSURE_SUCCESS(rv, rv); return NS_OK; @@ -2645,7 +2625,8 @@ nsNavHistoryQueryResultNode::OnDeleteURI(nsIURI* aURI, { nsNavHistoryResult* result = GetResult(); NS_ENSURE_STATE(result); - if (result->mBatchInProgress) { + if (result->mBatchInProgress && + ++mBatchChanges > MAX_BATCH_CHANGES_BEFORE_REFRESH) { nsresult rv = Refresh(); NS_ENSURE_SUCCESS(rv, rv); return NS_OK; @@ -2793,11 +2774,7 @@ nsNavHistoryQueryResultNode::NotifyIfTagsChanged(nsIURI* aURI) nsCOMArray matches; RecursiveFindURIs(onlyOneEntry, this, spec, &matches); - bool skipRemovedURI = false; - if (mRemovingURI) - (void)mRemovingURI->Equals(aURI, &skipRemovedURI); - - if (matches.Count() == 0 && mHasSearchTerms && !skipRemovedURI) { + if (matches.Count() == 0 && mHasSearchTerms && !mRemovingURI) { // A new tag has been added, it's possible it matches our query. NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY); rv = history->URIToResultNode(aURI, mOptions, getter_AddRefs(node)); @@ -2850,12 +2827,6 @@ nsNavHistoryQueryResultNode::OnItemAdded(int64_t aItemId, const nsACString& aGUID, const nsACString& aParentGUID) { - bool sameURI = false; - if (mRemovingURI && NS_SUCCEEDED(mRemovingURI->Equals(aURI, &sameURI)) && - sameURI) { - mRemovingURI = nullptr; - } - if (aItemType == nsINavBookmarksService::TYPE_BOOKMARK && mLiveUpdate != QUERYUPDATE_SIMPLE && mLiveUpdate != QUERYUPDATE_TIME) { nsresult rv = Refresh(); @@ -2875,7 +2846,6 @@ nsNavHistoryQueryResultNode::OnItemRemoved(int64_t aItemId, const nsACString& aParentGUID) { mRemovingURI = aURI; - if (aItemType == nsINavBookmarksService::TYPE_BOOKMARK && mLiveUpdate != QUERYUPDATE_SIMPLE && mLiveUpdate != QUERYUPDATE_TIME) { nsresult rv = Refresh(); @@ -3101,8 +3071,6 @@ nsNavHistoryFolderResultNode::OpenContainerAsync() NS_IMETHODIMP nsNavHistoryFolderResultNode::GetHasChildren(bool* aHasChildren) { - END_RESULT_BATCH_AND_REFRESH_CONTENTS(); - if (!mContentsValid) { nsresult rv = FillChildren(); NS_ENSURE_SUCCESS(rv, rv); @@ -4054,7 +4022,6 @@ nsNavHistoryResult::nsNavHistoryResult(nsNavHistoryContainerResultNode* aRoot) , mIsAllBookmarksObserver(false) , mBookmarkFolderObservers(128) , mBatchInProgress(false) - , mRelatedNotificationsCount(0) , mSuppressNotifications(false) { mRootNode->mResult = this; @@ -4281,9 +4248,6 @@ nsNavHistoryResult::SetSortingMode(uint16_t aSortingMode) return NS_OK; } - if (mBatchInProgress) - EndBatch(); - // Actually do sorting. nsNavHistoryContainerResultNode::SortComparator comparator = nsNavHistoryContainerResultNode::GetSortingComparator(aSortingMode); @@ -4379,49 +4343,6 @@ nsNavHistoryResult::requestRefresh(nsNavHistoryContainerResultNode* aContainer) mRefreshParticipants.AppendElement(aContainer); } -// This interval is used for smart batches handling. -// Count the number of related notification, by checking if the interval between -// the end of the previous notification and the beginning of the next one is -// smaller than RELATED_NOTIFICATIONS_INTERVAL_MS. -// If there are more than RELATED_NOTIFICATIONS_THRESHOLD notifications, start -// an automatic batch. -// Similarly, if there are no more related notifications for -// RELATED_NOTIFICATIONS_INTERVAL_MS, automatically close the batch. -// Note we use LoRes TimeStamps for performance reasons. -#define RELATED_NOTIFICATIONS_INTERVAL_MS 150 -#define RELATED_NOTIFICATIONS_THRESHOLD 10 -#define MS_FROM_NOW(_stamp) (TimeStamp::NowLoRes() - _stamp).ToMilliseconds() - -void -nsNavHistoryResult::MaybeBeginBatch() -{ - if (!mBatchInProgress && !mLastNotificationTimeStamp.IsNull() && - MS_FROM_NOW(mLastNotificationTimeStamp) < (double)RELATED_NOTIFICATIONS_INTERVAL_MS) { - if (++mRelatedNotificationsCount > RELATED_NOTIFICATIONS_THRESHOLD) { - mRelatedNotificationsCount = 0; - DebugOnly rv = BeginBatch(); - MOZ_ASSERT(NS_SUCCEEDED(rv)); - } - } else { - mRelatedNotificationsCount = 0; - } -} - -// static -void -nsNavHistoryResult::MaybeEndBatchCallback(nsITimer* aTimer, void* aClosure) -{ - nsNavHistoryResult* result = static_cast(aClosure); - MOZ_ASSERT(result); - if (result && - MS_FROM_NOW(result->mLastNotificationTimeStamp) > (double)RELATED_NOTIFICATIONS_INTERVAL_MS) { - DebugOnly rv = result->EndBatch(); - MOZ_ASSERT(NS_SUCCEEDED(rv)); - } -} - -#undef MS_FROM_NOW - // nsINavBookmarkObserver implementation // Here, it is important that we create a COPY of the observer array. Some @@ -4432,26 +4353,18 @@ nsNavHistoryResult::MaybeEndBatchCallback(nsITimer* aTimer, void* aClosure) FolderObserverList* _fol = BookmarkFolderObserversForId(_folderId, false); \ if (_fol) { \ FolderObserverList _listCopy(*_fol); \ - if (_listCopy.Length() > 0) { \ - MaybeBeginBatch(); \ - for (uint32_t _fol_i = 0; _fol_i < _listCopy.Length(); ++_fol_i) { \ - if (_listCopy[_fol_i]) \ - _listCopy[_fol_i]->_functionCall; \ - } \ - mLastNotificationTimeStamp = TimeStamp::NowLoRes(); \ + for (uint32_t _fol_i = 0; _fol_i < _listCopy.Length(); ++_fol_i) { \ + if (_listCopy[_fol_i]) \ + _listCopy[_fol_i]->_functionCall; \ } \ } \ PR_END_MACRO #define ENUMERATE_LIST_OBSERVERS(_listType, _functionCall, _observersList, _conditionCall) \ PR_BEGIN_MACRO \ _listType _listCopy(_observersList); \ - if (_listCopy.Length() > 0) { \ - MaybeBeginBatch(); \ - for (uint32_t _obs_i = 0; _obs_i < _listCopy.Length(); ++_obs_i) { \ - if (_listCopy[_obs_i] && _listCopy[_obs_i]->_conditionCall) \ - _listCopy[_obs_i]->_functionCall; \ - } \ - mLastNotificationTimeStamp = TimeStamp::NowLoRes(); \ + for (uint32_t _obs_i = 0; _obs_i < _listCopy.Length(); ++_obs_i) { \ + if (_listCopy[_obs_i] && _listCopy[_obs_i]->_conditionCall) \ + _listCopy[_obs_i]->_functionCall; \ } \ PR_END_MACRO #define ENUMERATE_QUERY_OBSERVERS(_functionCall, _observersList, _conditionCall) \ @@ -4460,66 +4373,50 @@ nsNavHistoryResult::MaybeEndBatchCallback(nsITimer* aTimer, void* aClosure) ENUMERATE_QUERY_OBSERVERS(_functionCall, mAllBookmarksObservers, IsQuery()) #define ENUMERATE_HISTORY_OBSERVERS(_functionCall) \ ENUMERATE_QUERY_OBSERVERS(_functionCall, mHistoryObservers, IsQuery()) -#define NOTIFY_REFRESH(_listType, _observersList, _conditionCall, _clear) \ + +#define NOTIFY_REFRESH_PARTICIPANTS() \ PR_BEGIN_MACRO \ - _listType _listCopy(_observersList); \ - for (uint32_t _obs_i = 0; _obs_i < _listCopy.Length(); ++_obs_i) { \ - if (_listCopy[_obs_i] && _listCopy[_obs_i]->_conditionCall) \ - _listCopy[_obs_i]->Refresh(); \ - } \ - if (_clear) \ - _observersList.Clear(); \ + ENUMERATE_LIST_OBSERVERS(ContainerObserverList, Refresh(), mRefreshParticipants, IsContainer()); \ + mRefreshParticipants.Clear(); \ PR_END_MACRO -nsresult -nsNavHistoryResult::BeginBatch() { - mBatchInProgress = true; - ENUMERATE_HISTORY_OBSERVERS(OnBeginUpdateBatch()); - ENUMERATE_ALL_BOOKMARKS_OBSERVERS(OnBeginUpdateBatch()); - NOTIFY_RESULT_OBSERVERS(this, Batching(true)); +NS_IMETHODIMP +nsNavHistoryResult::OnBeginUpdateBatch() +{ + // Since we could be observing both history and bookmarks, it's possible both + // notify the batch. We can safely ignore nested calls. + if (!mBatchInProgress) { + mBatchInProgress = true; + ENUMERATE_HISTORY_OBSERVERS(OnBeginUpdateBatch()); + ENUMERATE_ALL_BOOKMARKS_OBSERVERS(OnBeginUpdateBatch()); - if (!mEndBatchTimer) - mEndBatchTimer = do_CreateInstance("@mozilla.org/timer;1"); - MOZ_ASSERT(mEndBatchTimer); - if (mEndBatchTimer) { - mEndBatchTimer->InitWithFuncCallback(MaybeEndBatchCallback, this, - RELATED_NOTIFICATIONS_INTERVAL_MS, - nsITimer::TYPE_REPEATING_SLACK); - } else { - DebugOnly rv = EndBatch(); - MOZ_ASSERT(NS_SUCCEEDED(rv)); + NOTIFY_RESULT_OBSERVERS(this, Batching(true)); } return NS_OK; } -nsresult -nsNavHistoryResult::EndBatch() { - MOZ_ASSERT(mBatchInProgress); - if (mEndBatchTimer) - mEndBatchTimer->Cancel(); - - ENUMERATE_HISTORY_OBSERVERS(OnEndUpdateBatch()); - ENUMERATE_ALL_BOOKMARKS_OBSERVERS(OnEndUpdateBatch()); - - // Setting mBatchInProgress before notifying the end of the batch to - // observers would make evantual calls to Refresh() directly handled rather - // than enqueued. Thus set it just before handling refreshes. - mBatchInProgress = false; - NOTIFY_REFRESH(ContainerObserverList, mRefreshParticipants, IsContainer(), true); - NOTIFY_RESULT_OBSERVERS(this, Batching(false)); - return NS_OK; -} - -NS_IMETHODIMP -nsNavHistoryResult::OnBeginUpdateBatch() -{ - return NS_OK; -} NS_IMETHODIMP nsNavHistoryResult::OnEndUpdateBatch() { + // Since we could be observing both history and bookmarks, it's possible both + // notify the batch. We can safely ignore nested calls. + // Notice it's possible we are notified OnEndUpdateBatch more times than + // onBeginUpdateBatch, since the result could be created in the middle of + // nested batches. + if (mBatchInProgress) { + ENUMERATE_HISTORY_OBSERVERS(OnEndUpdateBatch()); + ENUMERATE_ALL_BOOKMARKS_OBSERVERS(OnEndUpdateBatch()); + + // Setting mBatchInProgress before notifying the end of the batch to + // observers would make evantual calls to Refresh() directly handled rather + // than enqueued. Thus set it just before handling refreshes. + mBatchInProgress = false; + NOTIFY_REFRESH_PARTICIPANTS(); + NOTIFY_RESULT_OBSERVERS(this, Batching(false)); + } + return NS_OK; } @@ -4748,7 +4645,7 @@ nsNavHistoryResult::OnVisit(nsIURI* aURI, int64_t aVisitId, PRTime aTime, // observers that are containers queries and refresh them. // We use a copy of the observers array since requerying could potentially // cause changes to the array. - NOTIFY_REFRESH(QueryObserverList, mHistoryObservers, IsContainersQuery(), false); + ENUMERATE_QUERY_OBSERVERS(Refresh(), mHistoryObservers, IsContainersQuery()); } return NS_OK; diff --git a/toolkit/components/places/nsNavHistoryResult.h b/toolkit/components/places/nsNavHistoryResult.h index 2ad3eb25d0c2..21aff6528363 100644 --- a/toolkit/components/places/nsNavHistoryResult.h +++ b/toolkit/components/places/nsNavHistoryResult.h @@ -18,7 +18,6 @@ #include "nsCycleCollectionParticipant.h" #include "mozilla/storage.h" #include "Helpers.h" -#include "mozilla/TimeStamp.h" class nsNavHistory; class nsNavHistoryQuery; @@ -173,16 +172,8 @@ public: bool aExpand); void InvalidateTree(); - + bool mBatchInProgress; - int32_t mRelatedNotificationsCount; - mozilla::TimeStamp mLastNotificationTimeStamp; - nsCOMPtr mEndBatchTimer; - - void MaybeBeginBatch(); - static void MaybeEndBatchCallback(nsITimer* aTimer, void* aClosure); - nsresult BeginBatch(); - nsresult EndBatch(); nsMaybeWeakPtrArray mObservers; bool mSuppressNotifications; @@ -693,6 +684,8 @@ public: nsCOMPtr mRemovingURI; nsresult NotifyIfTagsChanged(nsIURI* aURI); + uint32_t mBatchChanges; + // Tracks transition type filters shared by all mQueries. nsTArray mTransitions; }; diff --git a/toolkit/components/places/tests/queries/head_queries.js b/toolkit/components/places/tests/queries/head_queries.js index 3f7003f3b1c7..05169548f72f 100644 --- a/toolkit/components/places/tests/queries/head_queries.js +++ b/toolkit/components/places/tests/queries/head_queries.js @@ -277,7 +277,6 @@ function compareArrayToResult(aArray, aRoot) { // Debugging code for failures. dump_table("moz_places"); dump_table("moz_historyvisits"); - dump_table("moz_bookmarks"); LOG("Found children:"); for (let i = 0; i < aRoot.childCount; i++) { LOG(aRoot.getChild(i).uri); diff --git a/toolkit/components/places/tests/queries/test_history_queries_tags_liveUpdate.js b/toolkit/components/places/tests/queries/test_history_queries_tags_liveUpdate.js index ed6bebdbbbf3..da19401a3e26 100644 --- a/toolkit/components/places/tests/queries/test_history_queries_tags_liveUpdate.js +++ b/toolkit/components/places/tests/queries/test_history_queries_tags_liveUpdate.js @@ -68,12 +68,13 @@ add_task(function pages_query() testQueryContents(query, options, function (root) { compareArrayToResult([gTestData[0], gTestData[1], gTestData[2]], root); for (let i = 0; i < root.childCount; i++) { - let uri = NetUtil.newURI(root.getChild(i).uri); - do_check_eq(root.getChild(i).tags, null); + let node = root.getChild(i); + let uri = NetUtil.newURI(node.uri); + do_check_eq(node.tags, null); PlacesUtils.tagging.tagURI(uri, ["test-tag"]); - do_check_eq(root.getChild(i).tags, "test-tag"); + do_check_eq(node.tags, "test-tag"); PlacesUtils.tagging.untagURI(uri, ["test-tag"]); - do_check_eq(root.getChild(i).tags, null); + do_check_eq(node.tags, null); } }); }); @@ -85,12 +86,13 @@ add_task(function visits_query() testQueryContents(query, options, function (root) { compareArrayToResult([gTestData[0], gTestData[1], gTestData[2]], root); for (let i = 0; i < root.childCount; i++) { - let uri = NetUtil.newURI(root.getChild(i).uri); - do_check_eq(root.getChild(i).tags, null); + let node = root.getChild(i); + let uri = NetUtil.newURI(node.uri); + do_check_eq(node.tags, null); PlacesUtils.tagging.tagURI(uri, ["test-tag"]); - do_check_eq(root.getChild(i).tags, "test-tag"); + do_check_eq(node.tags, "test-tag"); PlacesUtils.tagging.untagURI(uri, ["test-tag"]); - do_check_eq(root.getChild(i).tags, null); + do_check_eq(node.tags, null); } }); }); @@ -102,12 +104,13 @@ add_task(function bookmarks_query() testQueryContents(query, options, function (root) { compareArrayToResult([gTestData[0], gTestData[1], gTestData[2]], root); for (let i = 0; i < root.childCount; i++) { - let uri = NetUtil.newURI(root.getChild(i).uri); - do_check_eq(root.getChild(i).tags, null); + let node = root.getChild(i); + let uri = NetUtil.newURI(node.uri); + do_check_eq(node.tags, null); PlacesUtils.tagging.tagURI(uri, ["test-tag"]); - do_check_eq(root.getChild(i).tags, "test-tag"); + do_check_eq(node.tags, "test-tag"); PlacesUtils.tagging.untagURI(uri, ["test-tag"]); - do_check_eq(root.getChild(i).tags, null); + do_check_eq(node.tags, null); } }); }); @@ -119,12 +122,13 @@ add_task(function pages_searchterm_query() testQueryContents(query, options, function (root) { compareArrayToResult([gTestData[0], gTestData[1], gTestData[2]], root); for (let i = 0; i < root.childCount; i++) { - let uri = NetUtil.newURI(root.getChild(i).uri); - do_check_eq(root.getChild(i).tags, null); + let node = root.getChild(i); + let uri = NetUtil.newURI(node.uri); + do_check_eq(node.tags, null); PlacesUtils.tagging.tagURI(uri, ["test-tag"]); - do_check_eq(root.getChild(i).tags, "test-tag"); + do_check_eq(node.tags, "test-tag"); PlacesUtils.tagging.untagURI(uri, ["test-tag"]); - do_check_eq(root.getChild(i).tags, null); + do_check_eq(node.tags, null); } }); }); @@ -137,12 +141,13 @@ add_task(function visits_searchterm_query() testQueryContents(query, options, function (root) { compareArrayToResult([gTestData[0], gTestData[1], gTestData[2]], root); for (let i = 0; i < root.childCount; i++) { - let uri = NetUtil.newURI(root.getChild(i).uri); - do_check_eq(root.getChild(i).tags, null); + let node = root.getChild(i); + let uri = NetUtil.newURI(node.uri); + do_check_eq(node.tags, null); PlacesUtils.tagging.tagURI(uri, ["test-tag"]); - do_check_eq(root.getChild(i).tags, "test-tag"); + do_check_eq(node.tags, "test-tag"); PlacesUtils.tagging.untagURI(uri, ["test-tag"]); - do_check_eq(root.getChild(i).tags, null); + do_check_eq(node.tags, null); } }); }); diff --git a/toolkit/components/places/tests/queries/test_history_queries_titles_liveUpdate.js b/toolkit/components/places/tests/queries/test_history_queries_titles_liveUpdate.js index dedf67aeeee2..7bf732c69c96 100644 --- a/toolkit/components/places/tests/queries/test_history_queries_titles_liveUpdate.js +++ b/toolkit/components/places/tests/queries/test_history_queries_titles_liveUpdate.js @@ -35,7 +35,6 @@ function searchNodeHavingUrl(aRoot, aUrl) { return aRoot.getChild(i); } } - return null; } function newQueryWithOptions() @@ -59,12 +58,13 @@ add_task(function pages_query() compareArrayToResult([gTestData[0], gTestData[1], gTestData[2]], root); for (let i = 0; i < root.childCount; i++) { - do_check_eq(root.getChild(i).title, gTestData[i].title); - let uri = NetUtil.newURI(root.getChild(i).uri); + let node = root.getChild(i); + do_check_eq(node.title, gTestData[i].title); + let uri = NetUtil.newURI(node.uri); yield promiseAddVisits({uri: uri, title: "changedTitle"}); - do_check_eq(root.getChild(i).title, "changedTitle"); + do_check_eq(node.title, "changedTitle"); yield promiseAddVisits({uri: uri, title: gTestData[i].title}); - do_check_eq(root.getChild(i).title, gTestData[i].title); + do_check_eq(node.title, gTestData[i].title); } root.containerOpen = false; @@ -109,12 +109,13 @@ add_task(function pages_searchterm_query() compareArrayToResult([gTestData[0], gTestData[1], gTestData[2]], root); for (let i = 0; i < root.childCount; i++) { - let uri = NetUtil.newURI(root.getChild(i).uri); - do_check_eq(root.getChild(i).title, gTestData[i].title); + let node = root.getChild(i); + let uri = NetUtil.newURI(node.uri); + do_check_eq(node.title, gTestData[i].title); yield promiseAddVisits({uri: uri, title: "changedTitle"}); - do_check_eq(root.getChild(i).title, "changedTitle"); + do_check_eq(node.title, "changedTitle"); yield promiseAddVisits({uri: uri, title: gTestData[i].title}); - do_check_eq(root.getChild(i).title, gTestData[i].title); + do_check_eq(node.title, gTestData[i].title); } root.containerOpen = false; diff --git a/toolkit/components/places/tests/unit/test_nsINavHistoryViewer.js b/toolkit/components/places/tests/unit/test_nsINavHistoryViewer.js index 06486edb6861..377274b7cf68 100644 --- a/toolkit/components/places/tests/unit/test_nsINavHistoryViewer.js +++ b/toolkit/components/places/tests/unit/test_nsINavHistoryViewer.js @@ -63,6 +63,7 @@ var resultObserver = { }, inBatchMode: false, batching: function(aToggleMode) { + do_check_neq(this.inBatchMode, aToggleMode); this.inBatchMode = aToggleMode; }, result: null, @@ -80,35 +81,13 @@ var resultObserver = { } }; -function promiseBatch(aResult) { - let query = PlacesUtils.asQuery(aResult.root).getQueries()[0]; - return Task.spawn(function() { - let deferred = Promise.defer(); - aResult.addObserver({ - batching: function (aStatus) { - if (!aStatus) { - deferred.resolve(); - aResult.removeObserver(this); - } - } - }, false); - for (let i = 0; i < 10; i++) { - if (query.onlyBookmarked) - bmsvc.insertBookmark(bmsvc.bookmarksMenuFolder, testURI, bmsvc.DEFAULT_INDEX, "foo"); - else - yield promiseAddVisits(testURI); - } - yield deferred.promise; - }); -} - var testURI = uri("http://mozilla.com"); function run_test() { run_next_test(); } -add_task(function check_history_query() { +add_test(function check_history_query() { var options = histsvc.getNewQueryOptions(); options.sortingMode = options.SORT_BY_DATE_DESCENDING; options.resultType = options.RESULTS_AS_VISIT; @@ -118,55 +97,69 @@ add_task(function check_history_query() { var root = result.root; root.containerOpen = true; - // nsINavHistoryResultObserver.containerStateChanged + // nsINavHistoryResultObserver.containerOpened do_check_neq(resultObserver.openedContainer, null); // nsINavHistoryResultObserver.nodeInserted - yield promiseAddVisits(testURI); - do_check_eq(testURI.spec, resultObserver.insertedNode.uri); + // add a visit + promiseAddVisits(testURI).then(function() { + do_check_eq(testURI.spec, resultObserver.insertedNode.uri); - // nsINavHistoryResultObserver.nodeHistoryDetailsChanged - // adding a visit causes nodeHistoryDetailsChanged for the folder - do_check_eq(root.uri, resultObserver.nodeChangedByHistoryDetails.uri); + // nsINavHistoryResultObserver.nodeHistoryDetailsChanged + // adding a visit causes nodeHistoryDetailsChanged for the folder + do_check_eq(root.uri, resultObserver.nodeChangedByHistoryDetails.uri); - // nsINavHistoryResultObserver.itemTitleChanged for a leaf node - yield promiseAddVisits({ uri: testURI, title: "baz" }); + // nsINavHistoryResultObserver.itemTitleChanged for a leaf node + promiseAddVisits({ uri: testURI, title: "baz" }).then(function () { + do_check_eq(resultObserver.nodeChangedByTitle.title, "baz"); - do_check_eq(resultObserver.nodeChangedByTitle.title, "baz"); + // nsINavHistoryResultObserver.nodeRemoved + var removedURI = uri("http://google.com"); + promiseAddVisits(removedURI).then(function() { + bhist.removePage(removedURI); + do_check_eq(removedURI.spec, resultObserver.removedNode.uri); - // nsINavHistoryResultObserver.nodeRemoved - var removedURI = uri("http://google.com"); - yield promiseAddVisits(removedURI); - bhist.removePage(removedURI); - do_check_eq(removedURI.spec, resultObserver.removedNode.uri); + // nsINavHistoryResultObserver.invalidateContainer + bhist.removePagesFromHost("mozilla.com", false); + do_check_eq(root.uri, resultObserver.invalidatedContainer.uri); - // nsINavHistoryResultObserver.sortingChanged - resultObserver.invalidatedContainer = null; - result.sortingMode = options.SORT_BY_TITLE_ASCENDING; - do_check_eq(resultObserver.sortingMode, options.SORT_BY_TITLE_ASCENDING); - do_check_eq(resultObserver.invalidatedContainer, result.root); + // nsINavHistoryResultObserver.sortingChanged + resultObserver.invalidatedContainer = null; + result.sortingMode = options.SORT_BY_TITLE_ASCENDING; + do_check_eq(resultObserver.sortingMode, options.SORT_BY_TITLE_ASCENDING); + do_check_eq(resultObserver.invalidatedContainer, result.root); - // nsINavHistoryResultObserver.invalidateContainer - bhist.removeAllPages(); - do_check_eq(root.uri, resultObserver.invalidatedContainer.uri); + // nsINavHistoryResultObserver.invalidateContainer + bhist.removeAllPages(); + do_check_eq(root.uri, resultObserver.invalidatedContainer.uri); - // nsINavHistoryResultObserver.batching - do_check_false(resultObserver.inBatchMode); - yield promiseBatch(result); + // nsINavHistoryResultObserver.batching + do_check_false(resultObserver.inBatchMode); + histsvc.runInBatchMode({ + runBatched: function (aUserData) { + do_check_true(resultObserver.inBatchMode); + } + }, null); + do_check_false(resultObserver.inBatchMode); + bmsvc.runInBatchMode({ + runBatched: function (aUserData) { + do_check_true(resultObserver.inBatchMode); + } + }, null); + do_check_false(resultObserver.inBatchMode); - // nsINavHistoryResultObserver.invalidateContainer - do_check_eq(root.uri, resultObserver.invalidatedContainer.uri); - - // nsINavHistoryResultObserver.containerStateChanged - root.containerOpen = false; - do_check_eq(resultObserver.closedContainer, resultObserver.openedContainer); - result.removeObserver(resultObserver); - resultObserver.reset(); - - yield promiseAsyncUpdates(); + // nsINavHistoryResultObserver.containerClosed + root.containerOpen = false; + do_check_eq(resultObserver.closedContainer, resultObserver.openedContainer); + result.removeObserver(resultObserver); + resultObserver.reset(); + promiseAsyncUpdates().then(run_next_test); + }); + }); + }); }); -add_task(function check_bookmarks_query() { +add_test(function check_bookmarks_query() { var options = histsvc.getNewQueryOptions(); var query = histsvc.getNewQuery(); query.setFolders([bmsvc.bookmarksMenuFolder], 1); @@ -175,7 +168,7 @@ add_task(function check_bookmarks_query() { var root = result.root; root.containerOpen = true; - // nsINavHistoryResultObserver.containerStateChanged + // nsINavHistoryResultObserver.containerOpened do_check_neq(resultObserver.openedContainer, null); // nsINavHistoryResultObserver.nodeInserted @@ -201,6 +194,8 @@ add_task(function check_bookmarks_query() { bmsvc.removeItem(testBookmark2); do_check_eq(testBookmark2, resultObserver.removedNode.itemId); + // XXX nsINavHistoryResultObserver.invalidateContainer + // nsINavHistoryResultObserver.sortingChanged resultObserver.invalidatedContainer = null; result.sortingMode = options.SORT_BY_TITLE_ASCENDING; @@ -209,21 +204,28 @@ add_task(function check_bookmarks_query() { // nsINavHistoryResultObserver.batching do_check_false(resultObserver.inBatchMode); - yield promiseBatch(result); + histsvc.runInBatchMode({ + runBatched: function (aUserData) { + do_check_true(resultObserver.inBatchMode); + } + }, null); + do_check_false(resultObserver.inBatchMode); + bmsvc.runInBatchMode({ + runBatched: function (aUserData) { + do_check_true(resultObserver.inBatchMode); + } + }, null); + do_check_false(resultObserver.inBatchMode); - // nsINavHistoryResultObserver.invalidateContainer - do_check_eq(root.uri, resultObserver.invalidatedContainer.uri); - - // nsINavHistoryResultObserver.containerStateChanged + // nsINavHistoryResultObserver.containerClosed root.containerOpen = false; do_check_eq(resultObserver.closedContainer, resultObserver.openedContainer); result.removeObserver(resultObserver); resultObserver.reset(); - - yield promiseAsyncUpdates(); + promiseAsyncUpdates().then(run_next_test); }); -add_task(function check_mixed_query() { +add_test(function check_mixed_query() { var options = histsvc.getNewQueryOptions(); var query = histsvc.getNewQuery(); query.onlyBookmarked = true; @@ -232,21 +234,28 @@ add_task(function check_mixed_query() { var root = result.root; root.containerOpen = true; - // nsINavHistoryResultObserver.containerStateChanged + // nsINavHistoryResultObserver.containerOpened do_check_neq(resultObserver.openedContainer, null); // nsINavHistoryResultObserver.batching do_check_false(resultObserver.inBatchMode); - yield promiseBatch(result); + histsvc.runInBatchMode({ + runBatched: function (aUserData) { + do_check_true(resultObserver.inBatchMode); + } + }, null); + do_check_false(resultObserver.inBatchMode); + bmsvc.runInBatchMode({ + runBatched: function (aUserData) { + do_check_true(resultObserver.inBatchMode); + } + }, null); + do_check_false(resultObserver.inBatchMode); - // nsINavHistoryResultObserver.invalidateContainer - do_check_eq(root.uri, resultObserver.invalidatedContainer.uri); - - // nsINavHistoryResultObserver.containerStateChanged + // nsINavHistoryResultObserver.containerClosed root.containerOpen = false; do_check_eq(resultObserver.closedContainer, resultObserver.openedContainer); result.removeObserver(resultObserver); resultObserver.reset(); - - yield promiseAsyncUpdates(); + promiseAsyncUpdates().then(run_next_test); });