Bug 213450 - [WTF] URL should support dataLog
Summary: [WTF] URL should support dataLog
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-06-21 19:26 PDT by Yusuke Suzuki
Modified: 2020-06-21 23:51 PDT (History)
6 users (show)

See Also:


Attachments
Patch (3.37 KB, patch)
2020-06-21 19:29 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (3.37 KB, patch)
2020-06-21 19:39 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (3.37 KB, patch)
2020-06-21 19:47 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2020-06-21 19:26:49 PDT
[WTF] URL should support dataLog
Comment 1 Yusuke Suzuki 2020-06-21 19:29:14 PDT
Created attachment 402439 [details]
Patch
Comment 2 Yusuke Suzuki 2020-06-21 19:39:27 PDT
Created attachment 402440 [details]
Patch
Comment 3 Yusuke Suzuki 2020-06-21 19:47:15 PDT
Created attachment 402441 [details]
Patch
Comment 4 Yusuke Suzuki 2020-06-21 19:48:53 PDT
I need to admit that we should eventually merge TextStream (WebCore side thing) and PrintStream (WTF / JSC side thing) into one, but it requires a grand plan anyway :)
Comment 5 Mark Lam 2020-06-21 19:53:04 PDT
Comment on attachment 402441 [details]
Patch

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

r=me with nit.

> Source/WTF/wtf/URL.cpp:770
> +void URL::dump(PrintStream& out) const
> +{
> +    out.print(m_string);
> +}

nit: should we keep this definition in the header instead?  If it's not normally used and we only need this for occasional debugging use, then it is more efficient to have it in the header vs explicitly putting it here.  There a lot of other less commonly dumped data structures that do this also.
Comment 6 Yusuke Suzuki 2020-06-21 20:07:43 PDT
Comment on attachment 402441 [details]
Patch

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

>> Source/WTF/wtf/URL.cpp:770
>> +}
> 
> nit: should we keep this definition in the header instead?  If it's not normally used and we only need this for occasional debugging use, then it is more efficient to have it in the header vs explicitly putting it here.  There a lot of other less commonly dumped data structures that do this also.

Discussed with Mark on Slack. We keep it as is to avoid including `#include <wtf/PrintStream.h>` in `URL.h`.
Comment 7 EWS 2020-06-21 23:50:24 PDT
Committed r263340: <https://trac.webkit.org/changeset/263340>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 402441 [details].
Comment 8 Radar WebKit Bug Importer 2020-06-21 23:51:16 PDT
<rdar://problem/64585651>