Don't ensure, in single-process mode, that WebUI render views are the only ones in the process, as …

Don't ensure, in single-process mode, that WebUI render views are the only ones in the process, as it makes no sense
 - we have single process only and no process-separation is expected.

This allows testing navigation between WebUI and standard pages
in single-process mode.

BUG=

Committed: https://crrev.com/bf50b9cc28f7a7749183a6c241c4836d5a66c54c
Cr-Commit-Position: refs/heads/master@{#297683}

[Krzysztof Olczyk, 2014-05-26 09:05:37]

Hi nasko,

could you please take a look at this small change
which permits navigation out from webUI
without CHECK() fail for single-process mode
where process-separation assurances do not apply?

[nasko, 2014-05-27 14:06:09]

Just one note on the change. Could you also add a test so we don't regress this
in the future?
Thanks

https://codereview.chromium.org/298133002/diff/1/content/browser/frame_host/r...
File content/browser/frame_host/render_frame_host_manager.cc (right):

https://codereview.chromium.org/298133002/diff/1/content/browser/frame_host/r...
content/browser/frame_host/render_frame_host_manager.cc:1014:
CommandLine::ForCurrentProcess()->HasSwitch(switches::kSingleProcess);
It will be better to use RenderProcessHost::run_renderer_in_process() instead of
checking the switch manually.

[Krzysztof Olczyk, 2014-05-28 09:12:49]

Hi nasko,

I've uploaded a unit test to cover this as you requested.

https://codereview.chromium.org/298133002/diff/1/content/browser/frame_host/r...
File content/browser/frame_host/render_frame_host_manager.cc (right):

https://codereview.chromium.org/298133002/diff/1/content/browser/frame_host/r...
content/browser/frame_host/render_frame_host_manager.cc:1014:
CommandLine::ForCurrentProcess()->HasSwitch(switches::kSingleProcess);
On 2014/05/27 14:06:09, nasko wrote:
> It will be better to use RenderProcessHost::run_renderer_in_process() instead
of
> checking the switch manually.

Done.

[nasko, 2014-05-28 17:02:04]

https://codereview.chromium.org/298133002/diff/20001/content/browser/frame_ho...
File content/browser/frame_host/render_frame_host_manager_unittest.cc (right):

https://codereview.chromium.org/298133002/diff/20001/content/browser/frame_ho...
content/browser/frame_host/render_frame_host_manager_unittest.cc:1096:
CanNavigateBetweenNormalPagesAndWebUIInSingleProcess) {
This test doesn't seem to work as expected. The renderer doesn't get WebUI
bindings, as the code that grants them checks for the actual "--single-process"
flag :(.

https://codereview.chromium.org/298133002/diff/20001/content/browser/frame_ho...
content/browser/frame_host/render_frame_host_manager_unittest.cc:1106:
web_contents())->GetRenderManagerForTesting();
nit: wrong indent

[nasko, 2014-09-25 16:36:39]

On 2014/05/28 17:02:04, nasko wrote:
>
https://codereview.chromium.org/298133002/diff/20001/content/browser/frame_ho...
> File content/browser/frame_host/render_frame_host_manager_unittest.cc (right):
> 
>
https://codereview.chromium.org/298133002/diff/20001/content/browser/frame_ho...
> content/browser/frame_host/render_frame_host_manager_unittest.cc:1096:
> CanNavigateBetweenNormalPagesAndWebUIInSingleProcess) {
> This test doesn't seem to work as expected. The renderer doesn't get WebUI
> bindings, as the code that grants them checks for the actual
"--single-process"
> flag :(.
> 
>
https://codereview.chromium.org/298133002/diff/20001/content/browser/frame_ho...
> content/browser/frame_host/render_frame_host_manager_unittest.cc:1106:
> web_contents())->GetRenderManagerForTesting();
> nit: wrong indent

Hey Krzysztof,
Are you still planning on working on this CL? If not, can you close it?
Thanks!
Nasko

[Krzysztof Olczyk, 2014-09-26 07:38:34]

On 2014/09/25 at 16:36:39, nasko wrote:
> On 2014/05/28 17:02:04, nasko wrote:
> >
https://codereview.chromium.org/298133002/diff/20001/content/browser/frame_ho...
> > File content/browser/frame_host/render_frame_host_manager_unittest.cc
(right):
> > 
> >
https://codereview.chromium.org/298133002/diff/20001/content/browser/frame_ho...
> > content/browser/frame_host/render_frame_host_manager_unittest.cc:1096:
> > CanNavigateBetweenNormalPagesAndWebUIInSingleProcess) {
> > This test doesn't seem to work as expected. The renderer doesn't get WebUI
> > bindings, as the code that grants them checks for the actual
"--single-process"
> > flag :(.
> > 
> >
https://codereview.chromium.org/298133002/diff/20001/content/browser/frame_ho...
> > content/browser/frame_host/render_frame_host_manager_unittest.cc:1106:
> > web_contents())->GetRenderManagerForTesting();
> > nit: wrong indent
> 
> Hey Krzysztof,
> Are you still planning on working on this CL? If not, can you close it?
> Thanks!
> Nasko

Hi Nasko,

thanks for recalling me about this CL. 
I've just updated it and made sure the test fails before the change in
implementation and passes after.
Hope it's fine for landing now.

[nasko, 2014-09-29 22:21:48]

Just a couple of comments and it should be good to go.

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

https://codereview.chromium.org/298133002/diff/40001/content/browser/frame_ho...
content/browser/frame_host/render_frame_host_manager.cc:1227: bool
is_single_process = RenderProcessHost::run_renderer_in_process();
nit: why create a local variable when you can just call the method in the if
statement?

https://codereview.chromium.org/298133002/diff/40001/content/browser/frame_ho...
File content/browser/frame_host/render_frame_host_manager_unittest.cc (right):

https://codereview.chromium.org/298133002/diff/40001/content/browser/frame_ho...
content/browser/frame_host/render_frame_host_manager_unittest.cc:1005: // on
RenderProcessHost
sidenote: It is best to have one source of truth in the codebase. Can you file a
bug to move all uses to one or the other? Feel free to cc me on the bug.

https://codereview.chromium.org/298133002/diff/40001/content/browser/frame_ho...
content/browser/frame_host/render_frame_host_manager_unittest.cc:1011:
static_cast<TestWebContents*>(
nit: this line should be able to fit on the previous, no?

[Krzysztof Olczyk, 2014-10-01 10:17:47]

Hi Nasko,

I've applied the style changes you suggested and posted a crbug.

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

https://codereview.chromium.org/298133002/diff/40001/content/browser/frame_ho...
content/browser/frame_host/render_frame_host_manager.cc:1227: bool
is_single_process = RenderProcessHost::run_renderer_in_process();
On 2014/09/29 at 22:21:48, nasko wrote:
> nit: why create a local variable when you can just call the method in the if
statement?

Done

https://codereview.chromium.org/298133002/diff/40001/content/browser/frame_ho...
File content/browser/frame_host/render_frame_host_manager_unittest.cc (right):

https://codereview.chromium.org/298133002/diff/40001/content/browser/frame_ho...
content/browser/frame_host/render_frame_host_manager_unittest.cc:1005: // on
RenderProcessHost
On 2014/09/29 at 22:21:48, nasko wrote:
> sidenote: It is best to have one source of truth in the codebase. Can you file
a bug to move all uses to one or the other? Feel free to cc me on the bug.

Done: https://code.google.com/p/chromium/issues/detail?id=419293

https://codereview.chromium.org/298133002/diff/40001/content/browser/frame_ho...
content/browser/frame_host/render_frame_host_manager_unittest.cc:1011:
static_cast<TestWebContents*>(
On 2014/09/29 at 22:21:48, nasko wrote:
> nit: this line should be able to fit on the previous, no?

Done

[nasko, 2014-10-01 13:57:26]

LGTM

[Krzysztof Olczyk, 2014-10-01 13:58:58]

The CQ bit was checked by kolczyk@opera.com

[commit-bot: I haz the power, 2014-10-01 13:59:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/298133002/60001

[commit-bot: I haz the power, 2014-10-01 14:03:21]

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

[commit-bot: I haz the power, 2014-10-01 14:03:23]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/67860)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_chromium_gn_compile_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Krzysztof Olczyk, 2014-10-01 15:52:45]

The CQ bit was checked by kolczyk@opera.com

[commit-bot: I haz the power, 2014-10-01 15:54:10]

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

[commit-bot: I haz the power, 2014-10-01 18:55:40]

Committed patchset #5 (id:80001) as b91e8bee3cc47da325083b95969c81b533906643

[commit-bot: I haz the power, 2014-10-01 18:57:19]

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

[nasko, 2014-10-01 20:49:10]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/618853004/ by nasko@chromium.org.

The reason for reverting is: This patch broke the memory.fyi bots, as they run
tests in single-process mode.

http://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20Tests%20%28v....

[Krzysztof Olczyk, 2014-10-10 05:50:29]

On 2014/10/01 at 20:49:10, nasko wrote:
> A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/618853004/ by nasko@chromium.org.
> 
> The reason for reverting is: This patch broke the memory.fyi bots, as they run
tests in single-process mode.
> 
>
http://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20Tests%20%28v....


Hi Nasko,

could you please help me understand how this CL has broke the bots?

[nasko, 2014-10-14 22:55:04]

I haven't looked in detail, but it looked like it can very well cause this
so it was speculatively reverted. I'd suggest running the reported failed
tests with the patch and seeing what causes the failure. Since this is a
memory bot, it likely requires ASAN build or something similar.

On Thu, Oct 9, 2014 at 10:50 PM, <kolczyk@opera.com> wrote:

> On 2014/10/01 at 20:49:10, nasko wrote:
>
>> A revert of this CL (patchset #5 id:80001) has been created in
>>
> https://codereview.chromium.org/618853004/ by nasko@chromium.org.
>
>  The reason for reverting is: This patch broke the memory.fyi bots, as
>> they run
>>
> tests in single-process mode.
>
>
> http://build.chromium.org/p/chromium.memory.fyi/builders/
> Linux%20Tests%20%28valgrind%29%281%29/builds/36766.
>
>
> Hi Nasko,
>
> could you please help me understand how this CL has broke the bots?
>
>
> https://codereview.chromium.org/298133002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Krzysztof Olczyk, 2015-06-01 06:49:39]

The CQ bit was checked by kolczyk@opera.com

[Sergiy Byelozyorov, 2015-06-05 10:46:49]

The CQ bit was unchecked by sergiyb@chromium.org

[Sergiy Byelozyorov, 2015-06-05 10:47:58]

On 2014/10/14 22:55:04, nasko wrote:
> I haven't looked in detail, but it looked like it can very well cause this
> so it was speculatively reverted. I'd suggest running the reported failed
> tests with the patch and seeing what causes the failure. Since this is a
> memory bot, it likely requires ASAN build or something similar.
> 
> On Thu, Oct 9, 2014 at 10:50 PM, <mailto:kolczyk@opera.com> wrote:
> 
> > On 2014/10/01 at 20:49:10, nasko wrote:
> >
> >> A revert of this CL (patchset #5 id:80001) has been created in
> >>
> > https://codereview.chromium.org/618853004/ by mailto:nasko@chromium.org.
> >
> >  The reason for reverting is: This patch broke the memory.fyi bots, as
> >> they run
> >>
> > tests in single-process mode.
> >
> >
> > http://build.chromium.org/p/chromium.memory.fyi/builders/
> > Linux%20Tests%20%28valgrind%29%281%29/builds/36766.
> >
> >
> > Hi Nasko,
> >
> > could you please help me understand how this CL has broke the bots?
> >
> >
> > https://codereview.chromium.org/298133002/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

This CL is too old to be processed by CQ (missing project field that we've
introduced a couple of months ago). Please re-upload it to codereview site and
land again:

git checkout branch_for_this_cl
git cl issue 0
git cl upload

[Krzysztof Olczyk, 2015-07-20 11:27:27]

On 2015/06/05 at 10:47:58, sergiyb wrote:
> On 2014/10/14 22:55:04, nasko wrote:
> > I haven't looked in detail, but it looked like it can very well cause this
> > so it was speculatively reverted. I'd suggest running the reported failed
> > tests with the patch and seeing what causes the failure. Since this is a
> > memory bot, it likely requires ASAN build or something similar.
> > 
> > On Thu, Oct 9, 2014 at 10:50 PM, <mailto:kolczyk@opera.com> wrote:
> > 
> > > On 2014/10/01 at 20:49:10, nasko wrote:
> > >
> > >> A revert of this CL (patchset #5 id:80001) has been created in
> > >>
> > > https://codereview.chromium.org/618853004/ by mailto:nasko@chromium.org.
> > >
> > >  The reason for reverting is: This patch broke the memory.fyi bots, as
> > >> they run
> > >>
> > > tests in single-process mode.
> > >
> > >
> > > http://build.chromium.org/p/chromium.memory.fyi/builders/
> > > Linux%20Tests%20%28valgrind%29%281%29/builds/36766.
> > >
> > >
> > > Hi Nasko,
> > >
> > > could you please help me understand how this CL has broke the bots?
> > >
> > >
> > > https://codereview.chromium.org/298133002/
> > >
> > 
> > To unsubscribe from this group and stop receiving emails from it, send an
email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.
> 
> This CL is too old to be processed by CQ (missing project field that we've
introduced a couple of months ago). Please re-upload it to codereview site and
land again:
> 
> git checkout branch_for_this_cl
> git cl issue 0
> git cl upload

Thanks for information. Resubmitted in
https://codereview.chromium.org/1229133005.