Commit 067b1656 authored by Marco Bonardo's avatar Marco Bonardo
Browse files

Bug 894042 - Disallow Places from creating async statements after asyncClose(). r=Mano, a=test-only

parent e92e6873

toolkit/components/places/Database.cpp

+5 −3
Changes for toolkit/components/places/Database.cpp: 5 added lines, 3 removed lines.
Original line number Diff line number Diff line
@@ -344,6 +344,7 @@ Database::Database() 
  , mDBPageSize(0)

  , mDatabaseStatus(nsINavHistoryService::DATABASE_STATUS_OK)

  , mShuttingDown(false)

  , mClosed(false)

{

  // Attempting to create two instances of the service?

  MOZ_ASSERT(!gDatabase);
@@ -1914,6 +1915,9 @@ Database::Shutdown() 
{

  MOZ_ASSERT(NS_IsMainThread());

  MOZ_ASSERT(!mShuttingDown);

  MOZ_ASSERT(!mClosed);



  mShuttingDown = true;



  mMainThreadStatements.FinalizeStatements();

  mMainThreadAsyncStatements.FinalizeStatements();
@@ -1929,9 +1933,7 @@ Database::Shutdown() 
  (void)mMainConn->AsyncClose(closeListener);

  closeListener->Spin();



  // Don't set this earlier, otherwise some internal helper used on shutdown

  // may bail out.

  mShuttingDown = true;

  mClosed = true;

}



////////////////////////////////////////////////////////////////////////////////

toolkit/components/places/Database.h

+2 −1
Changes for toolkit/components/places/Database.h: 2 added lines, 1 removed line.
Original line number Diff line number Diff line
@@ -122,7 +122,7 @@ public: 
   */

  void DispatchToAsyncThread(nsIRunnable* aEvent) const

  {

    if (mShuttingDown) {

    if (mClosed) {

      return;

    }

    nsCOMPtr<nsIEventTarget> target = do_GetInterface(mMainConn);
@@ -297,6 +297,7 @@ private: 
  int32_t mDBPageSize;

  uint16_t mDatabaseStatus;

  bool mShuttingDown;

  bool mClosed;

};



} // namespace places

toolkit/components/places/tests/bookmarks/test_async_observers.js

+32 −0
Changes for toolkit/components/places/tests/bookmarks/test_async_observers.js: 32 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -123,6 +123,38 @@ add_task(function cleanup() 
  PlacesUtils.bookmarks.removeObserver(observer, false);

});



add_task(function shutdown()

{

  // Check that async observers don't try to create async statements after

  // shutdown.  That would cause assertions, since the async thread is gone

  // already.  Note that in such a case the notifications are not fired, so we

  // cannot test for them.

  // Put an history notification that triggers AsyncGetBookmarksForURI between

  // asyncClose() and the actual connection closing.  Enqueuing a main-thread

  // event just after places-will-close-connection should ensure it runs before

  // places-connection-closed.

  // Notice this code is not using helpers cause it depends on a very specific

  // order, a change in the helpers code could make this test useless.

  let deferred = Promise.defer();



  Services.obs.addObserver(function onNotification() {

    Services.obs.removeObserver(onNotification, "places-will-close-connection");

    do_check_true(true, "Observed fake places shutdown");



    Services.tm.mainThread.dispatch(() => {

      // WARNING: this is very bad, never use out of testing code.

      PlacesUtils.bookmarks.QueryInterface(Ci.nsINavHistoryObserver)

                           .onPageChanged(NetUtil.newURI("http://book.ma.rk/"),

                                          Ci.nsINavHistoryObserver.ATTRIBUTE_FAVICON,

                                          "test", "test");

      deferred.resolve(promiseTopicObserved("places-connection-closed"));

    }, Ci.nsIThread.DISPATCH_NORMAL);

  }, "places-will-close-connection", false);

  shutdownPlaces();



  yield deferred.promise;

});



function run_test()

{

  // Add multiple bookmarks to the same uri.