Bug 606966 - Need an async history visit API exposed to JS
authorShawn Wilsher <me@shawnwilsher.com>
Mon, 24 Jan 2011 14:13:24 -0800
changeset 61387 af4e43e6d0484fb575c3a7b7823bfdb5e551cc03
parent 61386 3a4e6a96823ac51cf9ce8b2eeb0ecf49d262dc42
child 61388 3962f456fd9a51ad9049aaf3872fa3499bd1becf
push id1
push userroot
push dateTue, 10 Dec 2013 15:46:25 +0000
bugs606966
milestone2.0b10pre
Bug 606966 - Need an async history visit API exposed to JS Part 22 - Dispatch errors for URIs that fail nsINavHistory::canAddURI r=mak a=blocking
toolkit/components/places/src/History.cpp
toolkit/components/places/tests/unit/test_async_history_api.js
toolkit/components/places/tests/unit/test_isvisited.js
--- a/toolkit/components/places/src/History.cpp
+++ b/toolkit/components/places/src/History.cpp
@@ -516,16 +516,68 @@ private:
    * this object (and therefore cannot hold a strong reference to it).
    */
   mozIVisitInfoCallback* mCallback;
   VisitData mPlace;
   const nsresult mResult;
 };
 
 /**
+ * Checks to see if we can add aURI to history, and dispatches an error to
+ * aCallback (if provided) if we cannot.
+ *
+ * @param aURI
+ *        The URI to check.
+ * @param [optional] aGUID
+ *        The guid of the URI to check.  This is passed back to the callback.
+ * @param [optional] aPlaceId
+ *        The placeId of the URI to check.  This is passed back to the callback.
+ * @param [optional] aCallback
+ *        The callback to notify if the URI cannot be added to history.
+ * @return true if the URI can be added to history, false otherwise.
+ */
+bool
+CanAddURI(nsIURI* aURI,
+          const nsCString& aGUID = EmptyCString(),
+          PRInt64 aPlaceId = 0,
+          mozIVisitInfoCallback* aCallback = NULL)
+{
+  nsNavHistory* navHistory = nsNavHistory::GetHistoryService();
+  NS_ENSURE_TRUE(navHistory, false);
+
+  PRBool canAdd;
+  nsresult rv = navHistory->CanAddURI(aURI, &canAdd);
+  if (NS_SUCCEEDED(rv) && canAdd) {
+    return true;
+  };
+
+  // We cannot add the URI.  Notify the callback, if we were given one.
+  if (aCallback) {
+    // NotifyCompletion does not hold a strong reference to the callback, so we
+    // have to manage it by AddRefing now and then releasing it after the event
+    // has run.
+    NS_ADDREF(aCallback);
+
+    VisitData place(aURI);
+    place.guid = aGUID;
+    place.placeId = aPlaceId;
+    nsCOMPtr<nsIRunnable> event =
+      new NotifyCompletion(aCallback, place, NS_ERROR_INVALID_ARG);
+    (void)NS_DispatchToMainThread(event);
+
+    // Also dispatch an event to release our reference to the callback after
+    // NotifyCompletion has run.
+    nsCOMPtr<nsIThread> mainThread = do_GetMainThread();
+    (void)NS_ProxyRelease(mainThread, aCallback, PR_TRUE);
+  }
+
+  return false;
+}
+
+/**
  * Adds a visit to the database.
  */
 class InsertVisitedURIs : public nsRunnable
 {
 public:
   /**
    * Adds a visit to the database asynchronously.
    *
@@ -619,16 +671,23 @@ private:
 
       // Speculatively get a new session id for our visit if the current session
       // id is non-valid.  While it is true that we will use the session id from
       // the referrer if the visit was "recent" enough, we cannot call this
       // method off of the main thread, so we have to consume an id now.
       if (mPlaces[i].sessionId <= 0) {
         mPlaces[i].sessionId = navHistory->GetNewSessionID();
       }
+
+#ifdef DEBUG
+      nsCOMPtr<nsIURI> uri;
+      (void)NS_NewURI(getter_AddRefs(uri), mPlaces[i].spec);
+      NS_ASSERTION(CanAddURI(uri),
+                   "Passed a VisitData with a URI we cannot add to history!");
+#endif
     }
 
     // We AddRef on the main thread, and release it when we are destroyed.
     NS_IF_ADDREF(mCallback);
   }
 
   virtual ~InsertVisitedURIs()
   {
@@ -1782,16 +1841,22 @@ History::UpdatePlaces(const jsval& aPlac
       if (fatGUID.IsVoid()) {
         guid.SetIsVoid(PR_TRUE);
       }
       else {
         guid = NS_ConvertUTF16toUTF8(fatGUID);
       }
     }
 
+    // Make sure that any uri we are given can be added to history, and if not,
+    // skip it (CanAddURI will notify our callback for us).
+    if (uri && !CanAddURI(uri, guid, placeId, aCallback)) {
+      continue;
+    }
+
     // We must have at least one of uri, valid id, or guid.
     NS_ENSURE_ARG(uri || placeId > 0 || !guid.IsVoid());
 
     // If we were given a guid, make sure it is valid.
     bool isValidGUID = IsValidGUID(guid);
     NS_ENSURE_ARG(guid.IsVoid() || isValidGUID);
 
     nsString title;
@@ -1863,21 +1928,26 @@ History::UpdatePlaces(const jsval& aPlac
       nsCOMPtr<nsIURI> referrer = GetURIFromJSObject(aCtx, visit,
                                                      "referrerURI");
       if (referrer) {
         (void)referrer->GetSpec(data.referrerSpec);
       }
     }
   }
 
-  mozIStorageConnection* dbConn = GetDBConn();
-  NS_ENSURE_STATE(dbConn);
+  // It is possible that all of the visits we were passed were dissallowed by
+  // CanAddURI, which isn't an error.  If we have no visits to add, however,
+  // we should not call InsertVisitedURIs::Start.
+  if (visitData.Length()) {
+    mozIStorageConnection* dbConn = GetDBConn();
+    NS_ENSURE_STATE(dbConn);
 
-  nsresult rv = InsertVisitedURIs::Start(dbConn, visitData, aCallback);
-  NS_ENSURE_SUCCESS(rv, rv);
+    nsresult rv = InsertVisitedURIs::Start(dbConn, visitData, aCallback);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
 
   return NS_OK;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 //// nsIObserver
 
 NS_IMETHODIMP
--- a/toolkit/components/places/tests/unit/test_async_history_api.js
+++ b/toolkit/components/places/tests/unit/test_async_history_api.js
@@ -297,16 +297,65 @@ function test_add_visit_invalid_transiti
   }
   catch (e) {
     do_check_eq(e.result, Cr.NS_ERROR_INVALID_ARG);
   }
 
   run_next_test();
 }
 
+function test_non_addable_uri_errors()
+{
+  // Array of protocols that nsINavHistoryService::canAddURI returns false for.
+  const URLS = [
+    "about:config",
+    "imap://cyrus.andrew.cmu.edu/archive.imap",
+    "news://new.mozilla.org/mozilla.dev.apps.firefox",
+    "mailbox:Inbox",
+    "moz-anno:favicon:http://mozilla.org/made-up-favicon",
+    "view-source:http://mozilla.org",
+    "chrome://browser/content/browser.xul",
+    "resource://gre-resources/hiddenWindow.html",
+    "data:,Hello%2C%20World!",
+    "wyciwyg:/0/http://mozilla.org",
+    "javascript:alert('hello wolrd!');",
+  ];
+  let places = [];
+  URLS.forEach(function(url) {
+    try {
+      let place = {
+        uri: NetUtil.newURI(url),
+        title: "test for " + url,
+        visits: [
+          new VisitInfo(),
+        ],
+      };
+      places.push(place);
+    }
+    catch (e if e.result === Cr.NS_ERROR_MALFORMED_URI) {
+      // NetUtil.newURI() can throw if e.g. our app knows about imap://
+      // but the account is not set up and so the URL is invalid for us.
+      // Note this in the log but ignore as it's not the subject of this test.
+      do_log_info("Could not construct URI for '" + url + "'; ignoring");
+    }
+  });
+
+  let callbackCount = 0;
+  gHistory.updatePlaces(places, function(aResultCode, aPlaceInfo) {
+    do_log_info("Checking '" + aPlaceInfo.uri.spec + "'");
+    do_check_eq(aResultCode, Cr.NS_ERROR_INVALID_ARG);
+    do_check_false(gGlobalHistory.isVisited(aPlaceInfo.uri));
+
+    // If we have had all of our callbacks, continue running tests.
+    if (++callbackCount == places.length) {
+      run_next_test();
+    }
+  });
+}
+
 function test_add_visit()
 {
   const VISIT_TIME = Date.now() * 1000;
   let place = {
     uri: NetUtil.newURI(TEST_DOMAIN + "test_add_visit"),
     title: "test_add_visit title",
     visits: [],
   };
@@ -634,16 +683,17 @@ let gTests = [
   test_invalid_uri_throws,
   test_invalid_places_throws,
   test_invalid_id_throws,
   test_invalid_guid_throws,
   test_no_id_or_guid_no_visits_throws,
   test_add_visit_no_date_throws,
   test_add_visit_no_transitionType_throws,
   test_add_visit_invalid_transitionType_throws,
+  test_non_addable_uri_errors,
   test_add_visit,
   test_properties_saved,
   test_guid_saved,
   test_referrer_saved,
   test_sessionId_saved,
   test_guid_change_saved,
   test_title_change_saved,
 ];
--- a/toolkit/components/places/tests/unit/test_isvisited.js
+++ b/toolkit/components/places/tests/unit/test_isvisited.js
@@ -77,36 +77,37 @@ function run_test() {
 
   // check if a nonexistent uri is visited
   var uri4 = uri("http://foobarcheese.com");
   do_check_false(gh.isVisited(uri4));
 
   // check that certain schemes never show up as visited
   // even if we attempt to add them to history
   // see CanAddURI() in nsNavHistory.cpp
-  var urlsToIgnore = [
+  const URLS = [
     "about:config",
-    "data:,Hello%2C%20World!",
     "imap://cyrus.andrew.cmu.edu/archive.imap",
-    "news://news.mozilla.org/mozilla.dev.apps.firefox",
-    "moz-anno:favicon:http://www.mozilla.org/2005/made-up-favicon/84-1321",
+    "news://new.mozilla.org/mozilla.dev.apps.firefox",
+    "mailbox:Inbox",
+    "moz-anno:favicon:http://mozilla.org/made-up-favicon",
+    "view-source:http://mozilla.org",
     "chrome://browser/content/browser.xul",
-    "view-source:http://www.google.com/",
-    "javascript:alert('hello world!');",
     "resource://gre-resources/hiddenWindow.html",
+    "data:,Hello%2C%20World!",
+    "wyciwyg:/0/http://mozilla.org",
+    "javascript:alert('hello wolrd!');",
   ];
-
-  for each (var currentURL in urlsToIgnore) {
+  URLS.forEach(function(currentURL) {
     try {
       var cantAddUri = uri(currentURL);
     }
     catch(e) {
       // nsIIOService.newURI() can throw if e.g. our app knows about imap://
       // but the account is not set up and so the URL is invalid for us.
       // Note this in the log but ignore as it's not the subject of this test.
-      print("Exception thrown for '" + currentURL + "', ignored.");
+      do_log_info("Could not construct URI for '" + currentURL + "'; ignoring");
     }
     if (cantAddUri) {
       add_uri_to_history(cantAddUri, false);
       do_check_false(gh.isVisited(cantAddUri));
     }
-  }
+  });
 }