Bug 830291 - remove "downloads/destinationFileName" annotation. r=mak draft
authorAsaf Romano <asaf@sent.com>
Fri, 12 May 2017 15:44:25 +0200
changeset 576984 68e554d3eca400062c398cd4398a2a2989668a92
parent 576975 8a73de724f644192973e88b7685689b048023676
child 628381 560e1f60e00e677982f24c443b0695986bf3a5b1
push id58557
push usermak77@bonardo.net
push dateFri, 12 May 2017 16:12:04 +0000
reviewersmak
bugs830291
milestone55.0a1
Bug 830291 - remove "downloads/destinationFileName" annotation. r=mak MozReview-Commit-ID: GznhjKHOr9c
toolkit/components/places/History.cpp
toolkit/components/places/PlacesDBUtils.jsm
toolkit/components/places/tests/unit/test_download_history.js
--- a/toolkit/components/places/History.cpp
+++ b/toolkit/components/places/History.cpp
@@ -57,18 +57,16 @@ namespace places {
 #define URI_VISITED "visited"
 #define URI_NOT_VISITED "not visited"
 #define URI_VISITED_RESOLUTION_TOPIC "visited-status-resolution"
 // Observer event fired after a visit has been registered in the DB.
 #define URI_VISIT_SAVED "uri-visit-saved"
 
 #define DESTINATIONFILEURI_ANNO \
         NS_LITERAL_CSTRING("downloads/destinationFileURI")
-#define DESTINATIONFILENAME_ANNO \
-        NS_LITERAL_CSTRING("downloads/destinationFileName")
 
 ////////////////////////////////////////////////////////////////////////////////
 //// VisitData
 
 struct VisitData {
   VisitData()
   : placeId(0)
   , visitId(0)
@@ -1514,24 +1512,16 @@ public:
     if (!destinationFileURL) {
       return NS_OK;
     }
 
     nsCOMPtr<nsIURI> source;
     nsresult rv = aPlaceInfo->GetUri(getter_AddRefs(source));
     NS_ENSURE_SUCCESS(rv, rv);
 
-    nsCOMPtr<nsIFile> destinationFile;
-    rv = destinationFileURL->GetFile(getter_AddRefs(destinationFile));
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    nsAutoString destinationFileName;
-    rv = destinationFile->GetLeafName(destinationFileName);
-    NS_ENSURE_SUCCESS(rv, rv);
-
     nsAutoCString destinationURISpec;
     rv = destinationFileURL->GetSpec(destinationURISpec);
     NS_ENSURE_SUCCESS(rv, rv);
 
     // Use annotations for storing the additional download metadata.
     nsAnnotationService* annosvc = nsAnnotationService::GetAnnotationService();
     NS_ENSURE_TRUE(annosvc, NS_ERROR_OUT_OF_MEMORY);
 
@@ -1539,34 +1529,33 @@ public:
       source,
       DESTINATIONFILEURI_ANNO,
       NS_ConvertUTF8toUTF16(destinationURISpec),
       0,
       nsIAnnotationService::EXPIRE_WITH_HISTORY
     );
     NS_ENSURE_SUCCESS(rv, rv);
 
-    rv = annosvc->SetPageAnnotationString(
-      source,
-      DESTINATIONFILENAME_ANNO,
-      destinationFileName,
-      0,
-      nsIAnnotationService::EXPIRE_WITH_HISTORY
-    );
-    NS_ENSURE_SUCCESS(rv, rv);
-
     nsAutoString title;
     rv = aPlaceInfo->GetTitle(title);
     NS_ENSURE_SUCCESS(rv, rv);
 
     // In case we are downloading a file that does not correspond to a web
     // page for which the title is present, we populate the otherwise empty
     // history title with the name of the destination file, to allow it to be
     // visible and searchable in history results.
     if (title.IsEmpty()) {
+      nsCOMPtr<nsIFile> destinationFile;
+      rv = destinationFileURL->GetFile(getter_AddRefs(destinationFile));
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      nsAutoString destinationFileName;
+      rv = destinationFile->GetLeafName(destinationFileName);
+      NS_ENSURE_SUCCESS(rv, rv);
+
       rv = mHistory->SetURITitle(source, destinationFileName);
       NS_ENSURE_SUCCESS(rv, rv);
     }
 
     return NS_OK;
   }
 
   NS_IMETHOD HandleCompletion(uint32_t aUpdatedCount) override
--- a/toolkit/components/places/PlacesDBUtils.jsm
+++ b/toolkit/components/places/PlacesDBUtils.jsm
@@ -267,17 +267,18 @@ this.PlacesDBUtils = {
     // A.1 remove obsolete annotations from moz_annos.
     // The 'weave0' idiom exploits character ordering (0 follows /) to
     // efficiently select all annos with a 'weave/' prefix.
     let deleteObsoleteAnnos = DBConn.createAsyncStatement(
       `DELETE FROM moz_annos
        WHERE type = 4
           OR anno_attribute_id IN (
          SELECT id FROM moz_anno_attributes
-         WHERE name BETWEEN 'weave/' AND 'weave0'
+         WHERE name = 'downloads/destinationFileName' OR
+               name BETWEEN 'weave/' AND 'weave0'
        )`);
     cleanupStatements.push(deleteObsoleteAnnos);
 
     // A.2 remove obsolete annotations from moz_items_annos.
     let deleteObsoleteItemsAnnos = DBConn.createAsyncStatement(
       `DELETE FROM moz_items_annos
        WHERE type = 4
           OR anno_attribute_id IN (
--- a/toolkit/components/places/tests/unit/test_download_history.js
+++ b/toolkit/components/places/tests/unit/test_download_history.js
@@ -202,40 +202,35 @@ add_test(function test_dh_details() {
   const SOURCE_URI = NetUtil.newURI("http://example.com/test_dh_details");
   const DEST_FILE_NAME = "dest.txt";
 
   // We must build a real, valid file URI for the destination.
   let destFileUri = NetUtil.newURI(FileUtils.getFile("TmpD", [DEST_FILE_NAME]));
 
   let titleSet = false;
   let destinationFileUriSet = false;
-  let destinationFileNameSet = false;
 
   function checkFinished() {
-    if (titleSet && destinationFileUriSet && destinationFileNameSet) {
+    if (titleSet && destinationFileUriSet) {
       PlacesUtils.annotations.removeObserver(annoObserver);
       PlacesUtils.history.removeObserver(historyObserver);
 
       PlacesTestUtils.clearHistory().then(run_next_test);
     }
   }
 
   let annoObserver = {
     onPageAnnotationSet: function AO_onPageAnnotationSet(aPage, aName) {
       if (aPage.equals(SOURCE_URI)) {
         let value = PlacesUtils.annotations.getPageAnnotation(aPage, aName);
         switch (aName) {
           case "downloads/destinationFileURI":
             destinationFileUriSet = true;
             do_check_eq(value, destFileUri.spec);
             break;
-          case "downloads/destinationFileName":
-            destinationFileNameSet = true;
-            do_check_eq(value, DEST_FILE_NAME);
-            break;
         }
         checkFinished();
       }
     },
     onItemAnnotationSet() {},
     onPageAnnotationRemoved() {},
     onItemAnnotationRemoved() {}
   }