Bug 45479 – Implement layoutTestController.dumpResourceResponseMIMETypes in Chromium DRT

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Mihai Parparita]

Implement layoutTestController.dumpResourceResponseMIMETypes in Chromium DRT

[Mihai Parparita]

Created attachment 67065 [details]
Patch

[Mihai Parparita]

Tony, can you review this?

test_shell changes are in http://codereview.chromium.org/3317016. As long as those land before this, we should be OK (tests will pass unexpectedly till a WebKit roll picks this up).

[Tony Chang]

Comment on attachment 67065 [details]
Patch

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

> WebKitTools/DumpRenderTree/chromium/LayoutTestController.h:108
> +    // This function sets a flag that tells the test_shell to dump the MIME type
> +    // for each resource that was loaded. It takes no arguments, and ignores any
> +    // that may be present.
> +    void dumpResourceResponseMIMETypes(const CppArgumentList&, CppVariant* result);
Nit: remove |result|

> WebKitTools/DumpRenderTree/chromium/WebViewHost.cpp:922
> +        fputs(" - didReceiveResponse ", stdout);
> +        printResponseDescription(response);
> +        fputs("\n", stdout);
Nit: Can we use puts instead of fputs?

> WebKitTools/DumpRenderTree/chromium/WebViewHost.cpp:930
> +            // Simulate NSURLResponse's mapping of empty/unknown MIME types to application/octet-stream
> +            mimeType.isEmpty() ? "application/octet-stream" : mimeType.utf8().data());
Same question about mimeType.utf().data(), do we need to null terminate it?

[Mihai Parparita]

Created attachment 67081 [details]
Patch

[Mihai Parparita]

(In reply to comment #3)
> Nit: remove |result|

Done.

> Nit: Can we use puts instead of fputs?

Sure, switched the whole file over.
 
> Same question about mimeType.utf().data(), do we need to null terminate it?

CString::init null-terminates, so it should be OK.

[Tony Chang]

Comment on attachment 67081 [details]
Patch

Clearing flags on attachment: 67081

Committed r67119: <http://trac.webkit.org/changeset/67119>

[Tony Chang]

All reviewed patches have been landed.  Closing bug.

[Tony Chang]

Rolled out.  I'm dumb.  puts adds a newline and fputs doesn't, so we can't just replace them.

[Mihai Parparita]

*** Bug 45506 has been marked as a duplicate of this bug. ***

[Mihai Parparita]

Created attachment 67132 [details]
Patch

[Mihai Parparita]

New version of the patch leaves the fputs calls alone and adds a Linux expectation for fast/preloader/script.html, since we get a different MIME type for script files in file:/// URLs there.

[Kent Tamura]

Comment on attachment 67132 [details]
Patch

Looks ok.

[WebKit Commit Bot]

Comment on attachment 67132 [details]
Patch

Rejecting patch 67132 from commit-queue.

Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Kent Tamura', u'--force']" exit_code: 1
Last 500 characters of output:
nk #2 succeeded at 292 (offset 6 lines).
Hunk #3 succeeded at 493 (offset 6 lines).
patching file WebKitTools/DumpRenderTree/chromium/LayoutTestController.h
Hunk #1 succeeded at 100 (offset 4 lines).
Hunk #2 succeeded at 240 (offset 4 lines).
Hunk #3 succeeded at 337 (offset 5 lines).
Hunk #4 succeeded at 458 (offset 5 lines).
patching file WebKitTools/DumpRenderTree/chromium/TestShell.h
patching file WebKitTools/DumpRenderTree/chromium/WebViewHost.cpp
Hunk #1 succeeded at 920 (offset 5 lines).

Full output: http://queues.webkit.org/results/3941446

[Mihai Parparita]

Created attachment 67272 [details]
Patch

[Mihai Parparita]

Hopefully the latest version of the patch will merge cleanly.

[WebKit Commit Bot]

Comment on attachment 67272 [details]
Patch

Clearing flags on attachment: 67272

Committed r67294: <http://trac.webkit.org/changeset/67294>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.