DevTools: fix instances where asParsedURL is null

DevTools: fix instances where asParsedURL is null

BUG=639540
Committed: https://crrev.com/8bb0fdf8b67c2671d9c2282e40d0502108c5c12a
Cr-Commit-Position: refs/heads/master@{#413591}

[luoe, 2016-08-22 16:53:39]

Description was changed from

==========
Fix instances with parsedURL

BUG=
==========

to

==========
DevTools: fix instances where asParsedURL is null

BUG=639540
==========

[luoe, 2016-08-22 16:58:05]

luoe@chromium.org changed reviewers:
+ allada@chromium.org, dgozman@chromium.org, pfeldman@chromium.org

[luoe, 2016-08-22 16:58:06]

Since asParsedURL() can return null, we should always check that it's truthy
before using it.  Cases such as "about:blank" and data URIs are null cases.

I'm not sure how ServiceWorkerManager.js works, so I'd like some review on that
change.

Related:
https://codereview.chromium.org/2266473003/

[dgozman, 2016-08-22 17:05:43]

https://codereview.chromium.org/2265823002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js
(right):

https://codereview.chromium.org/2265823002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:621: var
filename = String.sprintf("%s-%d.log", parsedURL ? parsedURL.host :
WebInspector.UIString("log"), Date.now());
- log -> console;
- remove UIString, as it's not a user-readable string.

https://codereview.chromium.org/2265823002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js
(right):

https://codereview.chromium.org/2265823002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:991: var
filename = parsedURL ? parsedURL.host : WebInspector.UIString("filename");
Let's use |url| as a fallback.

https://codereview.chromium.org/2265823002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sdk/ServiceWorkerManager.js
(right):

https://codereview.chromium.org/2265823002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/ServiceWorkerManager.js:683:
this.securityOrigin = parsedURL.securityOrigin();
Missing the check here - use empty as a fallback.

https://codereview.chromium.org/2265823002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/ServiceWorkerManager.js:828:
this.securityOrigin = parsedURL.securityOrigin();
ditto

[luoe, 2016-08-22 17:40:40]

https://codereview.chromium.org/2265823002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js
(right):

https://codereview.chromium.org/2265823002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:621: var
filename = String.sprintf("%s-%d.log", parsedURL ? parsedURL.host :
WebInspector.UIString("log"), Date.now());
On 2016/08/22 17:05:42, dgozman wrote:
> - log -> console;
> - remove UIString, as it's not a user-readable string.

Done.

https://codereview.chromium.org/2265823002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js
(right):

https://codereview.chromium.org/2265823002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:991: var
filename = parsedURL ? parsedURL.host : WebInspector.UIString("filename");
On 2016/08/22 17:05:43, dgozman wrote:
> Let's use |url| as a fallback.

Done.

https://codereview.chromium.org/2265823002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sdk/ServiceWorkerManager.js
(right):

https://codereview.chromium.org/2265823002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/ServiceWorkerManager.js:683:
this.securityOrigin = parsedURL.securityOrigin();
On 2016/08/22 17:05:43, dgozman wrote:
> Missing the check here - use empty as a fallback.

parsedURL is never null, so securityOrigin will not fail here.  securityOrigin()
returns "data:" for data urls, "about://" for "about:blank", and "://" for other
unmatched cases.

Just to confirm, do we want empty string "" for all these invalid cases?

[dgozman, 2016-08-22 19:38:24]

lgtm

https://codereview.chromium.org/2265823002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sdk/ServiceWorkerManager.js
(right):

https://codereview.chromium.org/2265823002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/ServiceWorkerManager.js:683:
this.securityOrigin = parsedURL.securityOrigin();
On 2016/08/22 17:40:40, luoe wrote:
> On 2016/08/22 17:05:43, dgozman wrote:
> > Missing the check here - use empty as a fallback.
> 
> parsedURL is never null, so securityOrigin will not fail here. 
securityOrigin()
> returns "data:" for data urls, "about://" for "about:blank", and "://" for
other
> unmatched cases.
> 
> Just to confirm, do we want empty string "" for all these invalid cases?

I've misread it. This is good enough.

https://codereview.chromium.org/2265823002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js
(right):

https://codereview.chromium.org/2265823002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:991: var
filename = parsedURL ? parsedURL.host : "url";
"url" -> "network-log"

[luoe, 2016-08-22 19:40:51]

https://codereview.chromium.org/2265823002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sdk/ServiceWorkerManager.js
(right):

https://codereview.chromium.org/2265823002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/ServiceWorkerManager.js:683:
this.securityOrigin = parsedURL.securityOrigin();
On 2016/08/22 19:38:24, dgozman wrote:
> On 2016/08/22 17:40:40, luoe wrote:
> > On 2016/08/22 17:05:43, dgozman wrote:
> > > Missing the check here - use empty as a fallback.
> > 
> > parsedURL is never null, so securityOrigin will not fail here. 
> securityOrigin()
> > returns "data:" for data urls, "about://" for "about:blank", and "://" for
> other
> > unmatched cases.
> > 
> > Just to confirm, do we want empty string "" for all these invalid cases?
> 
> I've misread it. This is good enough.

Done.

https://codereview.chromium.org/2265823002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js
(right):

https://codereview.chromium.org/2265823002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:991: var
filename = parsedURL ? parsedURL.host : "url";
On 2016/08/22 19:38:24, dgozman wrote:
> "url" -> "network-log"

Done.

[luoe, 2016-08-22 19:41:20]

The CQ bit was checked by luoe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-22 19:42:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-22 23:42:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-22 23:42:18]

Dry run: This issue passed the CQ dry run.

[luoe, 2016-08-22 23:59:32]

The CQ bit was checked by luoe@chromium.org

[luoe, 2016-08-22 23:59:32]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/2265823002/#ps60001
(title: "Fix instances with parsedURL")

[commit-bot: I haz the power, 2016-08-22 23:59:57]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-23 00:09:20]

Description was changed from

==========
DevTools: fix instances where asParsedURL is null

BUG=639540
==========

to

==========
DevTools: fix instances where asParsedURL is null

BUG=639540
==========

[commit-bot: I haz the power, 2016-08-23 00:09:21]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-08-23 00:10:53]

Description was changed from

==========
DevTools: fix instances where asParsedURL is null

BUG=639540
==========

to

==========
DevTools: fix instances where asParsedURL is null

BUG=639540

Committed: https://crrev.com/8bb0fdf8b67c2671d9c2282e40d0502108c5c12a
Cr-Commit-Position: refs/heads/master@{#413591}
==========

[commit-bot: I haz the power, 2016-08-23 00:10:54]

Patchset 4 (id:??) landed as
https://crrev.com/8bb0fdf8b67c2671d9c2282e40d0502108c5c12a
Cr-Commit-Position: refs/heads/master@{#413591}