Rename RenderProcessHost::IsGuest to RenderProcessHost::IsIsolatedGuest

Rename RenderProcessHost::IsGuest to RenderProcessHost::IsIsolatedGuest

Not all guests are created equal. <webview> guests are isolated from other processes.
In other words, the content of a <webview> cannot live in the same process as any other
non-<webview> content.

This is not true for <appview>. <appview> guests are NOT isolated. In the future,
BrowserPluginGuest::IsGuest and GuestViewBase::IsGuest may return true even if
RenderProcessHost::IsIsolatedGuest returns false.

BUG=364141, 330264
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=273733

[lazyboy, 2014-05-27 19:22:22]

Sorry, I forgot about this one.

https://chromiumcodereview.appspot.com/293093010/diff/1/content/browser/brows...
File content/browser/browser_plugin/browser_plugin_guest.cc (left):

https://chromiumcodereview.appspot.com/293093010/diff/1/content/browser/brows...
content/browser/browser_plugin/browser_plugin_guest.cc:618: // The guest
RenderView should always live in a guest process.
I'd keep this until we start upstreaming appview code.

https://chromiumcodereview.appspot.com/293093010/diff/1/content/browser/rende...
File content/browser/renderer_host/render_process_host_impl.h (right):

https://chromiumcodereview.appspot.com/293093010/diff/1/content/browser/rende...
content/browser/renderer_host/render_process_host_impl.h:225: void
SetIsGuestForTesting(bool is_isolated_guest) {
set_is_isolated_guest_for_testing()

https://chromiumcodereview.appspot.com/293093010/diff/1/content/public/browse...
File content/public/browser/render_process_host.h (right):

https://chromiumcodereview.appspot.com/293093010/diff/1/content/public/browse...
content/public/browser/render_process_host.h:98: // Indicates whether the
current RenderProcessHost associated with a guest
This needs description, mostly what you have in CL description.

https://chromiumcodereview.appspot.com/293093010/diff/1/content/public/test/m...
File content/public/test/mock_render_process_host.h (right):

https://chromiumcodereview.appspot.com/293093010/diff/1/content/public/test/m...
content/public/test/mock_render_process_host.h:100: void SetIsIsolatedGuest(bool
is_guest) {
Since this is direct setter, should be set_is_isolated_guest()

[Fady Samuel, 2014-05-27 21:56:34]

PTAL Istiaque
+nasko@

https://codereview.chromium.org/293093010/diff/1/content/browser/browser_plug...
File content/browser/browser_plugin/browser_plugin_guest.cc (left):

https://codereview.chromium.org/293093010/diff/1/content/browser/browser_plug...
content/browser/browser_plugin/browser_plugin_guest.cc:618: // The guest
RenderView should always live in a guest process.
On 2014/05/27 19:22:22, lazyboy wrote:
> I'd keep this until we start upstreaming appview code.

This seems more appropriately in WebViewGuest. I've moved it there.

https://codereview.chromium.org/293093010/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/render_process_host_impl.h (right):

https://codereview.chromium.org/293093010/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/render_process_host_impl.h:225: void
SetIsGuestForTesting(bool is_isolated_guest) {
On 2014/05/27 19:22:22, lazyboy wrote:
> set_is_isolated_guest_for_testing()

Done.

https://codereview.chromium.org/293093010/diff/1/content/public/browser/rende...
File content/public/browser/render_process_host.h (right):

https://codereview.chromium.org/293093010/diff/1/content/public/browser/rende...
content/public/browser/render_process_host.h:98: // Indicates whether the
current RenderProcessHost associated with a guest
On 2014/05/27 19:22:22, lazyboy wrote:
> This needs description, mostly what you have in CL description.

Done.

https://codereview.chromium.org/293093010/diff/1/content/public/test/mock_ren...
File content/public/test/mock_render_process_host.h (right):

https://codereview.chromium.org/293093010/diff/1/content/public/test/mock_ren...
content/public/test/mock_render_process_host.h:100: void SetIsIsolatedGuest(bool
is_guest) {
On 2014/05/27 19:22:22, lazyboy wrote:
> Since this is direct setter, should be set_is_isolated_guest()

It looks like this isn't even used. I've removed it.

[lazyboy, 2014-05-28 00:13:03]

lgtm

[nasko, 2014-05-28 16:29:13]

https://codereview.chromium.org/293093010/diff/10001/content/public/browser/r...
File content/public/browser/render_process_host.h (right):

https://codereview.chromium.org/293093010/diff/10001/content/public/browser/r...
content/public/browser/render_process_host.h:101: // other non-guest renderers
in the same process if IsSIsolatedGuest is false.
nit: IsIsolatedGuest is misspelled.

https://codereview.chromium.org/293093010/diff/10001/content/public/test/mock...
File content/public/test/mock_render_process_host.cc (right):

https://codereview.chromium.org/293093010/diff/10001/content/public/test/mock...
content/public/test/mock_render_process_host.cc:104: return false;
Is it no longer true that content/ on its own can have isolated guests? It seems
to me that we need a test case inside content/ that ensures the isolation is
working without depending on chrome/ for it.

[fsamuel, 2014-05-28 16:37:01]

PTAL

https://codereview.chromium.org/293093010/diff/10001/content/public/browser/r...
File content/public/browser/render_process_host.h (right):

https://codereview.chromium.org/293093010/diff/10001/content/public/browser/r...
content/public/browser/render_process_host.h:101: // other non-guest renderers
in the same process if IsSIsolatedGuest is false.
On 2014/05/28 16:29:13, nasko wrote:
> nit: IsIsolatedGuest is misspelled.

Done.

https://codereview.chromium.org/293093010/diff/10001/content/public/test/mock...
File content/public/test/mock_render_process_host.cc (right):

https://codereview.chromium.org/293093010/diff/10001/content/public/test/mock...
content/public/test/mock_render_process_host.cc:104: return false;
On 2014/05/28 16:29:13, nasko wrote:
> Is it no longer true that content/ on its own can have isolated guests? It
seems
> to me that we need a test case inside content/ that ensures the isolation is
> working without depending on chrome/ for it.

Yes, it is true. BrowserPlugin is now inert without chrome.

[nasko, 2014-05-28 16:38:26]

Please file a bug to add a test for isolated guests that lives entirely in
content, so we make sure we don't regress this. Otherwise LGTM.

[Fady Samuel, 2014-05-28 17:39:30]

Filed bug:
https://code.google.com/p/chromium/issues/detail?id=378380&thanks=378380&ts=1...

Adding jam@ for OWNER review

[jam, 2014-05-28 19:01:35]

how will RPHI::isolated_guest_ be set? right now there are no public apis to set
this for non-test code.

[Fady Samuel, 2014-05-28 19:04:26]

On 2014/05/28 19:01:35, jam wrote:
> how will RPHI::isolated_guest_ be set? right now there are no public apis to
set
> this for non-test code.

https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/si...

If the site instance has a chrome-guest scheme, then
RenderProcessHost::IsIsolatedGuest is true.

[Fady Samuel, 2014-05-28 19:04:27]

On 2014/05/28 19:01:35, jam wrote:
> how will RPHI::isolated_guest_ be set? right now there are no public apis to
set
> this for non-test code.

https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/si...

If the site instance has a chrome-guest scheme, then
RenderProcessHost::IsIsolatedGuest is true.

[jam, 2014-05-28 19:45:21]

lgtm

[fsamuel, 2014-05-28 19:46:22]

The CQ bit was checked by fsamuel@google.com

[commit-bot: I haz the power, 2014-05-28 19:48:30]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/fsamuel@chromium.org/293093010/30001

[commit-bot: I haz the power, 2014-05-28 23:36:17]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/18...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-05-28 23:42:00]

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

[commit-bot: I haz the power, 2014-05-28 23:42:00]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)

[Fady Samuel, 2014-05-29 14:11:25]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2014-05-29 14:15:11]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/fsamuel@chromium.org/293093010/40001

[commit-bot: I haz the power, 2014-05-29 17:59:59]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-29 18:03:20]

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

[commit-bot: I haz the power, 2014-05-29 18:03:20]

Try jobs failed on following builders:
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)

[Fady Samuel, 2014-05-29 18:05:52]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2014-05-29 18:07:13]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/fsamuel@chromium.org/293093010/60001

[commit-bot: I haz the power, 2014-05-29 22:09:18]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/18...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[Fady Samuel, 2014-05-29 22:14:05]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2014-05-29 22:16:11]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/fsamuel@chromium.org/293093010/80001

[commit-bot: I haz the power, 2014-05-30 02:10:15]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-30 05:55:05]

Change committed as 273733