Consolidate isolated origin subframes into existing processes.

Consolidate isolated origin subframes into existing processes.

Previously, isolated origins subframes from unrelated tabs would
always get their own process.  This results in too many tabs for
scenarios like sign-in, where there's a sign-in subframe on every
Google tab.  To address this, set the process reuse policy to allow
isolated origin subframes to reuse existing processes for that origin.

BUG=713444
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2920473005
Cr-Commit-Position: refs/heads/master@{#476467}
Committed: https://chromium.googlesource.com/chromium/src/+/7b7555fc8a5b8eddc838c28595199eb663e2587a

[alexmos, 2017-06-01 18:09:22]

Description was changed from

==========
Consolidate isolated origin subframes into existing processes.

Previously, isolated origins subframes from unrelated tabs would
always get their own process.  This results in too many tabs for
scenarios like sign-in, where there's a sign-in subframe on every
Google tab.  To address this, set the process reuse policy to allow
isolated origin subframes to reuse existing processes for that origin.

BUG=713444
==========

to

==========
Consolidate isolated origin subframes into existing processes.

Previously, isolated origins subframes from unrelated tabs would
always get their own process.  This results in too many tabs for
scenarios like sign-in, where there's a sign-in subframe on every
Google tab.  To address this, set the process reuse policy to allow
isolated origin subframes to reuse existing processes for that origin.

BUG=713444
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[alexmos, 2017-06-01 18:09:27]

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

[commit-bot: I haz the power, 2017-06-01 18:10:06]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-01 19:26:10]

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

[commit-bot: I haz the power, 2017-06-01 19:26:11]

Dry run: This issue passed the CQ dry run.

[alexmos, 2017-06-01 20:47:57]

alexmos@chromium.org changed reviewers:
+ creis@chromium.org

[alexmos, 2017-06-01 20:47:58]

Charlie, can you please take a look?  

This doesn't fix the undesirable reuse on non-dedicated processes with
--renderer-process-limit=1, which I'm working on in a followup.  I think it
might make sense to fix that by making IsSuitableHost and
ShouldTryToUseExistingProcessHost aware of isolated origins and/or sites that
processes are dedicated to as outlined in https://crbug.com/513036.  We could
give all isolated origin SiteInstances a process reuse policy which will
guarantee this, though then the policy might have to be different for main
frames vs subframes, if we only want subframes to use the consolidation here. 
But we can discuss that separately in the followup CL, once I figure out some
more details.

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

https://codereview.chromium.org/2920473005/diff/1/content/browser/frame_host/...
content/browser/frame_host/render_frame_host_manager.cc:1273:
policy->IsIsolatedOrigin(url::Origin(new_instance_impl->GetSiteURL()))) {
We could consider doing this for all subframes that require a dedicated process,
not just isolated origins.  Do you think that's worth doing here or in a
followup?

[Charlie Reis, 2017-06-01 22:27:47]

LGTM!

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

https://codereview.chromium.org/2920473005/diff/1/content/browser/frame_host/...
content/browser/frame_host/render_frame_host_manager.cc:1273:
policy->IsIsolatedOrigin(url::Origin(new_instance_impl->GetSiteURL()))) {
On 2017/06/01 20:47:58, alexmos wrote:
> We could consider doing this for all subframes that require a dedicated
process,
> not just isolated origins.  Do you think that's worth doing here or in a
> followup?

I like that idea.  It would probably be safe to do here, but doing it in a
followup is probably worthwhile-- that makes it less likely that the isolated
origin case is reverted if the general case turns out to have issues.

[alexmos, 2017-06-01 22:52:41]

The CQ bit was checked by alexmos@chromium.org

[commit-bot: I haz the power, 2017-06-01 22:53:29]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-01 22:58:34]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1496357561827980, "parent_rev":
"3d5ca99fe1815a936278f4952f7e4497dc2bbaaa", "commit_rev":
"7b7555fc8a5b8eddc838c28595199eb663e2587a"}

[commit-bot: I haz the power, 2017-06-01 22:58:45]

Description was changed from

==========
Consolidate isolated origin subframes into existing processes.

Previously, isolated origins subframes from unrelated tabs would
always get their own process.  This results in too many tabs for
scenarios like sign-in, where there's a sign-in subframe on every
Google tab.  To address this, set the process reuse policy to allow
isolated origin subframes to reuse existing processes for that origin.

BUG=713444
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Consolidate isolated origin subframes into existing processes.

Previously, isolated origins subframes from unrelated tabs would
always get their own process.  This results in too many tabs for
scenarios like sign-in, where there's a sign-in subframe on every
Google tab.  To address this, set the process reuse policy to allow
isolated origin subframes to reuse existing processes for that origin.

BUG=713444
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2920473005
Cr-Commit-Position: refs/heads/master@{#476467}
Committed:
https://chromium.googlesource.com/chromium/src/+/7b7555fc8a5b8eddc838c2859519...
==========

[commit-bot: I haz the power, 2017-06-01 22:58:46]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/7b7555fc8a5b8eddc838c2859519...