Implement a ShowSecurityPanel() call in the browser.

Implement a ShowSecurityPanel() call in the browser.

BUG=543902
Committed: https://crrev.com/089e2aa3b01b0171419e29edd4ef6eae8b3ee109
Cr-Commit-Position: refs/heads/master@{#359521}

[lgarron, 2015-11-11 10:51:09]

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

[lgarron, 2015-11-11 10:51:10]

pfeldman@, could you review?

This is only the internal plumbing. The next step is to call this from Views and
Mac code.

I've tested this change by replacing 
DevToolsToggleAction::ShowConsole() with
DevToolsToggleAction::ShowSecurityPanel() under IDC_DEV_TOOLS_CONSOLE in
browser_command_controller.cc, and it correctly opens the DevTools to the
Security panel.

[lgarron, 2015-11-11 11:01:51]

https://codereview.chromium.org/1433953002/diff/20001/chrome/browser/devtools...
File chrome/browser/devtools/devtools_window.cc (right):

https://codereview.chromium.org/1433953002/diff/20001/chrome/browser/devtools...
chrome/browser/devtools/devtools_window.cc:460:
ToggleDevToolsWindow(browser->tab_strip_model()->GetActiveWebContents(),
External code can't call this private ToggleDevToolsWindow() implementation with
a custom force_open value directly, so I've opted for matching the way we
calculate force_open for Inspect().

[lgarron, 2015-11-11 19:03:39]

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

[commit-bot: I haz the power, 2015-11-11 19:04:08]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1433953002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1433953002/20001

[commit-bot: I haz the power, 2015-11-11 20:08:34]

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

[commit-bot: I haz the power, 2015-11-11 20:08:35]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_android_r...)

[lgarron, 2015-11-11 21:24:23]

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

[commit-bot: I haz the power, 2015-11-11 21:24:31]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1433953002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1433953002/20001

[commit-bot: I haz the power, 2015-11-11 22:46:02]

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

[commit-bot: I haz the power, 2015-11-11 22:46:03]

Dry run: This issue passed the CQ dry run.

[pfeldman, 2015-11-12 18:58:02]

https://codereview.chromium.org/1433953002/diff/20001/chrome/browser/devtools...
File chrome/browser/devtools/devtools_window.cc (right):

https://codereview.chromium.org/1433953002/diff/20001/chrome/browser/devtools...
chrome/browser/devtools/devtools_window.cc:459: action.type() ==
DevToolsToggleAction::kShowSecurityPanel;
This way it is unclear why Console behaves differently from SecurityPanel. Lets
expose a method that allows force-opening devtools with any of the actions.

https://codereview.chromium.org/1433953002/diff/20001/chrome/browser/devtools...
chrome/browser/devtools/devtools_window.cc:1205:
bindings_->CallClientFunction("DevToolsAPI.showSecurityPanel", NULL, NULL,
Lets generalize to "showPanel", "security"

[lgarron, 2015-11-13 00:03:38]

https://codereview.chromium.org/1433953002/diff/20001/chrome/browser/devtools...
File chrome/browser/devtools/devtools_window.cc (right):

https://codereview.chromium.org/1433953002/diff/20001/chrome/browser/devtools...
chrome/browser/devtools/devtools_window.cc:459: action.type() ==
DevToolsToggleAction::kShowSecurityPanel;
On 2015/11/12 at 18:58:02, pfeldman wrote:
> This way it is unclear why Console behaves differently from SecurityPanel.
Lets expose a method that allows force-opening devtools with any of the actions.

So, we may or may not need one.
The call from //c/b/ui/browser_command_controller.cc is:

browser_commands::ToggleDevToolsWindow(browser, DevToolsToggleAction action)
DevToolsWindow::ToggleDevToolsWindow(browser, action)
DevToolsWindow::ToggleDevToolsWindow(inspected_web_contents, force_open,
DevToolsToggleAction& action, settings)

However, it would, say, already be possible to call:

DevToolsWindow::OpenDevToolsWindow(browser->tab_strip_model()->GetActiveWebContents(),
DevToolsToggleAction::ShowSecurityPanel())

Are you okay with expecting code to call into DevToolsWindow() if they want to
force-open, or would you prefer a layer of indirection with
browser_command_controller?

[lgarron, 2015-11-13 00:58:39]

pfeldman@, could you review the latest patch?

[pfeldman, 2015-11-13 01:33:49]

https://codereview.chromium.org/1433953002/diff/40001/chrome/browser/devtools...
File chrome/browser/devtools/devtools_window.cc (right):

https://codereview.chromium.org/1433953002/diff/40001/chrome/browser/devtools...
chrome/browser/devtools/devtools_window.cc:1198: 
revert

https://codereview.chromium.org/1433953002/diff/40001/chrome/browser/devtools...
chrome/browser/devtools/devtools_window.cc:1202: "DevToolsAPI.showConsole",
NULL, NULL, NULL);
I meant that you refactor this one to use the new method.

https://codereview.chromium.org/1433953002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/components/InspectorView.js
(right):

https://codereview.chromium.org/1433953002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/components/InspectorView.js:85:
this.showPanel("console");
This would go then

[lgarron, 2015-11-13 01:34:46]

https://codereview.chromium.org/1433953002/diff/40001/chrome/browser/devtools...
File chrome/browser/devtools/devtools_window.cc (right):

https://codereview.chromium.org/1433953002/diff/40001/chrome/browser/devtools...
chrome/browser/devtools/devtools_window.cc:1202: "DevToolsAPI.showConsole",
NULL, NULL, NULL);
On 2015/11/13 at 01:33:49, pfeldman wrote:
> I meant that you refactor this one to use the new method.

I was gonna do this in a follow-up CL. Would you prefer it in a single CL?

[pfeldman, 2015-11-13 01:35:32]

On 2015/11/13 01:34:46, lgarron wrote:
>
https://codereview.chromium.org/1433953002/diff/40001/chrome/browser/devtools...
> File chrome/browser/devtools/devtools_window.cc (right):
> 
>
https://codereview.chromium.org/1433953002/diff/40001/chrome/browser/devtools...
> chrome/browser/devtools/devtools_window.cc:1202: "DevToolsAPI.showConsole",
> NULL, NULL, NULL);
> On 2015/11/13 at 01:33:49, pfeldman wrote:
> > I meant that you refactor this one to use the new method.
> 
> I was gonna do this in a follow-up CL. Would you prefer it in a single CL?

Yeah, it would be few lines, right?

[lgarron, 2015-11-13 01:37:32]

On 2015/11/13 at 01:35:32, pfeldman wrote:
> On 2015/11/13 01:34:46, lgarron wrote:
> >
https://codereview.chromium.org/1433953002/diff/40001/chrome/browser/devtools...
> > File chrome/browser/devtools/devtools_window.cc (right):
> > 
> >
https://codereview.chromium.org/1433953002/diff/40001/chrome/browser/devtools...
> > chrome/browser/devtools/devtools_window.cc:1202: "DevToolsAPI.showConsole",
> > NULL, NULL, NULL);
> > On 2015/11/13 at 01:33:49, pfeldman wrote:
> > > I meant that you refactor this one to use the new method.
> > 
> > I was gonna do this in a follow-up CL. Would you prefer it in a single CL?
> 
> Yeah, it would be few lines, right?

Doing it properly would also mean taking out the showConsole definitions in all
the files. But sure, I'll go ahead and put it in this CL.

[lgarron, 2015-11-13 01:44:14]

See latest patch.

[pfeldman, 2015-11-13 01:49:49]

lgtm

[lgarron, 2015-11-13 01:56:26]

The CQ bit was checked by lgarron@chromium.org

[commit-bot: I haz the power, 2015-11-13 01:57:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1433953002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1433953002/80001

[commit-bot: I haz the power, 2015-11-13 03:28:48]

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

[commit-bot: I haz the power, 2015-11-13 03:28:49]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[lgarron, 2015-11-13 08:54:54]

The CQ bit was checked by lgarron@chromium.org

[commit-bot: I haz the power, 2015-11-13 08:55:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1433953002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1433953002/80001

[commit-bot: I haz the power, 2015-11-13 09:35:35]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-11-13 09:36:34]

Patchset 5 (id:??) landed as
https://crrev.com/089e2aa3b01b0171419e29edd4ef6eae8b3ee109
Cr-Commit-Position: refs/heads/master@{#359521}