Make F12 close an undocked DevTools window rather than open another DevTools window for this one.

Make F12 close an undocked DevTools window rather than open another DevTools window for this one.


BUG=139581
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=149396

[apavlov, 2012-07-30 18:10:27]

Folks,

Please have a look at the patch. I'm not sure if this is the right way to tackle
it, but it works.

[yurys, 2012-07-31 14:10:21]

https://chromiumcodereview.appspot.com/10824089/diff/20/chrome/browser/debugg...
File chrome/browser/debugger/devtools_window.cc (right):

https://chromiumcodereview.appspot.com/10824089/diff/20/chrome/browser/debugg...
chrome/browser/debugger/devtools_window.cc:622: if
((*it)->tab_contents_->web_contents()->GetRenderViewHost() ==
Consider extracting this code and sharing it with isDevToolsWindow method.

https://chromiumcodereview.appspot.com/10824089/diff/20/chrome/browser/debugg...
chrome/browser/debugger/devtools_window.cc:625:
(*it)->inspected_tab_->web_contents()->GetRenderViewHost();
Note that inspected_tab_ may be NULL e.g. for shared worker inspector front-end.

[apavlov, 2012-07-31 14:38:03]

On 2012/07/31 14:10:21, Yury Semikhatsky wrote:
>
https://chromiumcodereview.appspot.com/10824089/diff/20/chrome/browser/debugg...
> File chrome/browser/debugger/devtools_window.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10824089/diff/20/chrome/browser/debugg...
> chrome/browser/debugger/devtools_window.cc:622: if
> ((*it)->tab_contents_->web_contents()->GetRenderViewHost() ==
> Consider extracting this code and sharing it with isDevToolsWindow method.

Fixed.

https://chromiumcodereview.appspot.com/10824089/diff/20/chrome/browser/debugg...
> chrome/browser/debugger/devtools_window.cc:625:
> (*it)->inspected_tab_->web_contents()->GetRenderViewHost();
> Note that inspected_tab_ may be NULL e.g. for shared worker inspector
front-end.

Fixed.

[yurys, 2012-07-31 15:16:31]

lgtm

https://chromiumcodereview.appspot.com/10824089/diff/5001/chrome/browser/debu...
File chrome/browser/debugger/devtools_window.cc (right):

https://chromiumcodereview.appspot.com/10824089/diff/5001/chrome/browser/debu...
chrome/browser/debugger/devtools_window.cc:662: DevToolsWindow*
DevToolsWindow::AsDevToolsWindow(RenderViewHost* window_rvh) {
window_rvh -> rvh as we don't know whether it is a DevToolsWindow's RVH.

[Nico, 2012-07-31 17:33:48]

Should the kDebugDevToolsFrontend be renamed to kLoadDevToolsFromDisk? It looks
partially redundant with the new flag at the moment.

[Nico, 2012-07-31 17:46:53]

lgtm

apavlov exmplained to me why this makes sense as is over gtalk. I hope he'll
post it here too :-)

[apavlov, 2012-08-01 10:19:20]

On 2012/07/31 17:46:53, Nico wrote:
> lgtm
> 
> apavlov exmplained to me why this makes sense as is over gtalk. I hope he'll
> post it here too :-)

I will! :-)

The existing flag specifies a path to the frontend, it's not a boolean. The new
flag is intended to add the respective menu item in the absence of the former,
and the new flag IS a boolean :) Yes, they will be redundant if specified at
once, but specifying the path without the debug mode makes no sense at all.

[commit-bot: I haz the power, 2012-08-01 10:19:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/apavlov@chromium.org/10824089/9001