Restricting the error type is not efficient as some promises may reject with exception codes (int) or some dedicated errors.
Created attachment 278488 [details] Patch
(In reply to comment #1) > Created attachment 278488 [details] > Patch Patch is removing the DOMPromise rejection type restriction. It also tries to remove some limitations of the resolve/reject methods. Currently these methods take const references as parameter. For DOM objects, it may be better to pass either plain references or Ref/RefPtr rvalues.
Comment on attachment 278488 [details] Patch Rejecting attachment 278488 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: Volumes/Data/EWS/WebKit/Source/WebCore/loader/archive/ArchiveFactory.cpp -o /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/ArchiveFactory.o ** BUILD FAILED ** The following build commands failed: CompileC /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/AccessibilityMediaControls.o accessibility/AccessibilityMediaControls.cpp normal x86_64 c++ com.apple.compilers.llvm.clang.1_0.compiler (1 failure) Full output: http://webkit-queues.webkit.org/results/1302453
Created attachment 278719 [details] Patch for landing
Comment on attachment 278719 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=278719&action=review > Source/WebCore/bindings/js/JSDOMBinding.h:544 > +template<typename T> inline JSC::JSValue toJS(JSC::ExecState* exec, JSDOMGlobalObject* globalObject, RefPtr<T>& ptr) I hesitated on adding this one. Ref<T> defines its cast to T&, but RefPtr<T> is not doing that for T*. Is there a reason for that?
Comment on attachment 278719 [details] Patch for landing Clearing flags on attachment: 278719 Committed r200766: <http://trac.webkit.org/changeset/200766>
All reviewed patches have been landed. Closing bug.
Re-opened since this is blocked by bug 157623
Created attachment 278824 [details] Patch for landing - updating Ref/RefPtr detection
Created attachment 278828 [details] Patch for landing
Comment on attachment 278828 [details] Patch for landing Rejecting attachment 278828 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 278828, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in Source/WTF/ChangeLog contains OOPS!. Full output: http://webkit-queues.webkit.org/results/1314216
Created attachment 278829 [details] Patch for landing
Comment on attachment 278829 [details] Patch for landing Clearing flags on attachment: 278829 Committed r200837: <http://trac.webkit.org/changeset/200837>
Last patch updated the way to detect Ref and RefPtr. To make Yosemite build happy, I added Ref::isRef and RefPtr::isRefPtr constexpr. The test is split in two tests (one for Ref and one for RefPtr) to make win bot happy. I am not sure why Ref has a cast to T&, but RefPtr does not have for T*.
(In reply to comment #15) > I am not sure why Ref has a cast to T&, but RefPtr does not have for T*. Both are dangerous. Silently converting a smart pointer or reference to a raw one can be dangerous because the lifetime of the object is not extended and can lead to use after free. This primary comes into play when assigning to a local variable. Both are convenient. Silently converting without a call to get() or ptr() can be much more elegant when passing an argument to a function, and in that case the lifetime works out just fine.
I am still working on confirming but the bots showed a 3% regression on Speedometer / iOS when this patch landed.
Comment on attachment 278829 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=278829&action=review > Source/WebCore/bindings/js/JSDOMBinding.h:-255 > -template<typename T> JSC::JSValue toJS(JSC::ExecState*, JSDOMGlobalObject*, const Ref<T>&); To more overloads for Ref<>? Isn't it so now that if someone passes a Ref<>&& we're going to use the RefPtr<>&& overload and therefore do null checks? This may have disabled my optimization from yesterday, which would explain why the benchmarks are back to their old scores.
Reverted r200837 for reason: Seems to have regressed Speedometer and JetStream on iOS Committed r200875: <http://trac.webkit.org/changeset/200875>
(In reply to comment #17) > I am still working on confirming but the bots showed a 3% regression on > Speedometer / iOS when this patch landed. Is it possible to have access to monitor these results? > > Source/WebCore/bindings/js/JSDOMBinding.h:-255 > > -template<typename T> JSC::JSValue toJS(JSC::ExecState*, JSDOMGlobalObject*, const Ref<T>&); > > To more overloads for Ref<>? Isn't it so now that if someone passes a > Ref<>&& we're going to use the RefPtr<>&& overload and therefore do null > checks? > This may have disabled my optimization from yesterday, which would explain > why the benchmarks are back to their old scores. I guess we could add an additional toJS(... Ref<T>&&).
(In reply to comment #20) > (In reply to comment #17) > > I am still working on confirming but the bots showed a 3% regression on > > Speedometer / iOS when this patch landed. > > Is it possible to have access to monitor these results? Not at the moment unfortunately. I am monitoring to make sure the scores go back up after the roll out.
(In reply to comment #19) > Reverted r200837 for reason: > > Seems to have regressed Speedometer and JetStream on iOS > > Committed r200875: <http://trac.webkit.org/changeset/200875> FYI, this patch was also a 2% regression on Kraken / iOS even though my original toJS() change did not progress Kraken at all. So it seems this did more than just disabling my optimization.
(In reply to comment #22) > (In reply to comment #19) > > Reverted r200837 for reason: > > > > Seems to have regressed Speedometer and JetStream on iOS > > > > Committed r200875: <http://trac.webkit.org/changeset/200875> > > FYI, this patch was also a 2% regression on Kraken / iOS even though my > original toJS() change did not progress Kraken at all. So it seems this did > more than just disabling my optimization. That seems like a lot to me. I was thinking Kraken was solely targetting JSC. I am surprised toJS() changes have such a big effect on Kraken results.
Created attachment 279107 [details] Patch for landing
(In reply to comment #24) > Created attachment 279107 [details] > Patch for landing I removed the toJS changes.
Comment on attachment 279107 [details] Patch for landing Clearing flags on attachment: 279107 Committed r201013: <http://trac.webkit.org/changeset/201013>