Improve IDL support for object arguments that are neither optional nor nullable
Created attachment 275511 [details] Patch
Attachment 275511 [details] did not pass style-queue: ERROR: Source/WebCore/ChangeLog:59: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/ChangeLog:60: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/ChangeLog:61: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/ChangeLog:62: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/ChangeLog:63: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/ChangeLog:64: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/ChangeLog:65: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/ChangeLog:66: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/ChangeLog:67: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/ChangeLog:68: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/ChangeLog:69: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/ChangeLog:70: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/ChangeLog:71: Line contains tab character. [whitespace/tab] [5] Total errors found: 13 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 275512 [details] Patch
Comment on attachment 275512 [details] Patch Attachment 275512 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1094547 New failing tests: fast/canvas/canvas-path-addPath.html
Created attachment 275513 [details] Archive of layout-test-results from ews102 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 275512 [details] Patch Attachment 275512 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/1094550 New failing tests: fast/canvas/canvas-path-addPath.html
Created attachment 275514 [details] Archive of layout-test-results from ews104 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 275512 [details] Patch Attachment 275512 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1094557 New failing tests: fast/canvas/canvas-path-addPath.html
Created attachment 275515 [details] Archive of layout-test-results from ews125 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 275512 [details] Patch Attachment 275512 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1094559 New failing tests: fast/canvas/canvas-path-addPath.html
Created attachment 275516 [details] Archive of layout-test-results from ews114 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 275982 [details] Patch
OK, looks like this newer patch now is passing on EWS. Review time! (Let me know if you think of any other potential reviewers I should cc.)
Comment on attachment 275982 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=275982&action=review r=me > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:3650 > + if ($isTearOff) { May be nicer as a ternary: $value = $isTearOff ? "$name->propertyReference()" : "*$name"; > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:3665 > + push @arguments, $value; Not a big deal but I think we usually use the version with parentheses: push(@arguments, $value); > Source/WebCore/bindings/scripts/test/JS/JSTestMediaQueryListListener.cpp:163 > RefPtr<MediaQueryListListener> listener = JSMediaQueryListListener::create(asObject(state->uncheckedArgument(0)), castedThis->globalObject()); This should probably use a Ref<> > Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:510 > RefPtr<TestCallbackFunction> testCallbackFunction = JSTestCallbackFunction::create(asObject(state->uncheckedArgument(1)), castedThis->globalObject()); If we used a Ref<> here, we would not need to add the stars below.
Comment on attachment 275982 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=275982&action=review >> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:3650 >> + if ($isTearOff) { > > May be nicer as a ternary: > $value = $isTearOff ? "$name->propertyReference()" : "*$name"; OK. >> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:3665 >> + push @arguments, $value; > > Not a big deal but I think we usually use the version with parentheses: > push(@arguments, $value); I see that now. In my own perl programming I don’t do that, but I’ll follow what most people do here. You’ll notice that the code I replaced was using push like this. Will change it. >> Source/WebCore/bindings/scripts/test/JS/JSTestMediaQueryListListener.cpp:163 >> RefPtr<MediaQueryListListener> listener = JSMediaQueryListListener::create(asObject(state->uncheckedArgument(0)), castedThis->globalObject()); > > This should probably use a Ref<> I think it should use auto. I’ll try to change this, but I’ll only do it if it’s easy. >> Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:510 >> RefPtr<TestCallbackFunction> testCallbackFunction = JSTestCallbackFunction::create(asObject(state->uncheckedArgument(1)), castedThis->globalObject()); > > If we used a Ref<> here, we would not need to add the stars below. I think it should use auto. I’ll try to change this, but I’ll only do it if it’s easy. I suspect it won’t be easy.
Committed r199265: <http://trac.webkit.org/changeset/199265>