Add ScopedSurfaceRequestManager

content/browser/android/browser_scoped_surface_request_manager.cc

Index: content/browser/android/browser_scoped_surface_request_manager.cc
diff --git a/content/browser/android/browser_scoped_surface_request_manager.cc b/content/browser/android/browser_scoped_surface_request_manager.cc
new file mode 100644
index 0000000000000000000000000000000000000000..8d8be640c5347f66e6b20ae6291ebc40fdbc01aa
--- /dev/null
+++ b/content/browser/android/browser_scoped_surface_request_manager.cc
@@ -0,0 +1,69 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/browser/android/browser_scoped_surface_request_manager.h"
+
+#include "ui/gl/android/scoped_java_surface.h"
+
+namespace content {
+
+// static
+BrowserScopedSurfaceRequestManager*
+BrowserScopedSurfaceRequestManager::GetInstance() {
+  return base::Singleton<
+      BrowserScopedSurfaceRequestManager,
+      base::LeakySingletonTraits<BrowserScopedSurfaceRequestManager>>::get();
+}
+
+void BrowserScopedSurfaceRequestManager::RegisterScopedSurfaceRequest(
+    uint64_t request_id,
+    ScopedSurfaceRequestCB request_cb) {
+  base::AutoLock lock(lock_);
+
+  DCHECK(request_callbacks_.find(request_id) == request_callbacks_.end());

[watk, 2016/08/26 22:01:32]

Can use .count() for this

[tguilbert, 2016/08/29 18:30:46]

Done.

+  request_callbacks_.insert(std::make_pair(request_id, request_cb));
+}
+
+gpu::ScopedSurfaceRequestManager::ScopedSurfaceRequestCB
+BrowserScopedSurfaceRequestManager::GetAndUnregisterScopedSurfaceRequest(

[watk, 2016/08/26 22:01:32]

I don't see the need for both this one and the internal one?

[tguilbert, 2016/08/29 18:30:46]

I added an internal one, so I could lock in the entirety of
FulfillScopedSurfaceRequest's scope.

You might say that we only want to lock when actually changing
|request_callbacks_| and I am fine with that.

My line of thinking was that, since any object that registers a request is
responsible for un-registering its unfulfilled requests before its destruction,
locking at the "FulfillRequest" scope (which is used in the single process case)
would be making extra sure that the object we callback the request on is still
alive.

Any thoughts as to whether we should lock when fulfilling requests in single
process mode?

[watk, 2016/08/29 19:42:25]

Ah, gotcha. Yeah, then I think what you have is best. I slightly prefer the
_Locked suffix instead of Internal, but up to you. 

Do we need to expose the "Get" version in the public interface? Probably just
UnregisterRequest would make the api clearer.

[tguilbert, 2016/08/30 22:53:09]

Ah! Yes, good call on the _Locked suffix. Although, the rest of this discussion
might make this obsolete soon.

I think from our offline discussion there was some confusion about typing that
should be cleared up with the latest patch.

+    uint64_t request_id) {
+  base::AutoLock lock(lock_);
+
+  return GetAndUnregisterInternal(request_id);
+}
+
+gpu::ScopedSurfaceRequestManager::ScopedSurfaceRequestCB
+BrowserScopedSurfaceRequestManager::GetAndUnregisterInternal(
+    uint64_t request_id) {
+  gpu::ScopedSurfaceRequestManager::ScopedSurfaceRequestCB request;
+
+  auto it = request_callbacks_.find(request_id);
+
+  if (it != request_callbacks_.end()) {
+    request = it->second;
+    request_callbacks_.erase(it);
+  }
+
+  return request;
+}
+
+void BrowserScopedSurfaceRequestManager::FulfillScopedSurfaceRequest(
+    uint64_t request_id,
+    gl::SurfaceTexture* surface_texture) {
+  base::AutoLock lock(lock_);
+
+  gpu::ScopedSurfaceRequestManager::ScopedSurfaceRequestCB request =
+      GetAndUnregisterInternal(request_id);
+
+  if (!request.is_null()) {
+    gl::ScopedJavaSurface surface(surface_texture);
+    request.Run(std::move(surface));
+  }
+}
+
+BrowserScopedSurfaceRequestManager::BrowserScopedSurfaceRequestManager() {}
+
+BrowserScopedSurfaceRequestManager::~BrowserScopedSurfaceRequestManager() {}
+
+}  // namespace content