681728 - Clean up inports of GeckoPrefConstants.h in files already importing PreferenceManager.h

Status: RESOLVED FIXED

(Camino Graveyard :: General, defect)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: Fix

(In reply to bug 408592 comment #3)
> Oh, one other question: what's our rule on #imports, specifically
> GeckoPrefConstants.h?
> 
> We get GeckoPrefConstants.h by virtue of PreferenceManager.h #imports it,
> but there are some places (e.g. KeychainService.mm) that include both
> explicitly; the comment
> http://mxr.mozilla.org/camino/source/camino/src/preferences/
> PreferenceManager.h#41 seems to say we should just be using
> PreferenceManager.h.

(In reply to bug 408592 comment #4)
> (In reply to comment #3)
> > but there are some places (e.g. KeychainService.mm) that include both
> > explicitly
> 
> In detail, files that #import or #include GeckoPrefConstants.h (or both it
> and PreferenceManager.h, marked "B"):
> 
>   PreferencePanes/Appearance/Appearance.mm
> B PreferencePanes/Downloads/Downloads.mm
>   PreferencePanes/General/General.mm
>   PreferencePanes/History/History.mm
>   PreferencePanes/Privacy/PrivacyPane.mm
>   PreferencePanes/Security/SecurityPane.mm
>   PreferencePanes/Tabs/Tabs.mm
> B PreferencePanes/WebFeatures/WebFeatures.mm
> B src/browser/CertErrorAboutModule.mm
>   src/embedding/CHBrowserService.mm
> B src/formfill/KeychainService.mm
>   src/preferences/GeckoPrefConstants.mm
>   src/preferences/PreferenceManager.h
> B src/safebrowsing/SafeBrowsingAboutModule.mm
> B src/safebrowsing/SafeBrowsingListManager.mm
> 
> It looks like
> 
> 1) Downloads.mm and WebFeatures.mm added calls to PreferenceManager methods
> after the GeckoPrefConstants rewrite and didn't remove the latter header
> when adding PreferenceManager.h
> 
> 2) SafeBrowsingAboutModule.mm (and by extension its twin,
> CertErrorAboutModule.mm, which was a later wholesale copy of
> SafeBrowsingAboutModule.mm) added both headers at the same time when adding
> a pref/PreferenceManager usage pair
> 
> 3) SafeBrowsingListManager has contained both headers since day 1
> 
> 4) KeychainService has included PreferenceManager.h since day 1, but gained
> GeckoPrefConstants.h in the GeckoPrefConstants rewrite rather than using the
> inheritance in PreferenceManager.h
> 
> So all the double-header cases seem like they're oversights, most of which
> happened as code changed over time. If the comment in PreferenceManager.h is
> correct, as well as my analysis, it seems all 6 of these double-import files
> can be fixed in a subsequent bug.

(In reply to bug 408592 comment #5)
> Correct, this is one place where we explicitly choose to rely on header
> chaining, so you don't need to include it yourself if you have
> PreferenceManager.h

…So this patch removes the superfluous #import in those 6 files.

Comment 1

[Stuart Morgan]

Comment on attachment 555497 [details] [diff] [review]
Fix

sr=smorgan, thanks!

Comment 2

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

http://hg.mozilla.org/camino/rev/bf6c73301b35