Bug 72950 - [DRT Chromium] Set right default value to baseURL in LayoutTestController::queueLoadHTMLString
Summary: [DRT Chromium] Set right default value to baseURL in LayoutTestController::qu...
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: Johnny(Jianning) Ding
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-22 06:28 PST by Johnny(Jianning) Ding
Modified: 2011-11-23 06:12 PST (History)
5 users (show)

See Also:


Attachments
patch v1 (2.84 KB, patch)
2011-11-22 06:33 PST, Johnny(Jianning) Ding
tony: review+
Details | Formatted Diff | Diff
patch v1 with addressing tony's nit. (2.75 KB, patch)
2011-11-22 19:17 PST, Johnny(Jianning) Ding
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Johnny(Jianning) Ding 2011-11-22 06:28:02 PST
fast/loader/non-deferred-substitute-load.html was crashed on the debug version of DRT Chromium port. The crash was caused by ASSERT(!newRequest.isNull()) in MainResourceLoader::willSendRequest.

After investigation, test non-deferred-substitute-load.html calls LayoutTestController::queueLoadHTMLString to load some HTML contents, however DRT Chromium port sets a empty URL as default base URL in LayoutTestController::queueLoadHTMLString, which causes the assert.
By looking at the general DRT implementation, (see http://trac.webkit.org/browser/trunk/Tools/DumpRenderTree/LayoutTestController.cpp#L1016.) it sets a "" string as default value to baseURL to avoid the assert, DRT Chromium port should follow it.

A patch is on the way
Comment 1 Johnny(Jianning) Ding 2011-11-22 06:33:47 PST
Created attachment 116216 [details]
patch v1
Comment 2 Tony Chang 2011-11-22 09:35:27 PST
Comment on attachment 116216 [details]
patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=116216&action=review

> Tools/DumpRenderTree/chromium/LayoutTestController.cpp:575
> +        // FIXME: Should use the URL of current document as default if it is specified?

Nit: I would remove this comment since it seems like this is intentional.
Comment 3 Johnny(Jianning) Ding 2011-11-22 19:17:11 PST
Created attachment 116307 [details]
patch v1 with addressing tony's nit.
Comment 4 WebKit Review Bot 2011-11-23 06:12:29 PST
Comment on attachment 116307 [details]
patch v1 with addressing tony's nit.

Clearing flags on attachment: 116307

Committed r101076: <http://trac.webkit.org/changeset/101076>
Comment 5 WebKit Review Bot 2011-11-23 06:12:33 PST
All reviewed patches have been landed.  Closing bug.