[DevTools] Fallback to MainFrameNavigated event url in Security panel

[DevTools] Fallback to MainFrameNavigated event url in Security panel

If a HTTPS connection is blocked for security reasons, an interstitial
is presented and the Network Manager does not raise a ResponseReceived
event. Previously, the lack of a matching ResponseReceived event would
prevent the MainFrameNavigated event from correctly setting the "Main
Origin" for the sidebar.

With this CL, if the main navigation request was not previously
captured by the ResponseReceived event, we will instead compute the
Main Origin by looking at the URL in the MainFrameNavigated event
itself.

BUG=669309
TEST=blink_layouttests. Alternatively, open Security Panel. Navigate through https://expired.badssl.com/
interstitial. Verify Main Origin appears in sidebar.

Review-Url: https://codereview.chromium.org/2756983002
Cr-Commit-Position: refs/heads/master@{#458639}
Committed: https://chromium.googlesource.com/chromium/src/+/b37aaf2370cd187b34d4853e85ed6ed58b3665f9

[elawrence, 2017-03-20 14:55:45]

elawrence@chromium.org changed reviewers:
+ lushnikov@chromium.org

[elawrence, 2017-03-20 14:55:45]

PTAL? This small tweak to the Security panel ensures that we select the proper
Main Origin even if we haven't yet seen a request to that origin.

[elawrence, 2017-03-20 18:26:32]

Description was changed from

==========
Fallback to MainFrameNavigated event url in DevTools Security panel

If a HTTPS connection is blocked for security reasons, an interstitial
is presented and the Network Manager does not raise a ResponseReceived
event. Previously, the lack of a matching ResponseReceived event would
prevent the MainFrameNavigated event from correctly setting the "Main
Origin" for the sidebar.

With this CL, if the main navigation request was not previously
captured by the ResponseReceived event, we will instead compute the
Main Origin by looking at the URL in the MainFrameNavigated event
itself.

BUG=669309
TEST=Open Security Panel. Navigate through https://expired.badssl.com/
interstitial. Verify Main Origin appears in sidebar.
==========

to

==========
[DevTools] Fallback to MainFrameNavigated event url in Security panel

If a HTTPS connection is blocked for security reasons, an interstitial
is presented and the Network Manager does not raise a ResponseReceived
event. Previously, the lack of a matching ResponseReceived event would
prevent the MainFrameNavigated event from correctly setting the "Main
Origin" for the sidebar.

With this CL, if the main navigation request was not previously
captured by the ResponseReceived event, we will instead compute the
Main Origin by looking at the URL in the MainFrameNavigated event
itself.

BUG=669309
TEST=Open Security Panel. Navigate through https://expired.badssl.com/
interstitial. Verify Main Origin appears in sidebar.
==========

[elawrence, 2017-03-20 19:52:32]

Description was changed from

==========
[DevTools] Fallback to MainFrameNavigated event url in Security panel

If a HTTPS connection is blocked for security reasons, an interstitial
is presented and the Network Manager does not raise a ResponseReceived
event. Previously, the lack of a matching ResponseReceived event would
prevent the MainFrameNavigated event from correctly setting the "Main
Origin" for the sidebar.

With this CL, if the main navigation request was not previously
captured by the ResponseReceived event, we will instead compute the
Main Origin by looking at the URL in the MainFrameNavigated event
itself.

BUG=669309
TEST=Open Security Panel. Navigate through https://expired.badssl.com/
interstitial. Verify Main Origin appears in sidebar.
==========

to

==========
[DevTools] Fallback to MainFrameNavigated event url in Security panel

If a HTTPS connection is blocked for security reasons, an interstitial
is presented and the Network Manager does not raise a ResponseReceived
event. Previously, the lack of a matching ResponseReceived event would
prevent the MainFrameNavigated event from correctly setting the "Main
Origin" for the sidebar.

With this CL, if the main navigation request was not previously
captured by the ResponseReceived event, we will instead compute the
Main Origin by looking at the URL in the MainFrameNavigated event
itself.

BUG=669309
TEST=blink_layouttests. Alternatively, open Security Panel. Navigate through
https://expired.badssl.com/
interstitial. Verify Main Origin appears in sidebar.
==========

[lushnikov, 2017-03-21 18:26:36]

lgtm

https://codereview.chromium.org/2756983002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/security/SecurityPanel.js
(right):

https://codereview.chromium.org/2756983002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/security/SecurityPanel.js:353:
const origin = Common.ParsedURL.extractOrigin(request ? request.url() :
frame.url);
nit: we don't use 'const' since it causes V8 deoptimization

[elawrence, 2017-03-21 18:44:55]

Thanks!

> nit: we don't use 'const' since it causes V8 deoptimization

RE: The ban on 'const'-- Is this still in force? The JavaScript coding guide no
longer has mention of this; it was dropped in October 2016 in Revision #50. The
"issue" that was the original justification was marked as "Won't Fix" with the
note that 'const' had changed and should not have the performance impact that
led to its original ban: https://bugs.chromium.org/p/v8/issues/detail?id=1851.
There appear to be many other usages under
C:\src\c\src\third_party\WebKit\source\devtools\

I'm happy to change if it's required...

[lushnikov, 2017-03-21 23:33:04]

On 2017/03/21 18:44:55, elawrence wrote:
> Thanks!
> 
> > nit: we don't use 'const' since it causes V8 deoptimization
> 
> RE: The ban on 'const'-- Is this still in force? The JavaScript coding guide
no
> longer has mention of this; it was dropped in October 2016 in Revision #50.
The
> "issue" that was the original justification was marked as "Won't Fix" with the
> note that 'const' had changed and should not have the performance impact that
> led to its original ban: https://bugs.chromium.org/p/v8/issues/detail?id=1851.
> There appear to be many other usages under
> C:\src\c\src\third_party\WebKit\source\devtools\

Right, the const is not *that* slow anymore, but still causes deopt. We try to
not introduce
any new usages; we're yet to cleanup the code from the old ones and introduce a
presubmit hook.

> 
> I'm happy to change if it's required...

I'd appreciate that!

[elawrence, 2017-03-22 01:04:41]

thanks!

https://codereview.chromium.org/2756983002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/security/SecurityPanel.js
(right):

https://codereview.chromium.org/2756983002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/security/SecurityPanel.js:353:
const origin = Common.ParsedURL.extractOrigin(request ? request.url() :
frame.url);
On 2017/03/21 18:26:36, lushnikov wrote:
> nit: we don't use 'const' since it causes V8 deoptimization

Done.

[elawrence, 2017-03-22 01:05:05]

The CQ bit was checked by elawrence@chromium.org

[elawrence, 2017-03-22 01:05:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
lushnikov@chromium.org
Link to the patchset: https://codereview.chromium.org/2756983002/#ps40001
(title: "Rebase, change const to let")

[commit-bot: I haz the power, 2017-03-22 01:06:03]

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, 2017-03-22 03:07:32]

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

[commit-bot: I haz the power, 2017-03-22 03:07:33]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't
started yet, builder probably lacks capacity)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_TIMED_OUT,
build hasn't started yet, builder probably lacks capacity)

[elawrence, 2017-03-22 03:44:40]

The CQ bit was checked by elawrence@chromium.org

[commit-bot: I haz the power, 2017-03-22 03:45:07]

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, 2017-03-22 05:23:53]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490154280659990,
"parent_rev": "a19209825989de7d6b620aea95dd0f95b5c3c03f", "commit_rev":
"b37aaf2370cd187b34d4853e85ed6ed58b3665f9"}

[commit-bot: I haz the power, 2017-03-22 05:28:58]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490154280659990,
"parent_rev": "a19209825989de7d6b620aea95dd0f95b5c3c03f", "commit_rev":
"b37aaf2370cd187b34d4853e85ed6ed58b3665f9"}

[commit-bot: I haz the power, 2017-03-22 05:29:01]

Description was changed from

==========
[DevTools] Fallback to MainFrameNavigated event url in Security panel

If a HTTPS connection is blocked for security reasons, an interstitial
is presented and the Network Manager does not raise a ResponseReceived
event. Previously, the lack of a matching ResponseReceived event would
prevent the MainFrameNavigated event from correctly setting the "Main
Origin" for the sidebar.

With this CL, if the main navigation request was not previously
captured by the ResponseReceived event, we will instead compute the
Main Origin by looking at the URL in the MainFrameNavigated event
itself.

BUG=669309
TEST=blink_layouttests. Alternatively, open Security Panel. Navigate through
https://expired.badssl.com/
interstitial. Verify Main Origin appears in sidebar.
==========

to

==========
[DevTools] Fallback to MainFrameNavigated event url in Security panel

If a HTTPS connection is blocked for security reasons, an interstitial
is presented and the Network Manager does not raise a ResponseReceived
event. Previously, the lack of a matching ResponseReceived event would
prevent the MainFrameNavigated event from correctly setting the "Main
Origin" for the sidebar.

With this CL, if the main navigation request was not previously
captured by the ResponseReceived event, we will instead compute the
Main Origin by looking at the URL in the MainFrameNavigated event
itself.

BUG=669309
TEST=blink_layouttests. Alternatively, open Security Panel. Navigate through
https://expired.badssl.com/
interstitial. Verify Main Origin appears in sidebar.

Review-Url: https://codereview.chromium.org/2756983002
Cr-Commit-Position: refs/heads/master@{#458639}
Committed:
https://chromium.googlesource.com/chromium/src/+/b37aaf2370cd187b34d4853e85ed...
==========

[commit-bot: I haz the power, 2017-03-22 05:29:02]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/b37aaf2370cd187b34d4853e85ed...