RESOLVED FIXED 122497
JS DOM wrappers' impl() functions should return references. 
https://bugs.webkit.org/show_bug.cgi?id=122497
Summary JS DOM wrappers' impl() functions should return references.
Andreas Kling
Reported 2013-10-08 00:56:58 PDT
JS DOM wrappers always have a corresponding WebCore object, so impl() should really return a reference.
Attachments
Patch (392.71 KB, patch)
2013-10-08 01:06 PDT, Andreas Kling 		eflews.bot: commit-queue-
DetailsFormatted DiffDiff
Patch (392.97 KB, patch)
2013-10-08 01:27 PDT, Andreas Kling 		eflews.bot: commit-queue-
DetailsFormatted DiffDiff
Patch (397.49 KB, patch)
2013-10-08 01:46 PDT, Andreas Kling 		eflews.bot: commit-queue-
DetailsFormatted DiffDiff
Patch (377.50 KB, patch)
2013-10-10 01:44 PDT, Andreas Kling 		no flags
DetailsFormatted DiffDiff
Patch (376.89 KB, patch)
2013-10-10 02:25 PDT, Andreas Kling 		eflews.bot: commit-queue-
DetailsFormatted DiffDiff
Patch (380.19 KB, patch)
2013-10-10 03:41 PDT, Andreas Kling 		no flags
DetailsFormatted DiffDiff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.
Andreas Kling
Comment 1 2013-10-08 01:06:21 PDT
Created attachment 213674 [details] Patch
WebKit Commit Bot
Comment 2 2013-10-08 01:08:25 PDT
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.
EFL EWS Bot
Comment 3 2013-10-08 01:18:33 PDT
Comment on attachment 213674 [details] Patch Attachment 213674 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/3342113
Andreas Kling
Comment 4 2013-10-08 01:27:12 PDT
Created attachment 213676 [details] Patch
EFL EWS Bot
Comment 5 2013-10-08 01:39:30 PDT
Comment on attachment 213676 [details] Patch Attachment 213676 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/3738047
kov's GTK+ EWS bot
Comment 6 2013-10-08 01:41:55 PDT
Comment on attachment 213676 [details] Patch Attachment 213676 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/3737031
Andreas Kling
Comment 7 2013-10-08 01:46:15 PDT
Created attachment 213678 [details] Patch
EFL EWS Bot
Comment 8 2013-10-08 01:54:50 PDT
Comment on attachment 213678 [details] Patch Attachment 213678 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/3712138
kov's GTK+ EWS bot
Comment 9 2013-10-08 01:56:27 PDT
Comment on attachment 213678 [details] Patch Attachment 213678 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/3716137
EFL EWS Bot
Comment 10 2013-10-08 01:57:46 PDT
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
Build Bot
Comment 11 2013-10-08 02:26:05 PDT
Comment on attachment 213678 [details] Patch Attachment 213678 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/3709149
Andreas Kling
Comment 12 2013-10-10 01:44:51 PDT
Created attachment 213858 [details] Patch
Andreas Kling
Comment 13 2013-10-10 02:25:38 PDT
Created attachment 213861 [details] Patch
EFL EWS Bot
Comment 14 2013-10-10 02:58:31 PDT
Comment on attachment 213861 [details] Patch Attachment 213861 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/3783025
Build Bot
Comment 15 2013-10-10 03:10:42 PDT
Comment on attachment 213861 [details] Patch Attachment 213861 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/3400054
Andreas Kling
Comment 16 2013-10-10 03:41:23 PDT
Created attachment 213864 [details] Patch
WebKit Commit Bot
Comment 17 2013-10-10 05:00:24 PDT
Comment on attachment 213864 [details] Patch Clearing flags on attachment: 213864 Committed r157215: <http://trac.webkit.org/changeset/157215>
WebKit Commit Bot
Comment 18 2013-10-10 05:00:29 PDT
All reviewed patches have been landed. Closing bug.
Darin Adler
Comment 19 2013-10-10 09:20:36 PDT
We should change the name of this function from impl to something made out of words. Maybe wrappedObject? Maybe something good but still shorter?
Andreas Kling
Comment 20 2013-10-10 09:22:15 PDT
(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?
Note You need to log in before you can comment on or make changes to this bug.