Summary: | JS DOM wrappers' impl() functions should return references. | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Andreas Kling <kling> | ||||||||||||||
Component: | WebCore JavaScript | Assignee: | Andreas Kling <kling> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | alecflett, benjamin, cdumez, commit-queue, darin, dino, d-r, eflews.bot, eric.carlson, fmalita, glenn, gtk-ews, gyuyoung.kim, jer.noble, jsbell, kling, kondapallykalyan, pdr, rakuco, rego+ews, schenney, xan.lopez | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Attachments: |
|
Description
Andreas Kling
2013-10-08 00:56:58 PDT
Created attachment 213674 [details]
Patch
Attachment 213674 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/Modules/geolocation/NavigatorGeolocation.cpp', u'Source/WebCore/Modules/geolocation/NavigatorGeolocation.h', u'Source/WebCore/Modules/indexeddb/DOMWindowIndexedDatabase.cpp', u'Source/WebCore/Modules/indexeddb/DOMWindowIndexedDatabase.h', u'Source/WebCore/Modules/indexeddb/WorkerGlobalScopeIndexedDatabase.cpp', u'Source/WebCore/Modules/indexeddb/WorkerGlobalScopeIndexedDatabase.h', u'Source/WebCore/Modules/notifications/DOMWindowNotifications.cpp', u'Source/WebCore/Modules/notifications/DOMWindowNotifications.h', u'Source/WebCore/Modules/notifications/Notification.cpp', u'Source/WebCore/Modules/notifications/WorkerGlobalScopeNotifications.cpp', u'Source/WebCore/Modules/notifications/WorkerGlobalScopeNotifications.h', u'Source/WebCore/Modules/quota/DOMWindowQuota.h', u'Source/WebCore/Modules/speech/DOMWindowSpeechSynthesis.cpp', u'Source/WebCore/Modules/speech/DOMWindowSpeechSynthesis.h', u'Source/WebCore/Modules/webdatabase/DOMWindowWebDatabase.cpp', u'Source/WebCore/Modules/webdatabase/DOMWindowWebDatabase.h', u'Source/WebCore/Modules/webdatabase/WorkerGlobalScopeWebDatabase.cpp', u'Source/WebCore/Modules/webdatabase/WorkerGlobalScopeWebDatabase.h', u'Source/WebCore/WebCore.exp.in', u'Source/WebCore/bindings/generic/BindingSecurity.cpp', u'Source/WebCore/bindings/generic/BindingSecurity.h', u'Source/WebCore/bindings/js/BindingState.cpp', u'Source/WebCore/bindings/js/JSAttrCustom.cpp', u'Source/WebCore/bindings/js/JSAudioBufferSourceNodeCustom.cpp', u'Source/WebCore/bindings/js/JSAudioTrackCustom.cpp', u'Source/WebCore/bindings/js/JSAudioTrackListCustom.cpp', u'Source/WebCore/bindings/js/JSBiquadFilterNodeCustom.cpp', u'Source/WebCore/bindings/js/JSCSSRuleCustom.cpp', u'Source/WebCore/bindings/js/JSCSSRuleListCustom.cpp', u'Source/WebCore/bindings/js/JSCSSStyleDeclarationCustom.cpp', u'Source/WebCore/bindings/js/JSCSSValueCustom.cpp', u'Source/WebCore/bindings/js/JSCanvasRenderingContext2DCustom.cpp', u'Source/WebCore/bindings/js/JSCanvasRenderingContextCustom.cpp', u'Source/WebCore/bindings/js/JSClipboardCustom.cpp', u'Source/WebCore/bindings/js/JSCryptoCustom.cpp', u'Source/WebCore/bindings/js/JSCustomXPathNSResolver.cpp', u'Source/WebCore/bindings/js/JSDOMBinding.cpp', u'Source/WebCore/bindings/js/JSDOMBinding.h', u'Source/WebCore/bindings/js/JSDOMFormDataCustom.cpp', u'Source/WebCore/bindings/js/JSDOMMimeTypeArrayCustom.cpp', u'Source/WebCore/bindings/js/JSDOMPluginArrayCustom.cpp', u'Source/WebCore/bindings/js/JSDOMPluginCustom.cpp', u'Source/WebCore/bindings/js/JSDOMStringListCustom.cpp', u'Source/WebCore/bindings/js/JSDOMStringMapCustom.cpp', u'Source/WebCore/bindings/js/JSDOMWindowBase.cpp', u'Source/WebCore/bindings/js/JSDOMWindowBase.h', u'Source/WebCore/bindings/js/JSDOMWindowCustom.cpp', u'Source/WebCore/bindings/js/JSDOMWindowShell.cpp', u'Source/WebCore/bindings/js/JSDedicatedWorkerGlobalScopeCustom.cpp', u'Source/WebCore/bindings/js/JSDocumentCustom.cpp', u'Source/WebCore/bindings/js/JSEventCustom.cpp', u'Source/WebCore/bindings/js/JSEventListener.cpp', u'Source/WebCore/bindings/js/JSEventTargetCustom.cpp', u'Source/WebCore/bindings/js/JSFileReaderCustom.cpp', u'Source/WebCore/bindings/js/JSHTMLAllCollectionCustom.cpp', u'Source/WebCore/bindings/js/JSHTMLCanvasElementCustom.cpp', u'Source/WebCore/bindings/js/JSHTMLCollectionCustom.cpp', u'Source/WebCore/bindings/js/JSHTMLDocumentCustom.cpp', u'Source/WebCore/bindings/js/JSHTMLElementCustom.cpp', u'Source/WebCore/bindings/js/JSHTMLFormControlsCollectionCustom.cpp', u'Source/WebCore/bindings/js/JSHTMLFormElementCustom.cpp', u'Source/WebCore/bindings/js/JSHTMLFrameElementCustom.cpp', u'Source/WebCore/bindings/js/JSHTMLFrameSetElementCustom.cpp', u'Source/WebCore/bindings/js/JSHTMLInputElementCustom.cpp', u'Source/WebCore/bindings/js/JSHTMLLinkElementCustom.cpp', u'Source/WebCore/bindings/js/JSHTMLMediaElementCustom.cpp', u'Source/WebCore/bindings/js/JSHTMLOptionsCollectionCustom.cpp', u'Source/WebCore/bindings/js/JSHTMLSelectElementCustom.cpp', u'Source/WebCore/bindings/js/JSHistoryCustom.cpp', u'Source/WebCore/bindings/js/JSIDBDatabaseCustom.cpp', u'Source/WebCore/bindings/js/JSIDBObjectStoreCustom.cpp', u'Source/WebCore/bindings/js/JSInjectedScriptHostCustom.cpp', u'Source/WebCore/bindings/js/JSInspectorFrontendHostCustom.cpp', u'Source/WebCore/bindings/js/JSJavaScriptCallFrameCustom.cpp', u'Source/WebCore/bindings/js/JSLocationCustom.cpp', u'Source/WebCore/bindings/js/JSMessageEventCustom.cpp', u'Source/WebCore/bindings/js/JSMessagePortCustom.cpp', u'Source/WebCore/bindings/js/JSMutationObserverCustom.cpp', u'Source/WebCore/bindings/js/JSNamedNodeMapCustom.cpp', u'Source/WebCore/bindings/js/JSNodeCustom.cpp', u'Source/WebCore/bindings/js/JSNodeFilterCustom.cpp', u'Source/WebCore/bindings/js/JSNodeListCustom.cpp', u'Source/WebCore/bindings/js/JSOscillatorNodeCustom.cpp', u'Source/WebCore/bindings/js/JSPannerNodeCustom.cpp', u'Source/WebCore/bindings/js/JSPluginElementFunctions.cpp', u'Source/WebCore/bindings/js/JSPopStateEventCustom.cpp', u'Source/WebCore/bindings/js/JSRTCStatsResponseCustom.cpp', u'Source/WebCore/bindings/js/JSSVGElementInstanceCustom.cpp', u'Source/WebCore/bindings/js/JSSVGLengthCustom.cpp', u'Source/WebCore/bindings/js/JSSharedWorkerCustom.cpp', u'Source/WebCore/bindings/js/JSStorageCustom.cpp', u'Source/WebCore/bindings/js/JSStyleSheetCustom.cpp', u'Source/WebCore/bindings/js/JSStyleSheetListCustom.cpp', u'Source/WebCore/bindings/js/JSTextTrackCueCustom.cpp', u'Source/WebCore/bindings/js/JSTextTrackCustom.cpp', u'Source/WebCore/bindings/js/JSTextTrackListCustom.cpp', u'Source/WebCore/bindings/js/JSTrackCustom.cpp', u'Source/WebCore/bindings/js/JSTrackEventCustom.cpp', u'Source/WebCore/bindings/js/JSVideoTrackCustom.cpp', u'Source/WebCore/bindings/js/JSVideoTrackListCustom.cpp', u'Source/WebCore/bindings/js/JSWebGLRenderingContextCustom.cpp', u'Source/WebCore/bindings/js/JSWorkerCustom.cpp', u'Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.h', u'Source/WebCore/bindings/js/JSWorkerGlobalScopeCustom.cpp', u'Source/WebCore/bindings/js/JSXMLHttpRequestCustom.cpp', u'Source/WebCore/bindings/js/JSXPathResultCustom.cpp', u'Source/WebCore/bindings/js/JSXSLTProcessorCustom.cpp', u'Source/WebCore/bindings/js/PageScriptDebugServer.cpp', u'Source/WebCore/bindings/js/ScheduledAction.cpp', u'Source/WebCore/bindings/js/ScriptCachedFrameData.cpp', u'Source/WebCore/bindings/js/ScriptController.cpp', u'Source/WebCore/bindings/js/ScriptDebugServer.cpp', u'Source/WebCore/bindings/js/ScriptState.cpp', u'Source/WebCore/bindings/objc/DOM.mm', u'Source/WebCore/bindings/objc/DOMImplementationFront.cpp', u'Source/WebCore/bindings/objc/DOMUtility.mm', u'Source/WebCore/bindings/objc/WebScriptObject.mm', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/bindings/scripts/test/JS/JSTestActiveDOMObject.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestActiveDOMObject.h', u'Source/WebCore/bindings/scripts/test/JS/JSTestCustomNamedGetter.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestCustomNamedGetter.h', u'Source/WebCore/bindings/scripts/test/JS/JSTestEventConstructor.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestEventConstructor.h', u'Source/WebCore/bindings/scripts/test/JS/JSTestEventTarget.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestEventTarget.h', u'Source/WebCore/bindings/scripts/test/JS/JSTestException.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestException.h', u'Source/WebCore/bindings/scripts/test/JS/JSTestInterface.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestInterface.h', u'Source/WebCore/bindings/scripts/test/JS/JSTestMediaQueryListListener.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestMediaQueryListListener.h', u'Source/WebCore/bindings/scripts/test/JS/JSTestNamedConstructor.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestNamedConstructor.h', u'Source/WebCore/bindings/scripts/test/JS/JSTestNode.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestNode.h', u'Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestObj.h', u'Source/WebCore/bindings/scripts/test/JS/JSTestOverloadedConstructors.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestOverloadedConstructors.h', u'Source/WebCore/bindings/scripts/test/JS/JSTestSerializedScriptValueInterface.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestSerializedScriptValueInterface.h', u'Source/WebCore/bindings/scripts/test/JS/JSTestTypedefs.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestTypedefs.h', u'Source/WebCore/bindings/scripts/test/JS/JSattribute.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSattribute.h', u'Source/WebCore/bindings/scripts/test/JS/JSreadonly.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSreadonly.h', u'Source/WebCore/bridge/runtime_method.cpp', u'Source/WebCore/inspector/InspectorIndexedDBAgent.cpp', u'Source/WebCore/svg/properties/SVGStaticListPropertyTearOff.h', u'Source/WebCore/svg/properties/SVGStaticPropertyTearOff.h', u'Source/WebCore/svg/properties/SVGStaticPropertyWithParentTearOff.h', u'Source/WebCore/testing/Internals.cpp', u'Source/WebKit/mac/WebView/WebFrame.mm', u'Source/WebKit/mac/WebView/WebScriptDebugger.mm', u'Source/WebKit2/WebProcess/WebPage/WebFrame.cpp']" exit_code: 1
Source/WebCore/bindings/js/JSEventTargetCustom.cpp:69: Should have a space between // and comment [whitespace/comments] [4]
Total errors found: 1 in 158 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 213674 [details] Patch Attachment 213674 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/3342113 Created attachment 213676 [details]
Patch
Comment on attachment 213676 [details] Patch Attachment 213676 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/3738047 Comment on attachment 213676 [details] Patch Attachment 213676 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/3737031 Created attachment 213678 [details]
Patch
Comment on attachment 213678 [details] Patch Attachment 213678 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/3712138 Comment on attachment 213678 [details] Patch Attachment 213678 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/3716137 Comment on attachment 213678 [details] Patch Attachment 213678 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/3706130 Comment on attachment 213678 [details] Patch Attachment 213678 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/3709149 Created attachment 213858 [details]
Patch
Created attachment 213861 [details]
Patch
Comment on attachment 213861 [details] Patch Attachment 213861 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/3783025 Comment on attachment 213861 [details] Patch Attachment 213861 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/3400054 Created attachment 213864 [details]
Patch
Comment on attachment 213864 [details] Patch Clearing flags on attachment: 213864 Committed r157215: <http://trac.webkit.org/changeset/157215> All reviewed patches have been landed. Closing bug. We should change the name of this function from impl to something made out of words. Maybe wrappedObject? Maybe something good but still shorter? (In reply to comment #19) > We should change the name of this function from impl to something made out of words. Maybe wrappedObject? Maybe something good but still shorter? Foo& JSFoo::core(); maybe? |