Cache the NetworkService interface pointer in the browser.

Cache the NetworkService interface pointer in the browser.

The NetworkService interface is meant to be used as a signleton, so rather than have it contain a BindingSet just cache the pointer in the browser.

This fixes
DOMStorageBrowserTest.SanityCheckIncognito
DatabaseTest.OffTheRecordCannotReadRegularDatabase
DatabaseTest.OffTheRecordDBPersistentAcrossWindows
when run in debug or release-with-asserts.

BUG=729848
Review-Url: https://codereview.chromium.org/2958173002
Cr-Commit-Position: refs/heads/master@{#482834}
Committed: https://chromium.googlesource.com/chromium/src/+/b98b8c85d3bb5969c3d3c17851acc40e032e2dca

[jam, 2017-06-27 22:28:29]

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

[commit-bot: I haz the power, 2017-06-27 22:28:52]

Dry run: CQ is trying da patch.

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

[jam, 2017-06-27 22:44:35]

jam@chromium.org changed reviewers:
+ rockot@chromium.org

[Ken Rockot(use gerrit already), 2017-06-27 22:52:39]

lgtm

https://codereview.chromium.org/2958173002/diff/1/content/browser/storage_par...
File content/browser/storage_partition_impl.cc (right):

https://codereview.chromium.org/2958173002/diff/1/content/browser/storage_par...
content/browser/storage_partition_impl.cc:536: static
base::LazyInstance<mojom::NetworkServicePtr>::Leaky
Is there any reason to use LazyInstance now? Why not just static
mojom::NetworkServicePtr*?

https://codereview.chromium.org/2958173002/diff/1/content/browser/storage_par...
content/browser/storage_partition_impl.cc:536: static
base::LazyInstance<mojom::NetworkServicePtr>::Leaky
I'm pretty sure the fact that we now have thread-safe statics means LazyInstance
is redundant in this case and you should just write

  static mojom::NetworkServicePtr* g_network_service = nullptr;

[Ken Rockot(use gerrit already), 2017-06-27 22:53:17]

On 2017/06/27 at 22:52:39, Ken Rockot(use gerrit already) wrote:
> lgtm
> 
>
https://codereview.chromium.org/2958173002/diff/1/content/browser/storage_par...
> File content/browser/storage_partition_impl.cc (right):
> 
>
https://codereview.chromium.org/2958173002/diff/1/content/browser/storage_par...
> content/browser/storage_partition_impl.cc:536: static
base::LazyInstance<mojom::NetworkServicePtr>::Leaky
> Is there any reason to use LazyInstance now? Why not just static
mojom::NetworkServicePtr*?
> 
>
https://codereview.chromium.org/2958173002/diff/1/content/browser/storage_par...
> content/browser/storage_partition_impl.cc:536: static
base::LazyInstance<mojom::NetworkServicePtr>::Leaky
> I'm pretty sure the fact that we now have thread-safe statics means
LazyInstance is redundant in this case and you should just write
> 
>   static mojom::NetworkServicePtr* g_network_service = nullptr;

(actually the thread-safe bit is irrelevant for this usage but the point still
stands that LazyInstance is redundant AFAICT)

[jam, 2017-06-27 23:42:19]

https://codereview.chromium.org/2958173002/diff/1/content/browser/storage_par...
File content/browser/storage_partition_impl.cc (right):

https://codereview.chromium.org/2958173002/diff/1/content/browser/storage_par...
content/browser/storage_partition_impl.cc:536: static
base::LazyInstance<mojom::NetworkServicePtr>::Leaky
On 2017/06/27 22:52:39, Ken Rockot(use gerrit already) wrote:
> Is there any reason to use LazyInstance now? Why not just static
> mojom::NetworkServicePtr*?

Done.

[jam, 2017-06-27 23:42:31]

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

[commit-bot: I haz the power, 2017-06-27 23:42:40]

Dry run: CQ is trying da patch.

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

[jam, 2017-06-27 23:44:33]

The CQ bit was checked by jam@chromium.org

[jam, 2017-06-27 23:44:33]

The patchset sent to the CQ was uploaded after l-g-t-m from rockot@chromium.org
Link to the patchset: https://codereview.chromium.org/2958173002/#ps20001
(title: "review comments")

[commit-bot: I haz the power, 2017-06-27 23:44:53]

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-28 01:08:45]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1498607073171810,
"parent_rev": "e6f83f0868cb11d23bb73d5a8763e72445f77558", "commit_rev":
"b98b8c85d3bb5969c3d3c17851acc40e032e2dca"}

[commit-bot: I haz the power, 2017-06-28 01:09:06]

Description was changed from

==========
Cache the NetworkService interface pointer in the browser.

The NetworkService interface is meant to be used as a signleton, so rather than
have it contain a BindingSet just cache the pointer in the browser.

This fixes
DOMStorageBrowserTest.SanityCheckIncognito
DatabaseTest.OffTheRecordCannotReadRegularDatabase
DatabaseTest.OffTheRecordDBPersistentAcrossWindows
when run in debug or release-with-asserts.

BUG=729848
==========

to

==========
Cache the NetworkService interface pointer in the browser.

The NetworkService interface is meant to be used as a signleton, so rather than
have it contain a BindingSet just cache the pointer in the browser.

This fixes
DOMStorageBrowserTest.SanityCheckIncognito
DatabaseTest.OffTheRecordCannotReadRegularDatabase
DatabaseTest.OffTheRecordDBPersistentAcrossWindows
when run in debug or release-with-asserts.

BUG=729848

Review-Url: https://codereview.chromium.org/2958173002
Cr-Commit-Position: refs/heads/master@{#482834}
Committed:
https://chromium.googlesource.com/chromium/src/+/b98b8c85d3bb5969c3d3c17851ac...
==========

[commit-bot: I haz the power, 2017-06-28 01:09:08]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/b98b8c85d3bb5969c3d3c17851ac...