With toV8Sequence() (r122555) now we can have generalised validation of the passed object for sequence type per WebIDL spec. We can remove the specialised check for MessagePort from V8Utilities::extractTransferables() using toV8Sequence()
Created attachment 152208 [details] Patch
Comment on attachment 152208 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152208&action=review > LayoutTests/ChangeLog:11 > + * platform/chromium-win/fast/workers/worker-context-multi-port-expected.txt: Removed. Currently this and below tests are in TextExpections of chromium as BUGWK74459 SKIP : fast/workers/worker-context-multi-port.html = PASS
Comment on attachment 152208 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152208&action=review > Source/WebCore/bindings/v8/V8Utilities.cpp:-105 > - if (value->IsArray()) { > - v8::Local<v8::Array> array = v8::Local<v8::Array>::Cast(value); > - length = array->Length(); > - } else { Is it OK to remove this part? I mean, the code might be: if (value->IsArray()) { v8::Local<v8::Array> array = v8::Local<v8::Array>::Cast(value); length = array->Length(); } else { if (toV8Sequence(value, length).IsEmpty()) return false; } > LayoutTests/ChangeLog:15 > + * platform/chromium-win/fast/workers/worker-context-multi-port-expected.txt: Removed. > + * platform/chromium-win/fast/workers/worker-multi-port-expected.txt: Removed. > + * platform/chromium/fast/dom/Window/window-postmessage-args-expected.txt: Removed. > + * platform/chromium/fast/events/constructors/message-event-constructor-expected.txt: Removed. > + * platform/chromium/fast/events/message-port-multi-expected.txt: Rebaselined. Great!
Comment on attachment 152208 [details] Patch Attachment 152208 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13230186 New failing tests: http/tests/w3c/webperf/approved/navigation-timing/html5/test_performance_attributes_exist_in_object.html
Created attachment 152228 [details] Archive of layout-test-results from gce-cr-linux-04 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-04 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Created attachment 152247 [details] Updated Patch (In reply to comment #3) > (From update of attachment 152208 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=152208&action=review > > > Source/WebCore/bindings/v8/V8Utilities.cpp:-105 > > - if (value->IsArray()) { > > - v8::Local<v8::Array> array = v8::Local<v8::Array>::Cast(value); > > - length = array->Length(); > > - } else { > > Is it OK to remove this part? I mean, the code might be: > > if (value->IsArray()) { > v8::Local<v8::Array> array = v8::Local<v8::Array>::Cast(value); > length = array->Length(); > } else { > if (toV8Sequence(value, length).IsEmpty()) > return false; > } Though removing IsArray() check doesn't cause any behavioural change but having it will surely faster than every time calling toV8Sequence(). Thanks!! > New failing tests: > http/tests/w3c/webperf/approved/navigation-timing/html5/test_performance_attributes_exist_in_object.html Regarding failing perf test though I have rebased test expected but I really not sure if that is expected?
Comment on attachment 152247 [details] Updated Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152247&action=review > LayoutTests/ChangeLog:11 > + * http/tests/w3c/webperf/approved/navigation-timing/html5/test_performance_attributes_exist_in_object-expected.txt: Rebased failing perf-test This is not related to this patch. Let's keep it as is and ping committers who caused the regression.
(In reply to comment #7) > This is not related to this patch. Let's keep it as is and ping committers who caused the regression. Just in case: "keep it as is" means please do not include the change in this patch.
Created attachment 152321 [details] updated_patch Thanks haraken for review. > > LayoutTests/ChangeLog:11 > > + * http/tests/w3c/webperf/approved/navigation-timing/html5/test_performance_attributes_exist_in_object-expected.txt: Rebased failing perf-test > This is not related to this patch. Let's keep it as is and ping committers who caused the regression. I think this regression of r122528.
Comment on attachment 152321 [details] updated_patch OK!
Comment on attachment 152321 [details] updated_patch Clearing flags on attachment: 152321 Committed r122628: <http://trac.webkit.org/changeset/122628>
All reviewed patches have been landed. Closing bug.