Bug 131954

Summary: [iOS WK2] Add test URL to crash reports for the UI process, clean up project
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: New BugsAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, ddkilzer, dfarler, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch darin: review+

Description Simon Fraser (smfr) 2014-04-21 14:45:15 PDT
[iOS WK2] Add test URL to crash reports for the UI process, clean up project
Comment 1 Simon Fraser (smfr) 2014-04-21 14:47:56 PDT
Created attachment 229836 [details]
Patch
Comment 2 Darin Adler 2014-04-21 18:42:03 PDT
Comment on attachment 229836 [details]
Patch

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

> Tools/WebKitTestRunner/cocoa/CrashReporterInfo.mm:26
> +#import "config.h"

Where’s the include of CrashReporterInfo.h?

> Tools/WebKitTestRunner/cocoa/CrashReporterInfo.mm:33
> +#import <wtf/text/WTFString.h>

I don’t think you need this include if you’re including StringBuilder.h.

> Tools/WebKitTestRunner/cocoa/CrashReporterInfo.mm:37
> +using namespace WTF;

Should not need this.

> Tools/WebKitTestRunner/cocoa/CrashReporterInfo.mm:48
> +    String schemeString(schemeCFString.get());

Seems a shame to convert this CFStringRef to a WTF::String just to call equalIgnoringCase. Doesn’t CF have functions that can do that?

> Tools/WebKitTestRunner/cocoa/CrashReporterInfo.mm:52
> +        String layoutTests("/LayoutTests/");

Doesn’t seem like this should be a WTF::String. Should use a const char[] instead.

> Tools/WebKitTestRunner/cocoa/CrashReporterInfo.mm:64
> +    String hostString(hostCFString.get());

Doesn’t seem necessary to convert this to a WTF::String just to compare it with "127.0.0.1".

> Tools/WebKitTestRunner/cocoa/CrashReporterInfo.mm:74
> +    if (!testPath.isNull()) {

I think early return is better.

> Tools/WebKitTestRunner/cocoa/CrashReporterInfo.mm:78
> +        StringBuilder builder;
> +        builder.appendLiteral("CRASHING TEST: ");
> +        builder.append(testPath);
> +        WKSetCrashReportApplicationSpecificInformation(builder.toString().createCFString().get());

No need for StringBuilder just to concatenate. Can write:

    WKSetCrashReportApplicationSpecificInformation(("CRASHING TEST: " + testPath).createCFString().get());
Comment 3 Csaba Osztrogonác 2014-04-22 07:00:32 PDT
*** Bug 131953 has been marked as a duplicate of this bug. ***
Comment 4 Simon Fraser (smfr) 2014-04-30 14:57:25 PDT
https://trac.webkit.org/r168045