WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
32718
Reduce #includes in a few high-use headers
https://bugs.webkit.org/show_bug.cgi?id=32718
Summary
Reduce #includes in a few high-use headers
Adam Roben (:aroben)
Reported
2009-12-18 09:31:50 PST
Created
attachment 45156
[details]
Patch Removed unnecessary #includes in these headers: - Frame.h - FrameWin.h - Node.h - Page.h - StringImpl.h The rest of the patch is all adding #includes to source files that now require them. Need a short description and bug URL (OOPS!) Reviewed by NOBODY (OOPS!). WebCore: * dom/Node.h: * page/Frame.h: * page/Page.h: * page/win/FrameWin.h: * platform/text/StringImpl.h: Removed unnecessary #includes. * accessibility/win/AXObjectCacheWin.cpp: * bindings/js/JSDOMWindowBase.cpp: * bindings/js/JSDOMWindowCustom.cpp: * dom/Document.cpp: * dom/InputElement.cpp: * dom/Node.cpp: * dom/SelectElement.cpp: * editing/EditorCommand.cpp: * html/HTMLFormControlElement.cpp: * html/HTMLFormElement.cpp: * html/HTMLMediaElement.cpp: * html/HTMLParser.cpp: * html/HTMLTextAreaElement.cpp: * html/HTMLVideoElement.cpp: * inspector/InspectorController.cpp: * loader/FrameLoader.cpp: * loader/HistoryController.cpp: * loader/appcache/ApplicationCacheGroup.cpp: * page/Console.cpp: * page/EventHandler.cpp: * page/Frame.cpp: * page/FrameView.cpp: * page/Navigator.cpp: * page/PageGroup.cpp: * page/Settings.cpp: * page/mac/EventHandlerMac.mm: * platform/KURL.h: * platform/ScrollbarThemeComposite.cpp: * platform/cf/BinaryPropertyList.cpp: * platform/graphics/mac/GraphicsLayerCA.h: * platform/graphics/win/WKCACFLayerRenderer.cpp: * platform/mac/PopupMenuMac.mm: * platform/mac/WidgetMac.mm: * platform/network/cf/DNSCFNet.cpp: * platform/text/StringImpl.cpp: * platform/win/FileSystemWin.cpp: * platform/win/PlatformScreenWin.cpp: * platform/win/PopupMenuWin.cpp: * platform/win/WidgetWin.cpp: * plugins/PluginView.cpp: * plugins/win/PluginViewWin.cpp: * rendering/InlineTextBox.cpp: * rendering/RenderBox.cpp: * rendering/RenderFileUploadControl.cpp: * rendering/RenderInline.cpp: * rendering/RenderLayerCompositor.cpp: * rendering/RenderObject.cpp: * rendering/RootInlineBox.cpp: * storage/DatabaseTracker.cpp: * storage/DatabaseTracker.h: * storage/SQLTransactionClient.cpp: * svg/graphics/SVGImage.cpp: Added now-needed #includes. WebKit/mac: Add #includes needed after WebCore clean-up * WebView/WebFrame.mm: * WebView/WebFrameView.mm: * WebView/WebView.mm: WebKit/win: Add #includes needed after WebCore clean-up * WebCoreSupport/WebContextMenuClient.cpp: * WebDataSource.cpp: * WebHTMLRepresentation.cpp: * WebView.cpp: * WebView.h: --- 68 files changed, 188 insertions(+), 50 deletions(-)
Attachments
Patch
(31.98 KB, patch)
2009-12-18 09:31 PST
,
Adam Roben (:aroben)
andersca
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Anders Carlsson
Comment 1
2009-12-18 09:34:16 PST
Comment on
attachment 45156
[details]
Patch rs=me
WebKit Review Bot
Comment 2
2009-12-18 09:34:23 PST
Attachment 45156
[details]
did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/page/win/FrameWin.h:36: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1
Darin Adler
Comment 3
2009-12-18 09:36:54 PST
Comment on
attachment 45156
[details]
Patch
> +namespace JSC { > +class UString; > +}
We normally indent the things inside namespace when doing such forward declarations.
Adam Roben (:aroben)
Comment 4
2009-12-18 09:39:25 PST
(In reply to
comment #3
)
> (From update of
attachment 45156
[details]
) > > +namespace JSC { > > +class UString; > > +} > > We normally indent the things inside namespace when doing such forward > declarations.
I waffled over that. I looked through WebCore and didn't see a clearly prevailing style, and our style guidelines don't mention this situation.
Adam Roben (:aroben)
Comment 5
2009-12-18 09:40:15 PST
Committed
r52314
: <
http://trac.webkit.org/changeset/52314
>
Adam Roben (:aroben)
Comment 6
2009-12-18 09:41:02 PST
(In reply to
comment #4
)
> (In reply to
comment #3
) > > (From update of
attachment 45156
[details]
[details]) > > > +namespace JSC { > > > +class UString; > > > +} > > > > We normally indent the things inside namespace when doing such forward > > declarations. > > I waffled over that. I looked through WebCore and didn't see a clearly > prevailing style, and our style guidelines don't mention this situation.
More accurately: I didn't see a clearly prevailing style *among files that don't indent within the WebCore namespace*. Among files that do indent within the WebCore namespace, indenting within other namespaces was (nearly?) always done.
Adam Barth
Comment 7
2009-12-18 15:20:03 PST
Is this a bug in the stylebot then?
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug