The default bug view has changed. See this FAQ.

docshell should use mozilla::Preferences

RESOLVED FIXED in mozilla7

Status

()

Core
Document Navigation
RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

Trunk
mozilla7
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [inbound])

Attachments

(2 attachments)

(Assignee)

Description

6 years ago
Created attachment 541655 [details] [diff] [review]
Patch v1.0
Attachment #541655 - Flags: review?(bzbarsky)
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.
Attachment #541655 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 2

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/9c2e57bef564

# I'm sorry, I attached -w patch that's the cause of the wrong indentation.
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/9c2e57bef564
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
Created attachment 555482 [details] [diff] [review]
followup: remove now-unused variable |nsresult rv;|

>@@ -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.
Attachment #555482 - Flags: review?(masayuki)
(Assignee)

Updated

6 years ago
Attachment #555482 - Flags: review?(masayuki) → review+
Landed followup: http://hg.mozilla.org/integration/mozilla-inbound/rev/31c4c7ad46d8
http://hg.mozilla.org/mozilla-central/rev/31c4c7ad46d8

Updated

6 years ago
Flags: in-testsuite-

Updated

3 years ago
Blocks: 309654
You need to log in before you can comment on or make changes to this bug.