Bug 607469 - IPC-only Crash [@ mozilla::places::History::NotifyVisited]
authorShawn Wilsher <me@shawnwilsher.com>
Wed, 27 Oct 2010 13:14:16 -0700
changeset 56630 16b8b0c760650bc3e250d9b18d0ba2943f1fa505
parent 56629 2ee46939bb9e668514c75059d284ae5205f44c95
child 56632 16c54dba9f6b4a931f73959bb1d2eb2729c633fa
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs607469
milestone2.0b8pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 607469 - IPC-only Crash [@ mozilla::places::History::NotifyVisited] r=dougt a=blocking
toolkit/components/places/src/History.cpp
toolkit/components/places/tests/cpp/test_IHistory.cpp
--- a/toolkit/components/places/src/History.cpp
+++ b/toolkit/components/places/src/History.cpp
@@ -1207,22 +1207,37 @@ History::RegisterVisitedCallback(nsIURI*
   if (observers.IsEmpty()) {
     NS_ASSERTION(!keyAlreadyExists,
                  "An empty key was kept around in our hashtable!");
 
     // We are the first Link node to ask about this URI, or there are no pending
     // Links wanting to know about this URI.  Therefore, we should query the
     // database now.
     nsresult rv = VisitedQuery::Start(aURI);
+
+    // In IPC builds, we are passed a NULL Link from
+    // ContentParent::RecvStartVisitedQuery.  Since we won't be adding a NULL
+    // entry to our list of observers, and the code after this point assumes
+    // that aLink is non-NULL, we will need to return now.
     if (NS_FAILED(rv) || !aLink) {
       // Remove our array from the hashtable so we don't keep it around.
       mObservers.RemoveEntry(aURI);
       return rv;
     }
   }
+#ifdef MOZ_IPC
+  // In IPC builds, we are passed a NULL Link from
+  // ContentParent::RecvStartVisitedQuery.  All of our code after this point
+  // assumes aLink is non-NULL, so we have to return now.
+  else if (!aLink) {
+    NS_ASSERTION(XRE_GetProcessType() == GeckoProcessType_Default,
+                 "We should only ever get a null Link in the default process!");
+    return NS_OK;
+  }
+#endif
 
   // Sanity check that Links are not registered more than once for a given URI.
   // This will not catch a case where it is registered for two different URIs.
   NS_ASSERTION(!observers.Contains(aLink),
                "Already tracking this Link object!");
 
   // Start tracking our Link.
   if (!observers.AppendElement(aLink)) {
--- a/toolkit/components/places/tests/cpp/test_IHistory.cpp
+++ b/toolkit/components/places/tests/cpp/test_IHistory.cpp
@@ -546,16 +546,44 @@ test_visituri_transition_embed()
   do_get_lastVisit(place.id, visit);
 
   do_check_true(visit.transitionType == nsINavHistoryService::TRANSITION_EMBED);
 
   run_next_test();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
+//// IPC-only Tests
+
+#ifdef MOZ_IPC
+void
+test_two_null_links_same_uri()
+{
+  // Tests that we do not crash when we have had two NULL Links passed to
+  // RegisterVisitedCallback and then the visit occurs (bug 607469).  This only
+  // happens in IPC builds.
+  nsCOMPtr<nsIURI> testURI(new_test_uri());
+
+  nsCOMPtr<IHistory> history(do_get_IHistory());
+  nsresult rv = history->RegisterVisitedCallback(testURI, NULL);
+  do_check_success(rv);
+  rv = history->RegisterVisitedCallback(testURI, NULL);
+  do_check_success(rv);
+
+  rv = history->VisitURI(testURI, NULL, mozilla::IHistory::TOP_LEVEL);
+  do_check_success(rv);
+
+  nsCOMPtr<VisitURIObserver> finisher = new VisitURIObserver();
+  finisher->WaitForNotification();
+
+  run_next_test();
+}
+#endif // MOZ_IPC
+
+////////////////////////////////////////////////////////////////////////////////
 //// Test Harness
 
 /**
  * Note: for tests marked "Order Important!", please see the test for details.
  */
 Test gTests[] = {
   TEST(test_unvisted_does_not_notify_part1), // Order Important!
   TEST(test_visited_notifies),
@@ -566,14 +594,19 @@ Test gTests[] = {
   TEST(test_RegisterVisitedCallback_returns_before_notifying),
   TEST(test_observer_topic_dispatched),
   TEST(test_visituri_inserts),
   TEST(test_visituri_updates),
   TEST(test_visituri_preserves_shown_and_typed),
   TEST(test_visituri_creates_visit),
   TEST(test_visituri_transition_typed),
   TEST(test_visituri_transition_embed),
+
+  // The rest of these tests are tests that are only run in IPC builds.
+#ifdef MOZ_IPC
+  TEST(test_two_null_links_same_uri),
+#endif // MOZ_IPC
 };
 
 const char* file = __FILE__;
 #define TEST_NAME "IHistory"
 #define TEST_FILE file
 #include "places_test_harness_tail.h"