Add new system url request context getter to the blimp engine.

Add new system url request context getter to the blimp engine.
This new class and function provides context for system type calls (e.g. metrics uploads) that is independent of user context.

BUG=601155
Committed: https://crrev.com/badd08c680342bb737a6f470002888f8b48edc36
Cr-Commit-Position: refs/heads/master@{#386185}

[Jess, 2016-04-06 19:59:48]

jessicag@chromium.org changed reviewers:
+ kmarshall@chromium.org, mmenke@chromium.org

[Jess, 2016-04-06 19:59:48]

jessicag@chromium.org changed required reviewers:
+ kmarshall@chromium.org

[Jess, 2016-04-06 19:59:49]

mmenke: Please review construction using URLRequestContextBuilder.

kmarshal: Please provide owners review.

[mmenke, 2016-04-06 20:44:56]

LGTM

https://codereview.chromium.org/1864163003/diff/1/blimp/engine/common/blimp_b...
File blimp/engine/common/blimp_browser_context.cc (right):

https://codereview.chromium.org/1864163003/diff/1/blimp/engine/common/blimp_b...
blimp/engine/common/blimp_browser_context.cc:101: system_context_getter_ = new
BlimpSystemURLRequestContextGetter();
if (!system_context_getter_)
  system_context_getter_ = new BlimpSystemURLRequestContextGetter();

URLRequestContexts are pretty heavy weight, so want to make sure we don't have
more than one hanging around at a time.

[Jess, 2016-04-06 21:00:52]

https://codereview.chromium.org/1864163003/diff/1/blimp/engine/common/blimp_b...
File blimp/engine/common/blimp_browser_context.cc (right):

https://codereview.chromium.org/1864163003/diff/1/blimp/engine/common/blimp_b...
blimp/engine/common/blimp_browser_context.cc:101: system_context_getter_ = new
BlimpSystemURLRequestContextGetter();
On 2016/04/06 20:44:56, mmenke wrote:
> if (!system_context_getter_)
>   system_context_getter_ = new BlimpSystemURLRequestContextGetter();
> 
> URLRequestContexts are pretty heavy weight, so want to make sure we don't have
> more than one hanging around at a time.

Done.

[Kevin M, 2016-04-08 01:09:08]

The CL subject line is too long. Could you shorten that line (<=72 chars) and
provide a bulleted summary of your changes underneath it? Thanks!

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/app/blimp_...
File blimp/engine/app/blimp_system_url_request_context_getter.cc (right):

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/app/blimp_...
blimp/engine/app/blimp_system_url_request_context_getter.cc:12: #include
"blimp/engine/app/blimp_network_delegate.h"
Is this needed?

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/app/blimp_...
blimp/engine/app/blimp_system_url_request_context_getter.cc:13: #include
"blimp/engine/common/blimp_content_client.h"
Is this needed?

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/app/blimp_...
blimp/engine/app/blimp_system_url_request_context_getter.cc:31: if
(!url_request_context_.get()) {
nit: get() isn't necessary for null checks

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/app/blimp_...
File blimp/engine/app/blimp_system_url_request_context_getter.h (right):

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/app/blimp_...
blimp/engine/app/blimp_system_url_request_context_getter.h:22: // The
URLRequestContextGetter for blimp system requests
Add clarifying comment for what constitutes a "system request"?

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/app/blimp_...
blimp/engine/app/blimp_system_url_request_context_getter.h:22: // The
URLRequestContextGetter for blimp system requests
nit: Capitalize Blimp

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/app/blimp_...
blimp/engine/app/blimp_system_url_request_context_getter.h:35:
scoped_ptr<net::URLRequestContext> url_request_context_;
Switch to std::unique_ptr & remove scoped_ptr include

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/app/blimp_...
File blimp/engine/app/blimp_url_request_context_getter.h (right):

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/app/blimp_...
blimp/engine/app/blimp_url_request_context_getter.h:30: // The
URLRequestContextGetter for blimp "user" requests.
Capitalize "Blimp"

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/common/bli...
File blimp/engine/common/blimp_browser_context.h (right):

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/common/bli...
blimp/engine/common/blimp_browser_context.h:63: net::URLRequestContextGetter*
GetSystemRequestContext();
GetSystemRequestContextGetter?

[Jess, 2016-04-08 17:17:29]

Description was changed from

==========
Add new system url request context getter to the blimp engine to provide context
for system type calls (e.g. metrics uploads).

BUG=601155
==========

to

==========
Add new system url request context getter to the blimp engine.
This new class and function provides context for system type calls (e.g. metrics
uploads) that is independent of user context.

BUG=601155
==========

[Jess, 2016-04-08 19:39:02]

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/app/blimp_...
File blimp/engine/app/blimp_system_url_request_context_getter.cc (right):

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/app/blimp_...
blimp/engine/app/blimp_system_url_request_context_getter.cc:12: #include
"blimp/engine/app/blimp_network_delegate.h"
On 2016/04/08 01:09:07, Kevin M wrote:
> Is this needed?

Nope. Removed.

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/app/blimp_...
blimp/engine/app/blimp_system_url_request_context_getter.cc:13: #include
"blimp/engine/common/blimp_content_client.h"
On 2016/04/08 01:09:08, Kevin M wrote:
> Is this needed?

Same here. Are they any includes checking tools I should be running?

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/app/blimp_...
blimp/engine/app/blimp_system_url_request_context_getter.cc:31: if
(!url_request_context_.get()) {
On 2016/04/08 01:09:07, Kevin M wrote:
> nit: get() isn't necessary for null checks

Done.

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/app/blimp_...
File blimp/engine/app/blimp_system_url_request_context_getter.h (right):

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/app/blimp_...
blimp/engine/app/blimp_system_url_request_context_getter.h:22: // The
URLRequestContextGetter for blimp system requests
On 2016/04/08 01:09:08, Kevin M wrote:
> nit: Capitalize Blimp

Done.

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/app/blimp_...
blimp/engine/app/blimp_system_url_request_context_getter.h:22: // The
URLRequestContextGetter for blimp system requests
On 2016/04/08 01:09:08, Kevin M wrote:
> Add clarifying comment for what constitutes a "system request"?

PTAL.

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/app/blimp_...
blimp/engine/app/blimp_system_url_request_context_getter.h:35:
scoped_ptr<net::URLRequestContext> url_request_context_;
On 2016/04/08 01:09:08, Kevin M wrote:
> Switch to std::unique_ptr & remove scoped_ptr include

Done.

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/app/blimp_...
File blimp/engine/app/blimp_url_request_context_getter.h (right):

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/app/blimp_...
blimp/engine/app/blimp_url_request_context_getter.h:30: // The
URLRequestContextGetter for blimp "user" requests.
On 2016/04/08 01:09:08, Kevin M wrote:
> Capitalize "Blimp"

Done.

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/common/bli...
File blimp/engine/common/blimp_browser_context.h (right):

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/common/bli...
blimp/engine/common/blimp_browser_context.h:63: net::URLRequestContextGetter*
GetSystemRequestContext();
On 2016/04/08 01:09:08, Kevin M wrote:
> GetSystemRequestContextGetter?

Sounds good.  I went with this naming because of the GetRequestContext() naming
above, but I agree this is less confusing.

Done.

[Kevin M, 2016-04-08 19:44:28]

lgtm

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/app/blimp_...
File blimp/engine/app/blimp_system_url_request_context_getter.cc (right):

https://codereview.chromium.org/1864163003/diff/20001/blimp/engine/app/blimp_...
blimp/engine/app/blimp_system_url_request_context_getter.cc:13: #include
"blimp/engine/common/blimp_content_client.h"
On 2016/04/08 19:39:02, Jess wrote:
> On 2016/04/08 01:09:08, Kevin M wrote:
> > Is this needed?
> 
> Same here. Are they any includes checking tools I should be running?

Last I checked, the Chromium IWYU tools were still kind of busted.

[Jess, 2016-04-08 20:10:01]

The CQ bit was checked by jessicag@chromium.org

[Jess, 2016-04-08 20:10:01]

The patchset sent to the CQ was uploaded after l-g-t-m from mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/1864163003/#ps40001
(title: "Comment updates and syncing ptr migration. Includes rebase.")

[commit-bot: I haz the power, 2016-04-08 20:10:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1864163003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1864163003/40001

[commit-bot: I haz the power, 2016-04-08 20:16:15]

Description was changed from

==========
Add new system url request context getter to the blimp engine.
This new class and function provides context for system type calls (e.g. metrics
uploads) that is independent of user context.

BUG=601155
==========

to

==========
Add new system url request context getter to the blimp engine.
This new class and function provides context for system type calls (e.g. metrics
uploads) that is independent of user context.

BUG=601155
==========

[commit-bot: I haz the power, 2016-04-08 20:16:16]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-08 20:18:32]

Description was changed from

==========
Add new system url request context getter to the blimp engine.
This new class and function provides context for system type calls (e.g. metrics
uploads) that is independent of user context.

BUG=601155
==========

to

==========
Add new system url request context getter to the blimp engine.
This new class and function provides context for system type calls (e.g. metrics
uploads) that is independent of user context.

BUG=601155

Committed: https://crrev.com/badd08c680342bb737a6f470002888f8b48edc36
Cr-Commit-Position: refs/heads/master@{#386185}
==========

[commit-bot: I haz the power, 2016-04-08 20:18:33]

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