fast/files/blob-constructor.html = CRASH fast/dom/HTMLAnchorElement/anchor-download-unset.html = TIMEOUT fast/filesystem/file-writer-gc-blob.html = TIMEOUT fast/filesystem/workers/file-writer-gc-blob.html = TEXT fast/filesystem/workers/file-writer-sync-truncate-extend.html http/tests/security/filesystem-iframe-from-remote.html = TEXT storage/indexeddb/noblobs.html = TEXT
http://trac.webkit.org/changeset/115582 https://bugs.webkit.org/show_bug.cgi?id=84555
Why not CC the authors of that change?
FYI, alecflett@ updated the storage/indexeddb/noblobs.html test in r116337 to use the new Blob() constructor, but didn't remove the expectations line. I'll do so now.
The root cause of fast/files/blob-constructor.html = CRASH is http://trac.webkit.org/browser/trunk/Source/WebCore/bindings/v8/custom/V8BlobCustom.cpp#L129 EXCEPTION_BLOCK(String, stringValue, toWebCoreString(item->ToString())); should be EXCEPTION_BLOCK(String, stringValue, toWebCoreString(item));
In the fast/files/script-tests/blob-constructor.js var throwingObj = { toString: function() { throw "Error"; } }; shouldThrow("new Blob([throwingObj])", "'Error'"); results in chromium DumpRenderTree crash.
Fixing the blob-constructor.html crash is only the first step to remove it from chromium test_expectations.txt. There are some gaps with JS binding. 1. shouldThrow("new Blob([], {endings:throwingObj})", "'Error'"); 2. shouldThrow("new Blob([], {endings:throwingObj1, type:throwingObj2})", "'Error 1'"); 3. shouldThrow("new Blob([], {type:throwingObj2, endings:throwingObj1})", "'Error 1'"); 4. shouldBe("window.Blob.length", "2"); It seems that Dictionary willn't throw exception. Both of tryCatchEndings.HasCaught() and tryCatchType.HasCaught() return 0.
> Fixing the blob-constructor.html crash is only the first step to remove it from chromium test_expectations.txt. > There are some gaps with JS binding. > 1. shouldThrow("new Blob([], {endings:throwingObj})", "'Error'"); > 2. shouldThrow("new Blob([], {endings:throwingObj1, type:throwingObj2})", "'Error 1'"); > 3. shouldThrow("new Blob([], {type:throwingObj2, endings:throwingObj1})", "'Error 1'"); > 4. shouldBe("window.Blob.length", "2"); > > It seems that Dictionary willn't throw exception. > Both of tryCatchEndings.HasCaught() and tryCatchType.HasCaught() return 0. Thanks for digging this, I'd like to take a look at the binding part...
Created attachment 145548 [details] Patch
Comment on attachment 145548 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=145548&action=review > Source/WebCore/bindings/v8/custom/V8MessageEventCustom.cpp:119 > - String originArg = v8ValueToWebCoreString(args[4]); > - String lastEventIdArg = v8ValueToWebCoreString(args[5]); > + EXCEPTION_BLOCK(String, originArg, v8ValueToWebCoreString(args[4])); > + EXCEPTION_BLOCK(String, lastEventIdArg, v8ValueToWebCoreString(args[5])); It seems that this is not the only place where we need to fix. For example, toWebCoreStringWithNullCheck() is used in V8DOMformDataCustom.cpp without being surrounded by EXCEPTION_BLOCK. Anyone: BTW, isn't there any way to nest TryCatches? As far as I see v8.h and api.cc, it seems that we can register only one TryCatch handler per Isolate, which means that TryCatches are not nest-able. But in that case what is TryCatch::ReThrow() for?
Ok it looks at least EWS test passes with this approach. (In reply to comment #9) > > Source/WebCore/bindings/v8/custom/V8MessageEventCustom.cpp:119 > > - String originArg = v8ValueToWebCoreString(args[4]); > > - String lastEventIdArg = v8ValueToWebCoreString(args[5]); > > + EXCEPTION_BLOCK(String, originArg, v8ValueToWebCoreString(args[4])); > > + EXCEPTION_BLOCK(String, lastEventIdArg, v8ValueToWebCoreString(args[5])); > > It seems that this is not the only place where we need to fix. For example, toWebCoreStringWithNullCheck() is used in V8DOMformDataCustom.cpp without being surrounded by EXCEPTION_BLOCK. Thanks, I'll wait a bit more to see if anyone has better idea about nested TryCatch handling before converting all those callsites. > Anyone: BTW, isn't there any way to nest TryCatches? As far as I see v8.h and api.cc, it seems that we can register only one TryCatch handler per Isolate, which means that TryCatches are not nest-able. But in that case what is TryCatch::ReThrow() for? As he wrote v8::TryCatch doesn't look nestable to me, but I want to make sure if this assumption is correct and if it's intended behavior. Does anyone know?
Created attachment 145702 [details] Crash Fix Fixing CRASH bug first h. (Thanks to Li Yin)
(In reply to comment #11) > Created an attachment (id=145702) [details] > Crash Fix > > Fixing CRASH bug first. (Thanks to Li Yin) The new patch only fixes the CRASH part to start with. Can someone take a look at it? (I'll continue fixing the Text mismatches, but the crash one looks worst)
Comment on attachment 145702 [details] Crash Fix View in context: https://bugs.webkit.org/attachment.cgi?id=145702&action=review OK, let's land this patch first, and then investigate the nested TryCatch issues. > Source/WebCore/ChangeLog:8 > + Fixes CRASH error for string arguments. (Still has TEXT mismatch problems) Would you elaborate the explanation?
(In reply to comment #13) > (From update of attachment 145702 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=145702&action=review > > OK, let's land this patch first, and then investigate the nested TryCatch issues. > > > Source/WebCore/ChangeLog:8 > > + Fixes CRASH error for string arguments. (Still has TEXT mismatch problems) > > Would you elaborate the explanation? Thanks. Will be adding description like below: + This fixes a crash problem which could happen when the constructor is + given an array which contains String-type item(s). + There're still some Text mismatches between v8 results and + JSC results, most of them are v8 not throwing exception + when it is given an object whose toString() method throws + exception. (The issue will be addresse in a separate patch.)
(In reply to comment #14) > Thanks. Will be adding description like below: > > + This fixes a crash problem which could happen when the constructor is > + given an array which contains String-type item(s). > + There're still some Text mismatches between v8 results and > + JSC results, most of them are v8 not throwing exception > + when it is given an object whose toString() method throws > + exception. (The issue will be addresse in a separate patch.) Sounds good!
Committed r119479: <http://trac.webkit.org/changeset/119479>
(More patches to come)
v8::TryCatch issue: http://code.google.com/p/v8/issues/detail?id=2166
(In reply to comment #18) > v8::TryCatch issue: http://code.google.com/p/v8/issues/detail?id=2166 It is now fixed!
(In reply to comment #19) > (In reply to comment #18) > > v8::TryCatch issue: http://code.google.com/p/v8/issues/detail?id=2166 > > It is now fixed! Unfortunately it is now reverted. I discussed the TryCatch bug with the V8 team, and it will take more time to fix it. (The original fix worked fine if all the nested TryCatch objects exist on V8 stack, but didn't work if any TryCatch object exists on heap.)
Marked LayoutTest bugs, bugs with Chromium IDs, and some others as WontFix. Test failure bugs still are trackable via TestExpectations or disabled unit tests.