gpu: Move ShaderDiskCache into //gpu/ipc/host component.

gpu: Move ShaderDiskCache into //gpu/ipc/host component.

BUG=643746, 604369
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/4a4522e8d9a3ae704b7a17ebb412d2a94aa820c2
Committed: https://crrev.com/9095a6d305b5ce5d2e7a781c3e3ebd628410913e
Cr-Original-Commit-Position: refs/heads/master@{#437771}
Cr-Commit-Position: refs/heads/master@{#438165}

[sadrul, 2016-12-08 01:45:12]

Description was changed from

==========
gpu: Move ShaderDiskCache into //gpu/ipc/host component.

BUG=643746
==========

to

==========
gpu: Move ShaderDiskCache into //gpu/ipc/host component.

BUG=643746
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[sadrul, 2016-12-08 01:45:16]

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

[commit-bot: I haz the power, 2016-12-08 01:45:52]

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, 2016-12-08 01:51:23]

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

[commit-bot: I haz the power, 2016-12-08 01:51:24]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sadrul, 2016-12-08 01:54:38]

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

[commit-bot: I haz the power, 2016-12-08 01:55:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sadrul, 2016-12-08 02:00:13]

sadrul@chromium.org changed reviewers:
+ piman@chromium.org

[sadrul, 2016-12-08 02:00:14]

Now that we have the gpu-host component, the shader-cache can move into that.
PTAL

[sadrul, 2016-12-08 02:09:43]

Description was changed from

==========
gpu: Move ShaderDiskCache into //gpu/ipc/host component.

BUG=643746
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
gpu: Move ShaderDiskCache into //gpu/ipc/host component.

BUG=643746, 604369
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[commit-bot: I haz the power, 2016-12-08 02:48:24]

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

[commit-bot: I haz the power, 2016-12-08 02:48:25]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[sadrul, 2016-12-08 06:07:17]

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

[commit-bot: I haz the power, 2016-12-08 06:07:36]

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, 2016-12-08 07:25:49]

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

[commit-bot: I haz the power, 2016-12-08 07:25:49]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[piman, 2016-12-08 20:18:16]

LGTM, thanks!

[sadrul, 2016-12-08 20:41:18]

sadrul@chromium.org changed reviewers:
+ rch@chromium.org

[sadrul, 2016-12-08 20:41:19]

+rch@ for net DEPS

[Ryan Hamilton, 2016-12-09 00:33:53]

rch@chromium.org changed reviewers:
+ rsleevi@chromium.org

[Ryan Hamilton, 2016-12-09 00:33:54]

+rsleevi

Making //gpu depend on net (which in turn depends on things like v8) seems a bit
surprising. Is there an alternative to doing this?

[sadrul, 2016-12-09 00:38:15]

On 2016/12/09 00:33:54, Ryan Hamilton wrote:
> +rsleevi
> 
> Making //gpu depend on net

Note that the dependency is in //gpu/ipc/host (as opposed to //gpu).

> (which in turn depends on things like v8) seems a bit
> surprising. Is there an alternative to doing this?

We already have this dependency, albeit indirectly, right now:

//gpu/ipc/host:host --[private]-->
//gpu/ipc/common:common --[public]-->
//gpu:gpu --[public]-->
//gpu/ipc/client:ipc_client_sources --[private]-->
//ui/base:base --[private]-->
//net:net

(gn path tells me there are 8 paths from //gpu/ipc/host to //net). This makes
this dependency more obvious.

[Ryan Sleevi, 2016-12-09 00:48:34]

On 2016/12/09 00:38:15, sadrul wrote:
> On 2016/12/09 00:33:54, Ryan Hamilton wrote:
> > +rsleevi
> > 
> > Making //gpu depend on net
> 
> Note that the dependency is in //gpu/ipc/host (as opposed to //gpu).
> 
> > (which in turn depends on things like v8) seems a bit
> > surprising. Is there an alternative to doing this?
> 
> We already have this dependency, albeit indirectly, right now:
> 
> //gpu/ipc/host:host --[private]-->
> //gpu/ipc/common:common --[public]-->
> //gpu:gpu --[public]-->
> //gpu/ipc/client:ipc_client_sources --[private]-->
> //ui/base:base --[private]-->
> //net:net
> 
> (gn path tells me there are 8 paths from //gpu/ipc/host to //net). This makes
> this dependency more obvious.

The //ui/base:base -> //net dependency is a bug we've been trying to minimize.
I'm surprised it's expanded (with filename_util.h and escape.h). This is part of
why //components/url_formatter was created (along with //components/mime_util).
I know thakis@ certainly has thoughts on the wtf-ness of that particular
dependency.

This significantly expands the scope from being two relatively small files in
//net/base to being a rather sizable part of the disk cache (and thus
transitively interacts with much more). Trying to minimize this is part of the
private-dep nature - to not expose //net to //ui consumers.

While I'd love to link to a recent (internal-only) design docs thread, in which
dependencies and design were raised and discussed, perhaps more concretely, we
can/should check with folks like brettw@, dpranke@, or beng@, who have all
expressed concerns about making sure our dependency tree is logical and
unsurprising. I think this definitely raises an eyebrow, and not something I
think we should expect, but they may feel otherwise and this is acceptable. This
is largely a question of overall Chrome dependency tree, rather than the //net
team being particular about it, so I want to make sure we've got some buy-in
from the folks tasked with overseeing that overall design.

[sadrul, 2016-12-09 00:51:46]

sadrul@chromium.org changed reviewers:
+ ben@chromium.org

[sadrul, 2016-12-09 00:51:47]

+ben@ for thoughts.

[Ben Goodger (Google), 2016-12-10 00:33:52]

On 2016/12/09 00:51:47, sadrul wrote:
> +ben@ for thoughts.

I don't feel so icky about this since it's technically not introducing a new
dependency, just moving some code around. I think it's totally fair for the
//net folk to ponder their public API & how it gets used by various places. That
should maybe happen async from this change, though I would say the mus team
should be ok to adapt in the near term.

-Ben

[Ryan Sleevi, 2016-12-10 00:40:24]

I missed that //net already has an encapsulation break by having allowed
net::SHADER_CACHE, net::PNACL_CACHE, and net::APP_CACHE.

That all feels weird/wrong, but in the absence of more time to clean this up and
better encapsulate (//net shouldn't have to know about these), LGTM.

[sadrul, 2016-12-10 03:49:32]

On 2016/12/10 00:40:24, Ryan Sleevi wrote:
> I missed that //net already has an encapsulation break by having allowed
> net::SHADER_CACHE, net::PNACL_CACHE, and net::APP_CACHE.
> 
> That all feels weird/wrong, but in the absence of more time to clean this up
and
> better encapsulate (//net shouldn't have to know about these), LGTM.

Thank you! I am happy to help with any necessary clean-up work on the gpu/mus
side.

[sadrul, 2016-12-10 03:49:54]

The CQ bit was checked by sadrul@chromium.org

[commit-bot: I haz the power, 2016-12-10 03:50:18]

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, 2016-12-10 04:38:16]

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

[commit-bot: I haz the power, 2016-12-10 04:38:17]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sadrul, 2016-12-10 06:07:52]

The CQ bit was checked by sadrul@chromium.org

[commit-bot: I haz the power, 2016-12-10 06:08:04]

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, 2016-12-10 07:03:09]

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

[commit-bot: I haz the power, 2016-12-10 07:03:10]

Failed to apply patch for content/browser/browser_main_loop.cc:
While running git apply --index -p1;
  error: patch failed: content/browser/browser_main_loop.cc:86
  error: content/browser/browser_main_loop.cc: patch does not apply

Patch:       content/browser/browser_main_loop.cc
Index: content/browser/browser_main_loop.cc
diff --git a/content/browser/browser_main_loop.cc
b/content/browser/browser_main_loop.cc
index
d6b6f98a3ea2427082b0d96fc5b40988cc6608fe..2df94bbc4840a19da4e9be89df5c19d19d368ffd
100644
--- a/content/browser/browser_main_loop.cc
+++ b/content/browser/browser_main_loop.cc
@@ -56,7 +56,6 @@
 #include "content/browser/gpu/gpu_data_manager_impl.h"
 #include "content/browser/gpu/gpu_process_host.h"
 #include "content/browser/gpu/gpu_process_host_ui_shim.h"
-#include "content/browser/gpu/shader_disk_cache.h"
 #include "content/browser/histogram_synchronizer.h"
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
 #include "content/browser/loader_delegate_impl.h"
@@ -86,6 +85,7 @@
 #include "content/public/common/result_codes.h"
 #include "device/battery/battery_status_service.h"
 #include "device/time_zone_monitor/time_zone_monitor.h"
+#include "gpu/ipc/host/shader_disk_cache.h"
 #include "media/base/media.h"
 #include "media/base/user_input_monitor.h"
 #include "media/midi/midi_manager.h"
@@ -1250,7 +1250,7 @@ int BrowserMainLoop::BrowserThreadsStarted() {
   // Initialize the GPU shader cache. This needs to be initialized before
   // BrowserGpuChannelHostFactory below, since that depends on an initialized
   // ShaderCacheFactory.
-  ShaderCacheFactory::InitInstance(
+  gpu::ShaderCacheFactory::InitInstance(
       BrowserThread::GetTaskRunnerForThread(BrowserThread::IO),
       BrowserThread::GetTaskRunnerForThread(BrowserThread::CACHE));

[sadrul, 2016-12-10 15:07:49]

The CQ bit was checked by sadrul@chromium.org

[sadrul, 2016-12-10 15:07:50]

The patchset sent to the CQ was uploaded after l-g-t-m from piman@chromium.org,
rsleevi@chromium.org
Link to the patchset: https://codereview.chromium.org/2555693005/#ps80001
(title: "tot merge")

[commit-bot: I haz the power, 2016-12-10 15:07:58]

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, 2016-12-10 16:51:14]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1481382469429950,
"parent_rev": "0157bdbeefc9f558cf89942f310d8e33fcb2891d", "commit_rev":
"281b2da3661edf1cab229dc1d8c7f64198e23eee"}

[commit-bot: I haz the power, 2016-12-10 16:51:45]

Description was changed from

==========
gpu: Move ShaderDiskCache into //gpu/ipc/host component.

BUG=643746, 604369
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
gpu: Move ShaderDiskCache into //gpu/ipc/host component.

BUG=643746, 604369
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2555693005
==========

[commit-bot: I haz the power, 2016-12-10 16:51:46]

Committed patchset #5 (id:80001)

[findit-for-me, 2016-12-10 18:41:53]

FYI: Findit identified this CL at revision 437771 as the culprit for
failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3Itb...

[gab, 2016-12-11 23:25:27]

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

The reason for reverting is: Makes
StoragePartitionShaderClearTest.ClearShaderCache fail on Linux Tests bot:
https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests%20%28dbg%2....

[commit-bot: I haz the power, 2016-12-12 15:07:04]

Description was changed from

==========
gpu: Move ShaderDiskCache into //gpu/ipc/host component.

BUG=643746, 604369
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2555693005
==========

to

==========
gpu: Move ShaderDiskCache into //gpu/ipc/host component.

BUG=643746, 604369
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/4a4522e8d9a3ae704b7a17ebb412d2a94aa820c2
Cr-Commit-Position: refs/heads/master@{#437771}
==========

[commit-bot: I haz the power, 2016-12-12 15:07:06]

Patchset 6 (id:??) landed as
https://crrev.com/4a4522e8d9a3ae704b7a17ebb412d2a94aa820c2
Cr-Commit-Position: refs/heads/master@{#437771}

[sadrul, 2016-12-13 04:33:08]

Description was changed from

==========
gpu: Move ShaderDiskCache into //gpu/ipc/host component.

BUG=643746, 604369
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/4a4522e8d9a3ae704b7a17ebb412d2a94aa820c2
Cr-Commit-Position: refs/heads/master@{#437771}
==========

to

==========
gpu: Move ShaderDiskCache into //gpu/ipc/host component.

BUG=643746, 604369
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/4a4522e8d9a3ae704b7a17ebb412d2a94aa820c2
Cr-Commit-Position: refs/heads/master@{#437771}
==========

[sadrul, 2016-12-13 04:33:40]

On 2016/12/11 23:25:27, gab wrote:
> A revert of this CL (patchset #5 id:80001) has been created in
> https://codereview.chromium.org/2565243002/ by mailto:gab@chromium.org.
> 
> The reason for reverting is: Makes
> StoragePartitionShaderClearTest.ClearShaderCache fail on Linux Tests bot:
>
https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests%20%28dbg%2....

The failure was because of an issue with the singleton object. I addressed that
separately here: https://codereview.chromium.org/2565223003/

[sadrul, 2016-12-13 07:50:24]

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

[commit-bot: I haz the power, 2016-12-13 07:50:46]

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, 2016-12-13 08:04:55]

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

[commit-bot: I haz the power, 2016-12-13 08:04:57]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[sadrul, 2016-12-13 13:16:12]

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

[commit-bot: I haz the power, 2016-12-13 13:16:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sadrul, 2016-12-13 14:01:15]

The CQ bit was checked by sadrul@chromium.org

[sadrul, 2016-12-13 14:01:16]

The patchset sent to the CQ was uploaded after l-g-t-m from piman@chromium.org,
rsleevi@chromium.org
Link to the patchset: https://codereview.chromium.org/2555693005/#ps140001
(title: "tot merge")

[commit-bot: I haz the power, 2016-12-13 14:01:22]

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, 2016-12-13 14:35:52]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1481637675910580,
"parent_rev": "6f4214f9b31f4580a3d98c29bf494244b5a2af8c", "commit_rev":
"d5bb57f81d1162a6f006c3d93336043a72527155"}

[commit-bot: I haz the power, 2016-12-13 14:36:18]

Description was changed from

==========
gpu: Move ShaderDiskCache into //gpu/ipc/host component.

BUG=643746, 604369
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/4a4522e8d9a3ae704b7a17ebb412d2a94aa820c2
Cr-Commit-Position: refs/heads/master@{#437771}
==========

to

==========
gpu: Move ShaderDiskCache into //gpu/ipc/host component.

BUG=643746, 604369
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/4a4522e8d9a3ae704b7a17ebb412d2a94aa820c2
Cr-Commit-Position: refs/heads/master@{#437771}
Review-Url: https://codereview.chromium.org/2555693005
==========

[commit-bot: I haz the power, 2016-12-13 14:36:19]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-12-13 14:38:42]

Description was changed from

==========
gpu: Move ShaderDiskCache into //gpu/ipc/host component.

BUG=643746, 604369
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/4a4522e8d9a3ae704b7a17ebb412d2a94aa820c2
Cr-Commit-Position: refs/heads/master@{#437771}
Review-Url: https://codereview.chromium.org/2555693005
==========

to

==========
gpu: Move ShaderDiskCache into //gpu/ipc/host component.

BUG=643746, 604369
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/4a4522e8d9a3ae704b7a17ebb412d2a94aa820c2
Committed: https://crrev.com/9095a6d305b5ce5d2e7a781c3e3ebd628410913e
Cr-Original-Commit-Position: refs/heads/master@{#437771}
Cr-Commit-Position: refs/heads/master@{#438165}
==========

[commit-bot: I haz the power, 2016-12-13 14:38:43]

Patchset 8 (id:??) landed as
https://crrev.com/9095a6d305b5ce5d2e7a781c3e3ebd628410913e
Cr-Commit-Position: refs/heads/master@{#438165}