Fix bugs with renderer-side debug URLs, like chrome://crash or javascript:.

Fix bugs with renderer-side debug URLs, like chrome://crash or javascript:.

These URLs should not cause a process swap when leaving pages like extensions
or view-source.  Also, they should be ignored if the renderer process is not
live.

BUG=335503
BUG=334214
TEST=chrome://kill works on extensions and is ignored on crashed tabs.

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

[Charlie Reis, 2014-01-31 23:34:24]

Nasko, can you take a look?  This fixes both http://crbug.com/335503 and
http://crbug.com/334214.  Those are technically independent bugs, but they both
depend on abstracting out IsRendererDebugURL, and they both need to be merged to
fix regressions.

[nasko, 2014-01-31 23:56:07]

Just one comment, otherwise LGTM.

https://codereview.chromium.org/151593004/diff/50001/content/browser/frame_ho...
File content/browser/frame_host/debug_urls.cc (right):

https://codereview.chromium.org/151593004/diff/50001/content/browser/frame_ho...
content/browser/frame_host/debug_urls.cc:93: url == GURL(kChromeUIShorthangURL);
Shouldn't that list include the rest of the ones present in HandleDebugURL?
Going to chrome://chrome-urls/ lists 7 extra URLs.

https://codereview.chromium.org/151593004/diff/50001/content/browser/frame_ho...
File content/browser/frame_host/render_frame_host_manager.cc (right):

https://codereview.chromium.org/151593004/diff/50001/content/browser/frame_ho...
content/browser/frame_host/render_frame_host_manager.cc:606: // Don't force a
new BrowsingInstance for renderer-side debug URLs like
nit: "renderer-side" to me implies that those are URLs coming from the renderer,
which isn't really the case.

[Charlie Reis, 2014-01-31 23:59:21]

Thanks.  Hope this clears it up.

https://codereview.chromium.org/151593004/diff/50001/content/browser/frame_ho...
File content/browser/frame_host/debug_urls.cc (right):

https://codereview.chromium.org/151593004/diff/50001/content/browser/frame_ho...
content/browser/frame_host/debug_urls.cc:93: url == GURL(kChromeUIShorthangURL);
On 2014/01/31 23:56:07, nasko wrote:
> Shouldn't that list include the rest of the ones present in HandleDebugURL?
> Going to chrome://chrome-urls/ lists 7 extra URLs.

No, there's a distinction between those 7 (which are handled in the browser
process and never get sent to the renderer) and these (which must be sent to the
renderer process and are handled there).

https://codereview.chromium.org/151593004/diff/50001/content/browser/frame_ho...
File content/browser/frame_host/render_frame_host_manager.cc (right):

https://codereview.chromium.org/151593004/diff/50001/content/browser/frame_ho...
content/browser/frame_host/render_frame_host_manager.cc:606: // Don't force a
new BrowsingInstance for renderer-side debug URLs like
On 2014/01/31 23:56:07, nasko wrote:
> nit: "renderer-side" to me implies that those are URLs coming from the
renderer,
> which isn't really the case.

I was trying to convey that they get processed in the renderer process, unlike
the other debug URLs.  I'll clarify it in the next patch.

[Charlie Reis, 2014-02-01 00:03:40]

I've updated the comments in the latest patch set.

[nasko, 2014-02-01 00:05:05]

Still LGTM

https://codereview.chromium.org/151593004/diff/50001/content/browser/frame_ho...
File content/browser/frame_host/debug_urls.cc (right):

https://codereview.chromium.org/151593004/diff/50001/content/browser/frame_ho...
content/browser/frame_host/debug_urls.cc:93: url == GURL(kChromeUIShorthangURL);
On 2014/01/31 23:59:22, Charlie Reis wrote:
> On 2014/01/31 23:56:07, nasko wrote:
> > Shouldn't that list include the rest of the ones present in HandleDebugURL?
> > Going to chrome://chrome-urls/ lists 7 extra URLs.
> 
> No, there's a distinction between those 7 (which are handled in the browser
> process and never get sent to the renderer) and these (which must be sent to
the
> renderer process and are handled there).

Ah, that makes sense. Thanks!

[Charlie Reis, 2014-02-02 22:21:18]

The CQ bit was checked by creis@chromium.org

[commit-bot: I haz the power, 2014-02-02 22:21:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/creis@chromium.org/151593004/150001

[commit-bot: I haz the power, 2014-02-03 06:54:11]

CQ bit was unchecked on CL. Ignoring.

[commit-bot: I haz the power, 2014-02-03 06:54:11]

Change committed as 248442

[commit-bot: I haz the power, 2014-02-03 06:54:16]

CQ bit was unchecked on CL. Ignoring.