Bug 122497

Summary: JS DOM wrappers' impl() functions should return references.
Product: WebKit Reporter: Andreas Kling <kling>
Component: WebCore JavaScriptAssignee: 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 Flags
Patch
eflews.bot: commit-queue-
Patch
eflews.bot: commit-queue-
Patch
eflews.bot: commit-queue-
Patch
none
Patch
eflews.bot: commit-queue-
Patch none

Description Andreas Kling 2013-10-08 00:56:58 PDT
JS DOM wrappers always have a corresponding WebCore object, so impl() should really return a reference.
Comment 1 Andreas Kling 2013-10-08 01:06:21 PDT
Created attachment 213674 [details]
Patch
Comment 2 WebKit Commit Bot 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.
Comment 3 EFL EWS Bot 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
Comment 4 Andreas Kling 2013-10-08 01:27:12 PDT
Created attachment 213676 [details]
Patch
Comment 5 EFL EWS Bot 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
Comment 6 kov's GTK+ EWS bot 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
Comment 7 Andreas Kling 2013-10-08 01:46:15 PDT
Created attachment 213678 [details]
Patch
Comment 8 EFL EWS Bot 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
Comment 9 kov's GTK+ EWS bot 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
Comment 10 EFL EWS Bot 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
Comment 11 Build Bot 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
Comment 12 Andreas Kling 2013-10-10 01:44:51 PDT
Created attachment 213858 [details]
Patch
Comment 13 Andreas Kling 2013-10-10 02:25:38 PDT
Created attachment 213861 [details]
Patch
Comment 14 EFL EWS Bot 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
Comment 15 Build Bot 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
Comment 16 Andreas Kling 2013-10-10 03:41:23 PDT
Created attachment 213864 [details]
Patch
Comment 17 WebKit Commit Bot 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>
Comment 18 WebKit Commit Bot 2013-10-10 05:00:29 PDT
All reviewed patches have been landed.  Closing bug.
Comment 19 Darin Adler 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?
Comment 20 Andreas Kling 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?