From 3b15782065c4b173038a05e13f61c47ac6deff78 Mon Sep 17 00:00:00 2001 From: "dtownsend@oxymoronical.com" Date: Wed, 7 May 2008 06:18:38 -0700 Subject: [PATCH] Backing out bug 432492 --- .../public/nsIUrlClassifierDBService.idl | 4 +- .../src/nsUrlClassifierDBService.cpp | 154 ++++-------------- .../src/nsUrlClassifierStreamUpdater.cpp | 89 +++------- .../src/nsUrlClassifierStreamUpdater.h | 8 +- 4 files changed, 56 insertions(+), 199 deletions(-) diff --git a/toolkit/components/url-classifier/public/nsIUrlClassifierDBService.idl b/toolkit/components/url-classifier/public/nsIUrlClassifierDBService.idl index 0f7574a9a06..e9386fb060e 100644 --- a/toolkit/components/url-classifier/public/nsIUrlClassifierDBService.idl +++ b/toolkit/components/url-classifier/public/nsIUrlClassifierDBService.idl @@ -83,10 +83,8 @@ interface nsIUrlClassifierUpdateObserver : nsISupports { * A stream update has completed. * * @param status The state of the update process. - * @param delay The amount of time the updater should wait to fetch the - * next URL in ms. */ - void streamFinished(in nsresult status, in unsigned long delay); + void streamFinished(in nsresult status); /* The update has encountered an error and should be cancelled */ void updateError(in nsresult error); diff --git a/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp b/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp index c2619783af2..7abcd41c25a 100644 --- a/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp +++ b/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp @@ -154,17 +154,6 @@ static const PRLogModuleInfo *gUrlClassifierDbServiceLog = nsnull; #define UPDATE_CACHE_SIZE_PREF "urlclassifier.updatecachemax" #define UPDATE_CACHE_SIZE_DEFAULT -1 -// Amount of time to spend updating before committing and delaying, in -// seconds. This is checked after each update stream, so the actual -// time spent can be higher than this, depending on update stream size. -#define UPDATE_WORKING_TIME "urlclassifier.workingtime" -#define UPDATE_WORKING_TIME_DEFAULT 5 - -// The amount of time to delay after hitting UPDATE_WORKING_TIME, in -// seconds. -#define UPDATE_DELAY_TIME "urlclassifier.updatetime" -#define UPDATE_DELAY_TIME_DEFAULT 60 - #define PAGE_SIZE 4096 class nsUrlClassifierDBServiceWorker; @@ -183,9 +172,6 @@ static PRInt32 gFreshnessGuarantee = CONFIRM_AGE_DEFAULT_SEC; static PRInt32 gUpdateCacheSize = UPDATE_CACHE_SIZE_DEFAULT; -static PRInt32 gWorkingTimeThreshold = UPDATE_WORKING_TIME_DEFAULT; -static PRInt32 gDelayTime = UPDATE_DELAY_TIME_DEFAULT; - static void SplitTables(const nsACString& str, nsTArray& tables) { @@ -1116,12 +1102,6 @@ private: // Handle chunk data from a stream update nsresult ProcessChunk(PRBool* done); - // Sets up a transaction and begins counting update time. - nsresult SetupUpdate(); - - // Applies the current transaction and resets the update/working times. - nsresult ApplyUpdate(); - // Reset the in-progress update stream void ResetStream(); @@ -1237,10 +1217,6 @@ private: // The MAC stated by the server. nsCString mServerMAC; - // Start time of the current update interval. This will be reset - // every time we apply the update. - PRIntervalTime mUpdateStartTime; - nsCOMPtr mHMAC; // The number of noise entries to add to the set of lookup results. PRInt32 mGethashNoise; @@ -1280,7 +1256,6 @@ nsUrlClassifierDBServiceWorker::nsUrlClassifierDBServiceWorker() , mCachedListsTable(PR_UINT32_MAX) , mHaveCachedAddChunks(PR_FALSE) , mHaveCachedSubChunks(PR_FALSE) - , mUpdateStartTime(0) , mGethashNoise(0) , mPendingLookupLock(nsnull) { @@ -2782,7 +2757,19 @@ nsUrlClassifierDBServiceWorker::BeginUpdate(nsIUrlClassifierUpdateObserver *obse return rv; } - rv = SetupUpdate(); + if (gUpdateCacheSize > 0) { + PRUint32 cachePages = gUpdateCacheSize / PAGE_SIZE; + nsCAutoString cacheSizePragma("PRAGMA cache_size="); + cacheSizePragma.AppendInt(cachePages); + rv = mConnection->ExecuteSimpleSQL(cacheSizePragma); + if (NS_FAILED(rv)) { + mUpdateStatus = rv; + return rv; + } + mGrewCache = PR_TRUE; + } + + rv = mConnection->BeginTransaction(); if (NS_FAILED(rv)) { mUpdateStatus = rv; return rv; @@ -2814,16 +2801,10 @@ nsUrlClassifierDBServiceWorker::BeginStream(const nsACString &table, NS_ENSURE_STATE(mUpdateObserver); NS_ENSURE_STATE(!mInStream); - // We may have committed the update in FinishStream, if so set it up - // again here. - nsresult rv = SetupUpdate(); - if (NS_FAILED(rv)) { - mUpdateStatus = rv; - return rv; - } - mInStream = PR_TRUE; + nsresult rv; + // If we're expecting a MAC, create the nsICryptoHMAC component now. if (!mUpdateClientKey.IsEmpty()) { nsCOMPtr keyObjectFactory(do_GetService( @@ -2960,8 +2941,6 @@ nsUrlClassifierDBServiceWorker::FinishStream() NS_ENSURE_STATE(mInStream); NS_ENSURE_STATE(mUpdateObserver); - PRInt32 nextStreamDelay = 0; - if (NS_SUCCEEDED(mUpdateStatus) && mHMAC) { nsCAutoString clientMAC; mHMAC->Finish(PR_TRUE, clientMAC); @@ -2972,80 +2951,15 @@ nsUrlClassifierDBServiceWorker::FinishStream() mServerMAC.get(), clientMAC.get())); mUpdateStatus = NS_ERROR_FAILURE; } - PRIntervalTime updateTime = PR_IntervalNow() - mUpdateStartTime; - if (PR_IntervalToSeconds(updateTime) >= - static_cast(gWorkingTimeThreshold)) { - // We've spent long enough working that we should commit what we - // have and hold off for a bit. - ApplyUpdate(); - - nextStreamDelay = gDelayTime * 1000; - } } - mUpdateObserver->StreamFinished(mUpdateStatus, - static_cast(nextStreamDelay)); + mUpdateObserver->StreamFinished(mUpdateStatus); ResetStream(); return NS_OK; } -nsresult -nsUrlClassifierDBServiceWorker::SetupUpdate() -{ - LOG(("nsUrlClassifierDBServiceWorker::SetupUpdate")); - PRBool inProgress; - nsresult rv = mConnection->GetTransactionInProgress(&inProgress); - if (inProgress) { - return NS_OK; - } - - mUpdateStartTime = PR_IntervalNow(); - - rv = mConnection->BeginTransaction(); - NS_ENSURE_SUCCESS(rv, rv); - - if (gUpdateCacheSize > 0) { - PRUint32 cachePages = gUpdateCacheSize / PAGE_SIZE; - nsCAutoString cacheSizePragma("PRAGMA cache_size="); - cacheSizePragma.AppendInt(cachePages); - rv = mConnection->ExecuteSimpleSQL(cacheSizePragma); - NS_ENSURE_SUCCESS(rv, rv); - mGrewCache = PR_TRUE; - } - - return NS_OK; -} - -nsresult -nsUrlClassifierDBServiceWorker::ApplyUpdate() -{ - LOG(("nsUrlClassifierDBServiceWorker::ApplyUpdate")); - - if (NS_FAILED(mUpdateStatus)) { - mConnection->RollbackTransaction(); - } else { - mUpdateStatus = FlushChunkLists(); - if (NS_SUCCEEDED(mUpdateStatus)) { - mUpdateStatus = mConnection->CommitTransaction(); - } - } - - if (mGrewCache) { - // During the update we increased the page cache to bigger than we - // want to keep around. At the moment, the only reliable way to make - // sure that the page cache is freed is to reopen the connection. - mGrewCache = PR_FALSE; - CloseDb(); - OpenDb(); - } - - mUpdateStartTime = 0; - - return NS_OK; -} - NS_IMETHODIMP nsUrlClassifierDBServiceWorker::FinishUpdate() { @@ -3055,7 +2969,16 @@ nsUrlClassifierDBServiceWorker::FinishUpdate() NS_ENSURE_STATE(!mInStream); NS_ENSURE_STATE(mUpdateObserver); - ApplyUpdate(); + if (NS_SUCCEEDED(mUpdateStatus)) { + mUpdateStatus = FlushChunkLists(); + } + + nsCAutoString arg; + if (NS_SUCCEEDED(mUpdateStatus)) { + mUpdateStatus = mConnection->CommitTransaction(); + } else { + mConnection->RollbackTransaction(); + } if (NS_SUCCEEDED(mUpdateStatus)) { mUpdateObserver->UpdateSuccess(mUpdateWait); @@ -3088,6 +3011,13 @@ nsUrlClassifierDBServiceWorker::FinishUpdate() // database reset. if (NS_SUCCEEDED(mUpdateStatus) && resetRequested) { ResetDatabase(); + } else if (mGrewCache) { + // During the update we increased the page cache to bigger than we + // want to keep around. At the moment, the only reliable way to make + // sure that the page cache is freed is to reopen the connection. + mGrewCache = PR_FALSE; + CloseDb(); + OpenDb(); } return NS_OK; @@ -3738,14 +3668,6 @@ nsUrlClassifierDBService::Init() rv = prefs->GetIntPref(UPDATE_CACHE_SIZE_PREF, &tmpint); PR_AtomicSet(&gUpdateCacheSize, NS_SUCCEEDED(rv) ? tmpint : UPDATE_CACHE_SIZE_DEFAULT); - - rv = prefs->GetIntPref(UPDATE_WORKING_TIME, &tmpint); - PR_AtomicSet(&gWorkingTimeThreshold, - NS_SUCCEEDED(rv) ? tmpint : UPDATE_WORKING_TIME_DEFAULT); - - rv = prefs->GetIntPref(UPDATE_DELAY_TIME, &tmpint); - PR_AtomicSet(&gDelayTime, - NS_SUCCEEDED(rv) ? tmpint : UPDATE_DELAY_TIME_DEFAULT); } // Start the background thread. @@ -4038,16 +3960,6 @@ nsUrlClassifierDBService::Observe(nsISupports *aSubject, const char *aTopic, PRInt32 tmpint; rv = prefs->GetIntPref(UPDATE_CACHE_SIZE_PREF, &tmpint); PR_AtomicSet(&gUpdateCacheSize, NS_SUCCEEDED(rv) ? tmpint : UPDATE_CACHE_SIZE_DEFAULT); - } else if (NS_LITERAL_STRING(UPDATE_WORKING_TIME).Equals(aData)) { - PRInt32 tmpint; - rv = prefs->GetIntPref(UPDATE_WORKING_TIME, &tmpint); - PR_AtomicSet(&gWorkingTimeThreshold, - NS_SUCCEEDED(rv) ? tmpint : UPDATE_WORKING_TIME_DEFAULT); - } else if (NS_LITERAL_STRING(UPDATE_DELAY_TIME).Equals(aData)) { - PRInt32 tmpint; - rv = prefs->GetIntPref(UPDATE_DELAY_TIME, &tmpint); - PR_AtomicSet(&gDelayTime, - NS_SUCCEEDED(rv) ? tmpint : UPDATE_DELAY_TIME_DEFAULT); } } else if (!strcmp(aTopic, "profile-before-change") || !strcmp(aTopic, "xpcom-shutdown-threads")) { diff --git a/toolkit/components/url-classifier/src/nsUrlClassifierStreamUpdater.cpp b/toolkit/components/url-classifier/src/nsUrlClassifierStreamUpdater.cpp index fa1086a7cdc..5dad171e349 100644 --- a/toolkit/components/url-classifier/src/nsUrlClassifierStreamUpdater.cpp +++ b/toolkit/components/url-classifier/src/nsUrlClassifierStreamUpdater.cpp @@ -75,7 +75,7 @@ nsUrlClassifierStreamUpdater::nsUrlClassifierStreamUpdater() } -NS_IMPL_THREADSAFE_ISUPPORTS9(nsUrlClassifierStreamUpdater, +NS_IMPL_THREADSAFE_ISUPPORTS8(nsUrlClassifierStreamUpdater, nsIUrlClassifierStreamUpdater, nsIUrlClassifierUpdateObserver, nsIRequestObserver, @@ -83,8 +83,7 @@ NS_IMPL_THREADSAFE_ISUPPORTS9(nsUrlClassifierStreamUpdater, nsIObserver, nsIBadCertListener2, nsISSLErrorListener, - nsIInterfaceRequestor, - nsITimerCallback) + nsIInterfaceRequestor) /** * Clear out the update. @@ -272,54 +271,29 @@ nsUrlClassifierStreamUpdater::RekeyRequested() nsnull); } -nsresult -nsUrlClassifierStreamUpdater::FetchNext() -{ - if (mPendingUpdates.Length() == 0) { - return NS_OK; - } - - PendingUpdate &update = mPendingUpdates[0]; - LOG(("Fetching update url: %s\n", update.mUrl.get())); - nsresult rv = FetchUpdate(update.mUrl, EmptyCString(), - update.mTable, update.mServerMAC); - if (NS_FAILED(rv)) { - LOG(("Error fetching update url: %s\n", update.mUrl.get())); - // We can commit the urls that we've applied so far. This is - // probably a transient server problem, so trigger backoff. - mDownloadErrorCallback->HandleEvent(EmptyCString()); - mDownloadError = PR_TRUE; - mDBService->FinishUpdate(); - return rv; - } - - mPendingUpdates.RemoveElementAt(0); - - return NS_OK; -} - NS_IMETHODIMP -nsUrlClassifierStreamUpdater::StreamFinished(nsresult status, - PRUint32 requestedDelay) +nsUrlClassifierStreamUpdater::StreamFinished(nsresult status) { - LOG(("nsUrlClassifierStreamUpdater::StreamFinished [%x, %d]", status, requestedDelay)); - if (NS_FAILED(status) || mPendingUpdates.Length() == 0) { - // We're done. - mDBService->FinishUpdate(); - return NS_OK; - } - - // Wait the requested amount of time before starting a new stream. nsresult rv; - mTimer = do_CreateInstance("@mozilla.org/timer;1", &rv); - if (NS_SUCCEEDED(rv)) { - rv = mTimer->InitWithCallback(this, requestedDelay, - nsITimer::TYPE_ONE_SHOT); - } - if (NS_FAILED(rv)) { - NS_WARNING("Unable to initialize timer, fetching next safebrowsing item immediately"); - return FetchNext(); + // Pop off a pending URL and update it. + if (NS_SUCCEEDED(status) && mPendingUpdates.Length() > 0) { + PendingUpdate &update = mPendingUpdates[0]; + rv = FetchUpdate(update.mUrl, EmptyCString(), + update.mTable, update.mServerMAC); + if (NS_FAILED(rv)) { + LOG(("Error fetching update url: %s\n", update.mUrl.get())); + // We can commit the urls that we've applied so far. This is + // probably a transient server problem, so trigger backoff. + mDownloadErrorCallback->HandleEvent(EmptyCString()); + mDownloadError = PR_TRUE; + mDBService->FinishUpdate(); + return rv; + } + + mPendingUpdates.RemoveElementAt(0); + } else { + mDBService->FinishUpdate(); } return NS_OK; @@ -526,10 +500,6 @@ nsUrlClassifierStreamUpdater::Observe(nsISupports *aSubject, const char *aTopic, mIsUpdating = PR_FALSE; mChannel = nsnull; } - if (mTimer) { - mTimer->Cancel(); - mTimer = nsnull; - } } return NS_OK; } @@ -568,20 +538,3 @@ nsUrlClassifierStreamUpdater::GetInterface(const nsIID & eventSinkIID, void* *_r { return QueryInterface(eventSinkIID, _retval); } - - -/////////////////////////////////////////////////////////////////////////////// -// nsITimerCallback implementation -NS_IMETHODIMP -nsUrlClassifierStreamUpdater::Notify(nsITimer *timer) -{ - LOG(("nsUrlClassifierStreamUpdater::Notify [%p]", this)); - - mTimer = nsnull; - - // Start the update process up again. - FetchNext(); - - return NS_OK; -} - diff --git a/toolkit/components/url-classifier/src/nsUrlClassifierStreamUpdater.h b/toolkit/components/url-classifier/src/nsUrlClassifierStreamUpdater.h index 7279d72e8bb..5048b322f37 100644 --- a/toolkit/components/url-classifier/src/nsUrlClassifierStreamUpdater.h +++ b/toolkit/components/url-classifier/src/nsUrlClassifierStreamUpdater.h @@ -49,7 +49,6 @@ #include "nsTArray.h" #include "nsIBadCertListener2.h" #include "nsISSLErrorListener.h" -#include "nsITimer.h" // Forward declare pointers class nsIURI; @@ -60,8 +59,7 @@ class nsUrlClassifierStreamUpdater : public nsIUrlClassifierStreamUpdater, public nsIObserver, public nsIBadCertListener2, public nsISSLErrorListener, - public nsIInterfaceRequestor, - public nsITimerCallback + public nsIInterfaceRequestor { public: nsUrlClassifierStreamUpdater(); @@ -75,7 +73,6 @@ public: NS_DECL_NSIBADCERTLISTENER2 NS_DECL_NSISSLERRORLISTENER NS_DECL_NSIOBSERVER - NS_DECL_NSITIMERCALLBACK private: // No subclassing @@ -99,8 +96,6 @@ private: const nsACString &aTable, const nsACString &aServerMAC); - nsresult FetchNext(); - PRBool mIsUpdating; PRBool mInitialized; PRBool mDownloadError; @@ -110,7 +105,6 @@ private: nsCString mServerMAC; nsCOMPtr mChannel; nsCOMPtr mDBService; - nsCOMPtr mTimer; struct PendingUpdate { nsCString mUrl;