headless: Introduce a browser context

headless: Introduce a browser context

A headless browser context can be used to isolate different WebContents
from one another. Each context has a separate cache, cookies, local
storage and other profile-related data.

This patch also adds a way to create a HeadlessWebContents using a
specific browser context instead of the default one. This can be used to
isolate tabs from one another.

Design doc: https://docs.google.com/document/d/1XAKvrxtSEoe65vNghSWC5S3kJ--z2Zpt2UWW1Fi8GiM/edit?pref=2&pli=1#heading=h.7r3ycehq63g2

BUG=546953, 617931
Committed: https://crrev.com/25c2c0123425d8ab3c9c02998d02d7accd1c42e2
Cr-Commit-Position: refs/heads/master@{#398853}

[Sami, 2016-06-08 16:11:00]

Description was changed from

==========
headless: Introduce a browser context

A headless browser context can be used to isolate different WebContents
from one another. Each context has a separate cache, cookies, local
storage and other profile-related data.

This patch also adds a way to create a HeadlessWebContents using a
specific browser context instead of the default one. This can be used to
isolate tabs from one another.

BUG=546953
==========

to

==========
headless: Introduce a browser context

A headless browser context can be used to isolate different WebContents
from one another. Each context has a separate cache, cookies, local
storage and other profile-related data.

This patch also adds a way to create a HeadlessWebContents using a
specific browser context instead of the default one. This can be used to
isolate tabs from one another.

Design doc:
https://docs.google.com/document/d/1XAKvrxtSEoe65vNghSWC5S3kJ--z2Zpt2UWW1Fi8G...

BUG=546953
==========

[Sami, 2016-06-08 16:11:01]

skyostil@chromium.org changed reviewers:
+ alexclarke@chromium.org, altimin@chromium.org, eseckler@chromium.org

[alex clarke (OOO till 29th), 2016-06-09 09:15:08]

https://codereview.chromium.org/2043603004/diff/80001/headless/lib/browser/he...
File headless/lib/browser/headless_browser_impl.cc (right):

https://codereview.chromium.org/2043603004/diff/80001/headless/lib/browser/he...
headless/lib/browser/headless_browser_impl.cc:52: browser_context_impl =
reinterpret_cast<HeadlessBrowserContextImpl*>(
Nit: Elsewhere we've used the style
HeadlessBrowserContextImpl::From(builder->browser_context_) consider using that
here.

https://codereview.chromium.org/2043603004/diff/80001/headless/lib/browser/he...
headless/lib/browser/headless_browser_impl.cc:59:
HeadlessWebContentsImpl::Create(browser_context_impl,
I wonder if it would be cleaner to pass the builder into the constructor,
particularly as the number of parameters grows.

You'd need to change the builder's constructor to initially set
browser_main_parts_->default_browser_context() as the default for
builder->browser_context_

https://codereview.chromium.org/2043603004/diff/80001/headless/lib/headless_b...
File headless/lib/headless_browser_browsertest.cc (right):

https://codereview.chromium.org/2043603004/diff/80001/headless/lib/headless_b...
headless/lib/headless_browser_browsertest.cc:264:
isolated_devtools_client_(HeadlessDevToolsClient::Create()) {
Can we call these web_contents2_ & devtools_client2_ instead? 

Currently it feels like these are isolated but the other isn't - which is a
misrepresentation of what's going on :)

https://codereview.chromium.org/2043603004/diff/80001/headless/public/headles...
File headless/public/headless_browser_context.h (right):

https://codereview.chromium.org/2043603004/diff/80001/headless/public/headles...
headless/public/headless_browser_context.h:20: 
Lets link the tracking bug 617931 here.

[Sami, 2016-06-09 10:55:49]

Description was changed from

==========
headless: Introduce a browser context

A headless browser context can be used to isolate different WebContents
from one another. Each context has a separate cache, cookies, local
storage and other profile-related data.

This patch also adds a way to create a HeadlessWebContents using a
specific browser context instead of the default one. This can be used to
isolate tabs from one another.

Design doc:
https://docs.google.com/document/d/1XAKvrxtSEoe65vNghSWC5S3kJ--z2Zpt2UWW1Fi8G...

BUG=546953
==========

to

==========
headless: Introduce a browser context

A headless browser context can be used to isolate different WebContents
from one another. Each context has a separate cache, cookies, local
storage and other profile-related data.

This patch also adds a way to create a HeadlessWebContents using a
specific browser context instead of the default one. This can be used to
isolate tabs from one another.

Design doc:
https://docs.google.com/document/d/1XAKvrxtSEoe65vNghSWC5S3kJ--z2Zpt2UWW1Fi8G...

BUG=546953,617931
==========

[Sami, 2016-06-09 11:14:39]

Thanks for the review -- all addressed.

https://codereview.chromium.org/2043603004/diff/80001/headless/lib/browser/he...
File headless/lib/browser/headless_browser_impl.cc (right):

https://codereview.chromium.org/2043603004/diff/80001/headless/lib/browser/he...
headless/lib/browser/headless_browser_impl.cc:52: browser_context_impl =
reinterpret_cast<HeadlessBrowserContextImpl*>(
On 2016/06/09 09:15:08, alex clarke wrote:
> Nit: Elsewhere we've used the style
> HeadlessBrowserContextImpl::From(builder->browser_context_) consider using
that
> here.

Thanks for the reminder, done.

https://codereview.chromium.org/2043603004/diff/80001/headless/lib/browser/he...
headless/lib/browser/headless_browser_impl.cc:59:
HeadlessWebContentsImpl::Create(browser_context_impl,
On 2016/06/09 09:15:08, alex clarke wrote:
> I wonder if it would be cleaner to pass the builder into the constructor,
> particularly as the number of parameters grows.
> 
> You'd need to change the builder's constructor to initially set
> browser_main_parts_->default_browser_context() as the default for
> builder->browser_context_

Yeah, I think that will be more maintainable in the long term. Done.

https://codereview.chromium.org/2043603004/diff/80001/headless/lib/headless_b...
File headless/lib/headless_browser_browsertest.cc (right):

https://codereview.chromium.org/2043603004/diff/80001/headless/lib/headless_b...
headless/lib/headless_browser_browsertest.cc:264:
isolated_devtools_client_(HeadlessDevToolsClient::Create()) {
On 2016/06/09 09:15:08, alex clarke wrote:
> Can we call these web_contents2_ & devtools_client2_ instead? 
> 
> Currently it feels like these are isolated but the other isn't - which is a
> misrepresentation of what's going on :)

Agreed, that matches the description above too.

https://codereview.chromium.org/2043603004/diff/80001/headless/public/headles...
File headless/public/headless_browser_context.h (right):

https://codereview.chromium.org/2043603004/diff/80001/headless/public/headles...
headless/public/headless_browser_context.h:20: 
On 2016/06/09 09:15:08, alex clarke wrote:
> Lets link the tracking bug 617931 here.

Done.

[alex clarke (OOO till 29th), 2016-06-09 11:26:08]

LGTM modulo two nits.

https://codereview.chromium.org/2043603004/diff/100001/headless/public/headle...
File headless/public/headless_web_contents.h (right):

https://codereview.chromium.org/2043603004/diff/100001/headless/public/headle...
headless/public/headless_web_contents.h:90: Builder(HeadlessBrowserImpl*
browser);
nit: explicit

https://codereview.chromium.org/2043603004/diff/100001/headless/public/headle...
headless/public/headless_web_contents.h:95: HeadlessBrowserContext*
browser_context_ = nullptr;
Nit: we don't need '= nullptr' anymore?

[Sami, 2016-06-09 11:33:17]

https://codereview.chromium.org/2043603004/diff/100001/headless/public/headle...
File headless/public/headless_web_contents.h (right):

https://codereview.chromium.org/2043603004/diff/100001/headless/public/headle...
headless/public/headless_web_contents.h:90: Builder(HeadlessBrowserImpl*
browser);
On 2016/06/09 11:26:08, alex clarke wrote:
> nit: explicit

Done.

https://codereview.chromium.org/2043603004/diff/100001/headless/public/headle...
headless/public/headless_web_contents.h:95: HeadlessBrowserContext*
browser_context_ = nullptr;
On 2016/06/09 11:26:07, alex clarke wrote:
> Nit: we don't need '= nullptr' anymore?

Done.

[Sami, 2016-06-09 11:33:23]

The CQ bit was checked by skyostil@chromium.org

[Sami, 2016-06-09 11:33:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
alexclarke@chromium.org
Link to the patchset: https://codereview.chromium.org/2043603004/#ps120001
(title: "Address nits")

[commit-bot: I haz the power, 2016-06-09 11:33:33]

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

[commit-bot: I haz the power, 2016-06-09 12:54:23]

Description was changed from

==========
headless: Introduce a browser context

A headless browser context can be used to isolate different WebContents
from one another. Each context has a separate cache, cookies, local
storage and other profile-related data.

This patch also adds a way to create a HeadlessWebContents using a
specific browser context instead of the default one. This can be used to
isolate tabs from one another.

Design doc:
https://docs.google.com/document/d/1XAKvrxtSEoe65vNghSWC5S3kJ--z2Zpt2UWW1Fi8G...

BUG=546953,617931
==========

to

==========
headless: Introduce a browser context

A headless browser context can be used to isolate different WebContents
from one another. Each context has a separate cache, cookies, local
storage and other profile-related data.

This patch also adds a way to create a HeadlessWebContents using a
specific browser context instead of the default one. This can be used to
isolate tabs from one another.

Design doc:
https://docs.google.com/document/d/1XAKvrxtSEoe65vNghSWC5S3kJ--z2Zpt2UWW1Fi8G...

BUG=546953,617931
==========

[commit-bot: I haz the power, 2016-06-09 12:54:24]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-06-09 12:55:58]

Description was changed from

==========
headless: Introduce a browser context

A headless browser context can be used to isolate different WebContents
from one another. Each context has a separate cache, cookies, local
storage and other profile-related data.

This patch also adds a way to create a HeadlessWebContents using a
specific browser context instead of the default one. This can be used to
isolate tabs from one another.

Design doc:
https://docs.google.com/document/d/1XAKvrxtSEoe65vNghSWC5S3kJ--z2Zpt2UWW1Fi8G...

BUG=546953,617931
==========

to

==========
headless: Introduce a browser context

A headless browser context can be used to isolate different WebContents
from one another. Each context has a separate cache, cookies, local
storage and other profile-related data.

This patch also adds a way to create a HeadlessWebContents using a
specific browser context instead of the default one. This can be used to
isolate tabs from one another.

Design doc:
https://docs.google.com/document/d/1XAKvrxtSEoe65vNghSWC5S3kJ--z2Zpt2UWW1Fi8G...

BUG=546953,617931

Committed: https://crrev.com/25c2c0123425d8ab3c9c02998d02d7accd1c42e2
Cr-Commit-Position: refs/heads/master@{#398853}
==========

[commit-bot: I haz the power, 2016-06-09 12:55:59]

Patchset 7 (id:??) landed as
https://crrev.com/25c2c0123425d8ab3c9c02998d02d7accd1c42e2
Cr-Commit-Position: refs/heads/master@{#398853}