Comment on attachment 541655 [details] [diff] [review] Patch v1.0 >+ if (prefPrefix) // XXX why don't you test by IsEmpty()? Because having an empty prefix is a totally reasonable thing to want. Please remove that comment. >+ if (prefSuffix) // XXX why don't you test by IsEmpty()? Likewise. >+++ b/docshell/base/nsDocShell.cpp > PingsEnabled(PRInt32 *maxPerLink, PRBool *requireSameHost) > if (allow) { >+ Preferences::GetInt(PREF_PINGS_MAX_PER_LINK, maxPerLink); >+ Preferences::GetBool(PREF_PINGS_REQUIRE_SAME_HOST, requireSameHost); > } Fix the indent, please. > nsDocShell::SetUseErrorPages(PRBool aUseErrorPages) > if (mObserveErrorPages) { >+ Preferences::RemoveObserver(this, "browser.xul.error_pages.enabled"); > mObserveErrorPages = PR_FALSE; > } Fix indent here too. >@@ -4555,22 +4529,19 @@ nsDocShell::Destroy() > if (mObserveErrorPages) { >+ Preferences::RemoveObserver(this, "browser.xul.error_pages.enabled"); > mObserveErrorPages = PR_FALSE; > } And here. >@@ -9207,33 +9170,27 @@ nsresult nsDocShell::DoChannelLoad(nsICh >+ switch (Preferences::GetInt("browser.cache.check_doc_frequency", -1)) { And around here. >+++ b/docshell/shistory/src/nsSHistory.cpp > nsSHistoryObserver::Observe(nsISupports *aSubject, const char *aTopic, .... > if (!strcmp(aTopic, NS_PREFBRANCH_PREFCHANGE_TOPIC_ID)) { >+ nsSHistory::UpdatePrefs(); > nsSHistory::EvictGlobalContentViewer(); > } else if (!strcmp(aTopic, NS_CACHESERVICE_EMPTYCACHE_TOPIC_ID) || And here. >+nsSHistory::UpdatePrefs() >+ Preferences::GetInt(PREF_SHISTORY_MAX_TOTAL_VIEWERS, > &sHistoryMaxTotalViewers); And here. > nsSHistory::Startup() And all over the place here. >+ if (!gObserver) { Can't happen, so don't check for it. r=me with the nits fixed.

http://hg.mozilla.org/integration/mozilla-inbound/rev/9c2e57bef564 # I'm sorry, I attached -w patch that's the cause of the wrong indentation.

>@@ -484,27 +466,27 @@ PRBool nsDefaultURIFixup::MakeAlternateU > > > nsresult rv; > > // Get the prefix and suffix to stick onto the new hostname. By default these > // are www. & .com but they could be any other value, e.g. www. & .org > > nsCAutoString prefix("www."); >- nsXPIDLCString prefPrefix; >- rv = mPrefBranch->GetCharPref("browser.fixup.alternate.prefix", getter_Copies(prefPrefix)); >- if (NS_SUCCEEDED(rv)) [...] > nsCAutoString suffix(".com"); >- nsXPIDLCString prefSuffix; >- rv = mPrefBranch->GetCharPref("browser.fixup.alternate.suffix", getter_Copies(prefSuffix)); >- if (NS_SUCCEEDED(rv)) These removed lines ^ were the only uses of the variable |nsresult rv| declared at the top of the context quoted there. So we now get this build warning: > docshell/base/nsDefaultURIFixup.cpp: In member function ‘PRBool nsDefaultURIFixup::MakeAlternateURI(nsIURI*)’ > docshell/base/nsDefaultURIFixup.cpp:468:14: warning: unused variable ‘rv’ [-Wunused-variable] The |nsresult rv;| can just go away. Attached followup-patch fixes this.

Landed followup: http://hg.mozilla.org/integration/mozilla-inbound/rev/31c4c7ad46d8