Bug 131954 - [iOS WK2] Add test URL to crash reports for the UI process, clean up project
Summary: [iOS WK2] Add test URL to crash reports for the UI process, clean up project
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords:
: 131953 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-04-21 14:45 PDT by Simon Fraser (smfr)
Modified: 2014-04-30 14:57 PDT (History)
4 users (show)

See Also:


Attachments
Patch (51.88 KB, patch)
2014-04-21 14:47 PDT, Simon Fraser (smfr)
darin: review+
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) 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