Bug 69898 - Layout tests asserting in LayoutTestController::pathToLocalResource()
Summary: Layout tests asserting in LayoutTestController::pathToLocalResource()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Seidel (no email)
URL:
Keywords: LayoutTestFailure, MakingBotsRed
: 69899 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-10-11 21:56 PDT by Simon Fraser (smfr)
Modified: 2011-10-12 17:14 PDT (History)
4 users (show)

See Also:


Attachments
Patch (1.45 KB, patch)
2011-10-12 16:04 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2011-10-11 21:56:59 PDT
Various layout tests are asserting in LayoutTestController::pathToLocalResource() on the SL leaks bot:

http://build.webkit.org/results/SnowLeopard%20Intel%20Leaks/r97218%20(19479)/results.html

fast/loader/local-CSS-from-local.html	stderr	crash log (DumpRenderTree: LayoutTestController::pathToLocalResource(OpaqueJSContext const*, OpaqueJSString*) + 601)
fast/loader/local-JavaScript-from-local.html	stderr	crash log (DumpRenderTree: LayoutTestController::pathToLocalResource(OpaqueJSContext const*, OpaqueJSString*) + 601)
fast/loader/local-iFrame-source-from-local.html	stderr	crash log (DumpRenderTree: LayoutTestController::pathToLocalResource(OpaqueJSContext const*, OpaqueJSString*) + 601)
fast/loader/local-image-from-local.html	stderr	crash log (DumpRenderTree: LayoutTestController::pathToLocalResource(OpaqueJSContext const*, OpaqueJSString*) + 601)

Thread 0 Crashed:  Dispatch queue: com.apple.main-thread
0   DumpRenderTree                	0x0000000100030c91 LayoutTestController::pathToLocalResource(OpaqueJSContext const*, OpaqueJSString*) + 601 (LayoutTestControllerMac.mm:416)
1   DumpRenderTree                	0x00000001000260be pathToLocalResourceCallback(OpaqueJSContext const*, OpaqueJSValue*, OpaqueJSValue*, unsigned long, OpaqueJSValue const* const*, OpaqueJSValue const**) + 199 (LayoutTestController.cpp:288)
2   com.apple.JavaScriptCore      	0x00000001002c1ef9 JSC::JSCallbackFunction::call(JSC::ExecState*) + 301 (JSCallbackFunction.cpp:73)
3   com.apple.JavaScriptCore      	0x00000001002aa570 cti_op_call_NotJSFunction + 425 (JITStubs.cpp:2323)
4   com.apple.JavaScriptCore      	0x00000001002a2d2d jscGeneratedNativeCode + 0 (JITStubs.cpp:946)
5   com.apple.JavaScriptCore      	0x00000001002819ea JSC::JITCode::execute(JSC::RegisterFile*, JSC::ExecState*, JSC::JSGlobalData*) + 86 (JITCode.h:103)
6   com.apple.JavaScriptCore      	0x000000010027bd1b JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 1359
Comment 1 Ryosuke Niwa 2011-10-11 22:00:43 PDT
There is clearly a bug here:

http://trac.webkit.org/changeset/95588/trunk/Tools/DumpRenderTree/mac/LayoutTestControllerMac.mm
414	    // Note: It's important that we keep the file:// or http tests will get confused. 
415	    if (localResourceString.find("file://") != std::string::npos) { 
416	        ASSERT(absolutePathToLocalResource[0] == '/'); 

It should read:

415	    if (localResourceString.find("file://") == std::string::npos) { 
416	        ASSERT(absolutePathToLocalResource[0] == '/');
Comment 2 Eric Seidel (no email) 2011-10-11 22:08:40 PDT
Did this just start?  This change was landed long ago, no?
Comment 3 Ryosuke Niwa 2011-10-11 22:22:50 PDT
(In reply to comment #1)
> There is clearly a bug here:
> 
> http://trac.webkit.org/changeset/95588/trunk/Tools/DumpRenderTree/mac/LayoutTestControllerMac.mm
> 414        // Note: It's important that we keep the file:// or http tests will get confused. 
> 415        if (localResourceString.find("file://") != std::string::npos) { 
> 416            ASSERT(absolutePathToLocalResource[0] == '/'); 
> 
> It should read:
> 
> 415        if (localResourceString.find("file://") == std::string::npos) { 
> 416            ASSERT(absolutePathToLocalResource[0] == '/');

Oh wait, this is not it. I misread the code.
Comment 4 Ryosuke Niwa 2011-10-11 22:40:15 PDT
(In reply to comment #2)
> Did this just start?  This change was landed long ago, no?

I think this has always failed. It appears that it's coming from webkit port difference. I can reproduce the crash if I run:
 Tools/Scripts/run-webkit-tests --debug --leaks fast/loader/local-JavaScript-from-local.html

but can't reproduce the crash if I run:
 Tools/Scripts/run-webkit-tests --debug fast/loader/local-JavaScript-from-local.html
Comment 5 Eric Seidel (no email) 2011-10-12 15:56:50 PDT
I am able to reproduce the failure.  Not yet sure what's causing it. :)
Comment 6 Eric Seidel (no email) 2011-10-12 15:57:54 PDT
Ah.  Looks like --leaks is using ORWT.  I thought it was using NRWT.  I'll make the change to ORWT.
Comment 7 Ryosuke Niwa 2011-10-12 15:58:49 PDT
(In reply to comment #6)
> Ah.  Looks like --leaks is using ORWT.  I thought it was using NRWT.  I'll make the change to ORWT.

No wonder!
Comment 8 Eric Seidel (no email) 2011-10-12 16:01:44 PDT
*** Bug 69899 has been marked as a duplicate of this bug. ***
Comment 9 Eric Seidel (no email) 2011-10-12 16:04:35 PDT
Created attachment 110762 [details]
Patch
Comment 10 WebKit Review Bot 2011-10-12 17:14:00 PDT
Comment on attachment 110762 [details]
Patch

Clearing flags on attachment: 110762

Committed r97321: <http://trac.webkit.org/changeset/97321>
Comment 11 WebKit Review Bot 2011-10-12 17:14:05 PDT
All reviewed patches have been landed.  Closing bug.