Bug 327331 - nsNavHistory::Init allows improper initialization. r=dietrich. This is a comment only change patch to better document the methods used in nsNavHistory::Init for this patchers sanity. No code was changed in the making of this patch.

toolkit/components/places/src/nsNavHistory.cpp

@@ -441,10 +441,13 @@ nsNavHistory::Init()
   NS_ENSURE_TRUE(mRecentBookmark.Init(128), NS_ERROR_OUT_OF_MEMORY);
   NS_ENSURE_TRUE(mRecentRedirects.Init(128), NS_ERROR_OUT_OF_MEMORY);
 
-  // The AddObserver calls must be the last lines in this function, because
-  // this function may fail, and thus, this object would be not completely
-  // initialized), but the observerservice would still keep a reference to us
-  // and notify us about shutdown, which may cause crashes.
+  /*****************************************************************************
+   *** IMPORTANT NOTICE!
+   ***
+   *** Nothing after these add observer calls should return anything but NS_OK.
+   *** If a failure code is returned, this nsNavHistory object will be held onto
+   *** by the observer service and the preference service. 
+   ****************************************************************************/
 
   nsCOMPtr<nsIObserverService> observerService =
     do_GetService("@mozilla.org/observer-service;1", &rv);
@@ -464,6 +467,14 @@ nsNavHistory::Init()
   observerService->AddObserver(this, gQuitApplicationMessage, PR_FALSE);
   observerService->AddObserver(this, gXpcomShutdown, PR_FALSE);
 
+  /*****************************************************************************
+   *** IMPORTANT NOTICE!
+   ***
+   *** NO CODE SHOULD GO BEYOND THIS POINT THAT WOULD PROPAGATE AN ERROR.  IN
+   *** OTHER WORDS, THE ONLY THING THAT SHOULD BE RETURNED AFTER THIS POINT IS
+   *** NS_OK.
+   ****************************************************************************/
+
   if (migrationType == DB_MIGRATION_CREATED) {
     nsCOMPtr<nsIFile> historyFile;
     rv = NS_GetSpecialDirectory(NS_APP_HISTORY_50_FILE,

toolkit/components/places/src/nsNavHistory.h

@@ -396,6 +396,15 @@ protected:
   nsCOMPtr<mozIStorageStatement> mDBUrlToUrlResult; // kGetInfoIndex_* results
   nsCOMPtr<mozIStorageStatement> mDBBookmarkToUrlResult; // kGetInfoIndex_* results
 
+  /**
+   * Recalculates aCount frecencies.  If aRecalcOld, it will also calculate
+   * the frecency of aCount history visits that have not occurred recently.
+   *
+   * @param aCount
+   *        The number of entries to update.
+   * @param aRecalcOld
+   *        Indicates that we should update old visits as well.
+   */
   nsresult RecalculateFrecencies(PRInt32 aCount, PRBool aRecalcOld);
   nsresult RecalculateFrecenciesInternal(mozIStorageStatement *aStatement, PRInt64 aBindParameter, PRInt32 aCount);
 
@@ -410,8 +419,36 @@ protected:
   nsCOMPtr<mozIStorageStatement> mDBVisitCountForFrecency;
   nsCOMPtr<mozIStorageStatement> mDBTrueVisitCount;
 
+  /**
+   * Initializes the database file.  If the database does not exist, was
+   * corrupted, or aForceInit is true, we recreate the database.  We also backup
+   * the database if it was corrupted or aForceInit is true.
+   *
+   * @param aForceInit
+   *        Indicates if we should close an open database connection or not.
+   */
   nsresult InitDBFile(PRBool aForceInit);
+
+  /**
+   * Creates a uniquely named backup of the places database.
+   */
   nsresult BackupDBFile();
+
+  /**
+   * Initializes the database.  This performs any necessary migrations for the
+   * database.  All migration is done inside a transaction that is rolled back
+   * if any error occurs.  Upon initialization, history is imported, and some
+   * preferences that are used are set.
+   *
+   * @param aMadeChanges [out]
+   *        Returns a constant indicating what occurred:
+   *        DB_MIGRATION_NONE
+   *          No migration occurred.
+   *        DB_MIGRATION_CREATED
+   *          The database did not exist in the past, and was created.
+   *        DB_MIGRATION_UPDATED
+   *          The database was migrated to a new version.
+   */
   nsresult InitDB(PRInt16 *aMadeChanges);
   nsresult InitStatements();
   nsresult ForceMigrateBookmarksDB(mozIStorageConnection *aDBConn);
@@ -446,6 +483,14 @@ protected:
   PRBool FindLastVisit(nsIURI* aURI, PRInt64* aVisitID,
                        PRInt64* aSessionID);
   PRBool IsURIStringVisited(const nsACString& url);
+
+  /**
+   * This loads all of the preferences that we use into member variables.
+   * NOTE:  If mPrefBranch is NULL, this does nothing.
+   *
+   * @param aInitializing
+   *        Indicates if the autocomplete queries should be regenerated or not.
+   */
   nsresult LoadPrefs(PRBool aInitializing);
 
   // Current time optimization
@@ -680,6 +725,10 @@ protected:
                            nsCOMArray<nsNavHistoryQuery>* aQueries,
                            nsNavHistoryQueryOptions* aOptions);
 
+  /**
+   * Used to setup the idle timer used to perform various tasks when the user is
+   * idle..
+   */
   nsCOMPtr<nsITimer> mIdleTimer;
   nsresult InitializeIdleTimer();
   static void IdleTimerCallback(nsITimer* aTimer, void* aClosure);