Bug 122497

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:

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

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-	Details Formatted Diff Diff
Patch (392.97 KB, patch) 2013-10-08 01:27 PDT, Andreas Kling	eflews.bot: commit-queue-	Details Formatted Diff Diff
Patch (397.49 KB, patch) 2013-10-08 01:46 PDT, Andreas Kling	eflews.bot: commit-queue-	Details Formatted Diff Diff
Patch (377.50 KB, patch) 2013-10-10 01:44 PDT, Andreas Kling	no flags	Details Formatted Diff Diff
Patch (376.89 KB, patch) 2013-10-10 02:25 PDT, Andreas Kling	eflews.bot: commit-queue-	Details Formatted Diff Diff
Patch (380.19 KB, patch) 2013-10-10 03:41 PDT, Andreas Kling	no flags	Details Formatted Diff Diff
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.