From 50c28059f282dbbeb1c12c419d57d72a2b9ae623 Mon Sep 17 00:00:00 2001 From: Mohsen Izadi Date: Fri, 7 Sep 2018 00:32:14 +0000 Subject: [PATCH] Some cleanup to GpuHost implementation This a a follow-up to introducing viz::GpuHostImpl (crrev/c/1180658). This includes: - Removing an expired CHECK; - Limiting GpuHost::SetChildSurface() to Windows; - Removing unused GpuProcessHost::OnDestroyingVideoSurfaceAck(). BUG=709332 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel Change-Id: Ida7488d887ff5d511b53e8e6997835486572b648 Reviewed-on: https://chromium-review.googlesource.com/1195545 Commit-Queue: Mohsen Izadi Reviewed-by: Sadrul Chowdhury Reviewed-by: Ken Buchanan Reviewed-by: Antoine Labour Cr-Commit-Position: refs/heads/master@{#589381} --- components/viz/host/gpu_host_impl.cc | 9 ++------- components/viz/host/gpu_host_impl.h | 6 ++++-- content/browser/gpu/gpu_process_host.cc | 17 +++-------------- content/browser/gpu/gpu_process_host.h | 11 ++--------- .../viz/privileged/interfaces/gl/gpu_host.mojom | 2 ++ services/ws/gpu_host/gpu_host.cc | 6 ++---- services/ws/gpu_host/gpu_host.h | 2 ++ 7 files changed, 17 insertions(+), 36 deletions(-) diff --git a/components/viz/host/gpu_host_impl.cc b/components/viz/host/gpu_host_impl.cc index 3b9b27aac1cc72..10603fc327d0a4 100644 --- a/components/viz/host/gpu_host_impl.cc +++ b/components/viz/host/gpu_host_impl.cc @@ -368,9 +368,6 @@ std::string GpuHostImpl::GetShaderPrefixKey() { #if defined(OS_ANDROID) std::string build_fp = base::android::BuildInfo::GetInstance()->android_build_fp(); - // TODO(ericrk): Remove this after it's up for a few days. - // https://crbug.com/699122. - CHECK(!build_fp.empty()); shader_prefix_key_ += "-" + build_fp; #endif } @@ -529,9 +526,9 @@ void GpuHostImpl::DisableGpuCompositing() { delegate_->DisableGpuCompositing(); } +#if defined(OS_WIN) void GpuHostImpl::SetChildSurface(gpu::SurfaceHandle parent, gpu::SurfaceHandle child) { -#if defined(OS_WIN) constexpr char kBadMessageError[] = "Bad parenting request from gpu process."; if (!params_.in_process) { DWORD parent_process_id = 0; @@ -555,10 +552,8 @@ void GpuHostImpl::SetChildSurface(gpu::SurfaceHandle parent, child)) { LOG(ERROR) << kBadMessageError; } -#else - NOTREACHED(); -#endif } +#endif // defined(OS_WIN) void GpuHostImpl::StoreShaderToDisk(int32_t client_id, const std::string& key, diff --git a/components/viz/host/gpu_host_impl.h b/components/viz/host/gpu_host_impl.h index c96d6bdfe834b3..3e9eb539f10c06 100644 --- a/components/viz/host/gpu_host_impl.h +++ b/components/viz/host/gpu_host_impl.h @@ -105,14 +105,14 @@ class VIZ_HOST_EXPORT GpuHostImpl : public mojom::GpuHost { bool in_process = false; // Whether caching GPU shader on disk is disabled or not. - bool disable_gpu_shader_disk_cache; + bool disable_gpu_shader_disk_cache = false; // A string representing the product name and version; used to build a // prefix for shader keys. std::string product; // Number of frames to CompositorFrame activation deadline. - base::Optional deadline_to_synchronize_surfaces = base::nullopt; + base::Optional deadline_to_synchronize_surfaces; // Task runner corresponding to the main thread. scoped_refptr main_thread_task_runner; @@ -203,8 +203,10 @@ class VIZ_HOST_EXPORT GpuHostImpl : public mojom::GpuHost { gpu::error::ContextLostReason reason, const GURL& active_url) override; void DisableGpuCompositing() override; +#if defined(OS_WIN) void SetChildSurface(gpu::SurfaceHandle parent, gpu::SurfaceHandle child) override; +#endif void StoreShaderToDisk(int32_t client_id, const std::string& key, const std::string& shader) override; diff --git a/content/browser/gpu/gpu_process_host.cc b/content/browser/gpu/gpu_process_host.cc index a135b69de1b642..00972418f311ca 100644 --- a/content/browser/gpu/gpu_process_host.cc +++ b/content/browser/gpu/gpu_process_host.cc @@ -872,13 +872,6 @@ void GpuProcessHost::OnChannelConnected(int32_t peer_pid) { } } -#if defined(OS_ANDROID) -void GpuProcessHost::OnDestroyingVideoSurfaceAck() { - TRACE_EVENT0("gpu", "GpuProcessHost::OnDestroyingVideoSurfaceAck"); - if (!send_destroying_video_surface_done_cb_.is_null()) - base::ResetAndReturn(&send_destroying_video_surface_done_cb_).Run(); -} -#endif void GpuProcessHost::OnProcessLaunched() { UMA_HISTOGRAM_TIMES("GPU.GPUProcessLaunchTime", @@ -952,14 +945,13 @@ void GpuProcessHost::DidCreateContextSuccessfully() { #endif } -void GpuProcessHost::BlockDomainFrom3DAPIs(const GURL& url, - Delegate::DomainGuilt guilt) { +void GpuProcessHost::BlockDomainFrom3DAPIs(const GURL& url, DomainGuilt guilt) { GpuDataManagerImpl::DomainGuilt gpu_data_manager_guilt; switch (guilt) { - case Delegate::DomainGuilt::kKnown: + case DomainGuilt::kKnown: gpu_data_manager_guilt = GpuDataManagerImpl::DOMAIN_GUILT_KNOWN; break; - case Delegate::DomainGuilt::kUnknown: + case DomainGuilt::kUnknown: gpu_data_manager_guilt = GpuDataManagerImpl::DOMAIN_GUILT_UNKNOWN; } GpuDataManagerImpl::GetInstance()->BlockDomainFrom3DAPIs( @@ -1118,9 +1110,6 @@ void GpuProcessHost::SendOutstandingReplies() { if (gpu_host_) gpu_host_->SendOutstandingReplies(); - - if (!send_destroying_video_surface_done_cb_.is_null()) - base::ResetAndReturn(&send_destroying_video_surface_done_cb_).Run(); } void GpuProcessHost::RecordProcessCrash() { diff --git a/content/browser/gpu/gpu_process_host.h b/content/browser/gpu/gpu_process_host.h index e56abaeffa73e1..c9a47f6cce83ab 100644 --- a/content/browser/gpu/gpu_process_host.h +++ b/content/browser/gpu/gpu_process_host.h @@ -144,8 +144,7 @@ class GpuProcessHost : public BrowserChildProcessHostDelegate, gpu_feature_info_for_hardware_gpu) override; void DidFailInitialize() override; void DidCreateContextSuccessfully() override; - void BlockDomainFrom3DAPIs(const GURL& url, - Delegate::DomainGuilt guilt) override; + void BlockDomainFrom3DAPIs(const GURL& url, DomainGuilt guilt) override; void DisableGpuCompositing() override; bool GpuAccessAllowed() const override; gpu::ShaderCacheFactory* GetShaderCacheFactory() override; @@ -162,10 +161,7 @@ class GpuProcessHost : public BrowserChildProcessHostDelegate, void SendGpuProcessMessage(IPC::Message* message) override; #endif -// Message handlers. -#if defined(OS_ANDROID) - void OnDestroyingVideoSurfaceAck(); -#endif + // Message handlers. void OnFieldTrialActivated(const std::string& trial_name); bool LaunchGpuProcess(); @@ -183,9 +179,6 @@ class GpuProcessHost : public BrowserChildProcessHostDelegate, // GPU process id in case GPU is not in-process. base::ProcessId process_id_ = base::kNullProcessId; - // A callback to signal the completion of a SendDestroyingVideoSurface call. - base::Closure send_destroying_video_surface_done_cb_; - // Qeueud messages to send when the process launches. base::queue queued_messages_; diff --git a/services/viz/privileged/interfaces/gl/gpu_host.mojom b/services/viz/privileged/interfaces/gl/gpu_host.mojom index 11849c9ad901a6..2e081bb5cdf776 100644 --- a/services/viz/privileged/interfaces/gl/gpu_host.mojom +++ b/services/viz/privileged/interfaces/gl/gpu_host.mojom @@ -35,8 +35,10 @@ interface GpuHost { // track of this decision in case the GPU process crashes. DisableGpuCompositing(); + [EnableIf=is_win] SetChildSurface(gpu.mojom.SurfaceHandle parent, gpu.mojom.SurfaceHandle child); + StoreShaderToDisk(int32 client_id, string key, string shader); RecordLogMessage(int32 severity, string header, string message); diff --git a/services/ws/gpu_host/gpu_host.cc b/services/ws/gpu_host/gpu_host.cc index 24c3515b003e50..4606e3eee0a9a1 100644 --- a/services/ws/gpu_host/gpu_host.cc +++ b/services/ws/gpu_host/gpu_host.cc @@ -193,9 +193,9 @@ void DefaultGpuHost::DidLoseContext(bool offscreen, void DefaultGpuHost::DisableGpuCompositing() {} +#if defined(OS_WIN) void DefaultGpuHost::SetChildSurface(gpu::SurfaceHandle parent, gpu::SurfaceHandle child) { -#if defined(OS_WIN) // Verify that |parent| was created by the window server. DWORD process_id = 0; DWORD thread_id = GetWindowThreadProcessId(parent, &process_id); @@ -210,10 +210,8 @@ void DefaultGpuHost::SetChildSurface(gpu::SurfaceHandle parent, child)) { OnBadMessageFromGpu(); } -#else - NOTREACHED(); -#endif } +#endif // defined(OS_WIN) void DefaultGpuHost::StoreShaderToDisk(int32_t client_id, const std::string& key, diff --git a/services/ws/gpu_host/gpu_host.h b/services/ws/gpu_host/gpu_host.h index 0ff47fe662bf4f..d81ca9910a4ee3 100644 --- a/services/ws/gpu_host/gpu_host.h +++ b/services/ws/gpu_host/gpu_host.h @@ -110,8 +110,10 @@ class DefaultGpuHost : public GpuHost, public viz::mojom::GpuHost { gpu::error::ContextLostReason reason, const GURL& active_url) override; void DisableGpuCompositing() override; +#if defined(OS_WIN) void SetChildSurface(gpu::SurfaceHandle parent, gpu::SurfaceHandle child) override; +#endif void StoreShaderToDisk(int32_t client_id, const std::string& key, const std::string& shader) override;