DevTools security panel: prompt user to reload for mixed requests

DevTools security panel: prompt user to reload for mixed requests

When there are no network requests recorded, prompt the user to reload,
instead of making them click over to the network panel where they are
prompted to reload.

BUG=648646
Committed: https://crrev.com/81e423ee392a3b383431c906b6a2767fcfdf0764
Cr-Commit-Position: refs/heads/master@{#427240}

[estark, 2016-09-20 17:25:23]

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

[commit-bot: I haz the power, 2016-09-20 17:25:43]

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

[estark, 2016-09-20 17:25:59]

Description was changed from

==========
DevTools security panel: prompt user to reload for mixed requests

When there are no network requests recorded, prompt the user to reload,
instead of making them click over to the network panel where they are
prompted to reload.

BUG=648646
==========

to

==========
DevTools security panel: prompt user to reload for mixed requests

When there are no network requests recorded, prompt the user to reload,
instead of making them click over to the network panel where they are
prompted to reload.

BUG=648646
==========

[estark, 2016-09-20 17:28:22]

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

[estark, 2016-09-20 17:28:23]

elawrence: if you want to grab this and finish it by adding some more tests,
here's how:
1. Create a new local branch, run `git cl patch 2354873003` to patch it in and
then `git cl issue 0` to disassociate your branch from this issue.
2. I think we could use a few more tests just like mixed-content-reload.html,
except where is both active and passive mixed content, and where there is active
mixed content only.

[pfeldman, 2016-09-20 18:14:46]

pfeldman@chromium.org changed reviewers:
+ pfeldman@chromium.org

[pfeldman, 2016-09-20 18:14:47]

Please provide a screenshot in the bug or in the codereview along for all visual
changes.

Otherwise code looks good.

https://codereview.chromium.org/2354873003/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/security/SecurityPanel.js
(right):

https://codereview.chromium.org/2354873003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/security/SecurityPanel.js:775: if
(filterRequestCount == 0) {
style: !filterRequestCount (Blink preferred) or
filterRequestCount === 0

[caseq, 2016-09-20 18:16:40]

caseq@chromium.org changed reviewers:
+ caseq@chromium.org

[caseq, 2016-09-20 18:16:41]

https://codereview.chromium.org/2354873003/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/security/SecurityPanel.js
(right):

https://codereview.chromium.org/2354873003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/security/SecurityPanel.js:775: if
(filterRequestCount == 0) {
"if (!filterRequestCount)" (we usually don't compare to 0, and generally use ===
instead of ==)

https://codereview.chromium.org/2354873003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/security/SecurityPanel.js:776: //
Network instrumentation might not have been enabled for the page load, so the
security panel does not necessarily know a count of individual mixed requests at
this point. Prompt them to refresh instead of pointing them to the Network panel
to get prompted to refresh.
please split into multiple lines.

https://codereview.chromium.org/2354873003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/security/SecurityPanel.js:783:
requestsAnchor.textContent = WebInspector.UIString("View %d request%s in Network
Panel", filterRequestCount, (filterRequestCount > 1 ? "s" : ""));
JFYI since you were just moving this around -- this is a localization land mine,
should either be two different messages for singular/plural, or just something
like "View %d request(s)".

[estark, 2016-09-20 18:48:07]

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

[commit-bot: I haz the power, 2016-09-20 18:48:41]

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

[estark, 2016-09-20 18:50:05]

Thanks, both. I added a screenshot to the bug.

https://codereview.chromium.org/2354873003/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/security/SecurityPanel.js
(right):

https://codereview.chromium.org/2354873003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/security/SecurityPanel.js:775: if
(filterRequestCount == 0) {
On 2016/09/20 18:16:40, caseq wrote:
> "if (!filterRequestCount)" (we usually don't compare to 0, and generally use
===
> instead of ==)

Done.

https://codereview.chromium.org/2354873003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/security/SecurityPanel.js:775: if
(filterRequestCount == 0) {
On 2016/09/20 18:14:47, pfeldman wrote:
> style: !filterRequestCount (Blink preferred) or
> filterRequestCount === 0

Done.

https://codereview.chromium.org/2354873003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/security/SecurityPanel.js:776: //
Network instrumentation might not have been enabled for the page load, so the
security panel does not necessarily know a count of individual mixed requests at
this point. Prompt them to refresh instead of pointing them to the Network panel
to get prompted to refresh.
On 2016/09/20 18:16:40, caseq wrote:
> please split into multiple lines.

Done.

https://codereview.chromium.org/2354873003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/security/SecurityPanel.js:783:
requestsAnchor.textContent = WebInspector.UIString("View %d request%s in Network
Panel", filterRequestCount, (filterRequestCount > 1 ? "s" : ""));
On 2016/09/20 18:16:40, caseq wrote:
> JFYI since you were just moving this around -- this is a localization land
mine,
> should either be two different messages for singular/plural, or just something
> like "View %d request(s)".

Done.

[caseq, 2016-09-20 18:52:01]

lgtm

[commit-bot: I haz the power, 2016-09-20 19:24:14]

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

[commit-bot: I haz the power, 2016-09-20 19:24:15]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[estark, 2016-10-24 23:27:12]

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

[commit-bot: I haz the power, 2016-10-24 23:28:00]

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-10-25 01:19:31]

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

[commit-bot: I haz the power, 2016-10-25 01:19:36]

Dry run: This issue passed the CQ dry run.

[estark, 2016-10-25 01:32:15]

The CQ bit was checked by estark@chromium.org

[estark, 2016-10-25 01:32:16]

The patchset sent to the CQ was uploaded after l-g-t-m from caseq@chromium.org
Link to the patchset: https://codereview.chromium.org/2354873003/#ps60001
(title: "add a test that includes active mixed content")

[commit-bot: I haz the power, 2016-10-25 01:32:59]

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

[elawrence, 2016-10-25 01:36:37]

The CQ bit was unchecked by elawrence@chromium.org

[estark, 2016-10-25 01:44:41]

The CQ bit was checked by estark@chromium.org

[commit-bot: I haz the power, 2016-10-25 01:45:14]

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-10-25 02:07:24]

Description was changed from

==========
DevTools security panel: prompt user to reload for mixed requests

When there are no network requests recorded, prompt the user to reload,
instead of making them click over to the network panel where they are
prompted to reload.

BUG=648646
==========

to

==========
DevTools security panel: prompt user to reload for mixed requests

When there are no network requests recorded, prompt the user to reload,
instead of making them click over to the network panel where they are
prompted to reload.

BUG=648646
==========

[commit-bot: I haz the power, 2016-10-25 02:07:25]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-25 02:11:34]

Description was changed from

==========
DevTools security panel: prompt user to reload for mixed requests

When there are no network requests recorded, prompt the user to reload,
instead of making them click over to the network panel where they are
prompted to reload.

BUG=648646
==========

to

==========
DevTools security panel: prompt user to reload for mixed requests

When there are no network requests recorded, prompt the user to reload,
instead of making them click over to the network panel where they are
prompted to reload.

BUG=648646

Committed: https://crrev.com/81e423ee392a3b383431c906b6a2767fcfdf0764
Cr-Commit-Position: refs/heads/master@{#427240}
==========

[commit-bot: I haz the power, 2016-10-25 02:11:35]

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