Make the ExtensionFunction RunWithValidation() explicit to call once.

The GpuFeatureChecker callback is only called once, so convert to a
OnceCallback.

R=avi@chromium.org, reillyg@chromium.org

Bug: 1007763
Change-Id: I396324c692681f26e715bdd800153fbf8e38202b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1958187
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#723055}

chrome/browser/android/compositor/compositor_view.cc

@@ -95,10 +95,11 @@ CompositorView::CompositorView(JNIEnv* env,
   // It is safe to not keep a ref on the feature checker because it adds one
   // internally in CheckGpuFeatureAvailability and unrefs after the callback is
   // dispatched.
-  auto surface_control_feature_checker = content::GpuFeatureChecker::Create(
-      gpu::GpuFeatureType::GPU_FEATURE_TYPE_ANDROID_SURFACE_CONTROL,
-      base::Bind(&CompositorView::OnSurfaceControlFeatureStatusUpdate,
-                 weak_factory_.GetWeakPtr()));
+  scoped_refptr<content::GpuFeatureChecker> surface_control_feature_checker =
+      content::GpuFeatureChecker::Create(
+          gpu::GpuFeatureType::GPU_FEATURE_TYPE_ANDROID_SURFACE_CONTROL,
+          base::BindOnce(&CompositorView::OnSurfaceControlFeatureStatusUpdate,
+                         weak_factory_.GetWeakPtr()));
   surface_control_feature_checker->CheckGpuFeatureAvailability();
 }
 

chrome/browser/extensions/api/webstore_private/webstore_private_api.cc

@@ -603,8 +603,8 @@ ExtensionFunction::ResponseAction WebstorePrivateSetStoreLoginFunction::Run() {
 WebstorePrivateGetWebGLStatusFunction::WebstorePrivateGetWebGLStatusFunction()
     : feature_checker_(content::GpuFeatureChecker::Create(
           gpu::GPU_FEATURE_TYPE_ACCELERATED_WEBGL,
-          base::Bind(&WebstorePrivateGetWebGLStatusFunction::OnFeatureCheck,
-                     base::Unretained(this)))) {}
+          base::BindOnce(&WebstorePrivateGetWebGLStatusFunction::OnFeatureCheck,
+                         base::Unretained(this)))) {}
 
 WebstorePrivateGetWebGLStatusFunction::
     ~WebstorePrivateGetWebGLStatusFunction() {}

content/browser/gpu/gpu_feature_checker_impl.cc

@@ -20,12 +20,17 @@ scoped_refptr<GpuFeatureChecker> GpuFeatureChecker::Create(
 
 GpuFeatureCheckerImpl::GpuFeatureCheckerImpl(gpu::GpuFeatureType feature,
                                              FeatureAvailableCallback callback)
-    : feature_(feature), callback_(callback) {}
+    : feature_(feature), callback_(std::move(callback)) {}
 
 GpuFeatureCheckerImpl::~GpuFeatureCheckerImpl() {}
 
 void GpuFeatureCheckerImpl::CheckGpuFeatureAvailability() {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  // May only call CheckGpuFeatureAvailability() once.
+  DCHECK(!checking_);
+  checking_ = true;
+
   AddRef();  // Matched with a Release in OnGpuInfoUpdate.
   GpuDataManagerImpl* manager = GpuDataManagerImpl::GetInstance();
   manager->AddObserver(this);
@@ -38,7 +43,7 @@ void GpuFeatureCheckerImpl::OnGpuInfoUpdate() {
     manager->RemoveObserver(this);
     bool feature_allowed =
         manager->GetFeatureStatus(feature_) == gpu::kGpuFeatureStatusEnabled;
-    callback_.Run(feature_allowed);
+    std::move(callback_).Run(feature_allowed);
     Release();  // Matches the AddRef in CheckGpuFeatureAvailability().
   }
 }

content/browser/gpu/gpu_feature_checker_impl.h

@@ -31,6 +31,7 @@ class CONTENT_EXPORT GpuFeatureCheckerImpl : public GpuFeatureChecker,
 
   gpu::GpuFeatureType feature_;
   FeatureAvailableCallback callback_;
+  bool checking_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(GpuFeatureCheckerImpl);
 };

content/public/browser/gpu_feature_checker.h

@@ -12,17 +12,28 @@
 
 namespace content {
 
+// GpuFeatureChecker does an asynchronous lookup for a GpuFeatureType. It will
+// keep itself alive (via being a refcounted object) until it calls the given
+// |callback|. The feature check is not initiated (and a self-reference is not
+// taken) until CheckGpuFeatureAvailability() is called. If the feature is
+// already available, the |callback| may be called synchronously inside of the
+// CheckGpuFeatureAvailability() call.
 class GpuFeatureChecker : public base::RefCountedThreadSafe<GpuFeatureChecker> {
  public:
-  typedef base::Callback<void(bool feature_available)> FeatureAvailableCallback;
+  using FeatureAvailableCallback =
+      base::OnceCallback<void(bool feature_available)>;
 
   CONTENT_EXPORT static scoped_refptr<GpuFeatureChecker> Create(
       gpu::GpuFeatureType feature,
       FeatureAvailableCallback callback);
 
   // Checks to see if |feature_| is available on the current GPU. |callback_|
-  // will be called to indicate the availability of the feature. Must be called
+  // will be called to indicate the availability of the feature. It may be
+  // called synchronously if the data is already available, or else will be
+  // called asynchronously when it becomes available. This method must be called
   // from the the UI thread.
+  // CheckGpuFeatureAvailability() may be called at most once for any given
+  // GpuFeatureChecker
   virtual void CheckGpuFeatureAvailability() = 0;
 
  protected:

extensions/browser/extension_function.cc

@@ -327,6 +327,11 @@ bool ExtensionFunction::PreRunValidation(std::string* error) {
 }
 
 ExtensionFunction::ResponseAction ExtensionFunction::RunWithValidation() {
+#if DCHECK_IS_ON()
+  DCHECK(!did_run_);
+  did_run_ = true;
+#endif
+
   std::string error;
   if (!PreRunValidation(&error)) {
     DCHECK(!error.empty() || bad_message_);

extensions/browser/extension_function.h

@@ -166,11 +166,13 @@ class ExtensionFunction : public base::RefCountedThreadSafe<
   // this case). If this returns true, execution continues on to Run().
   virtual bool PreRunValidation(std::string* error);
 
-  // Runs the extension function if PreRunValidation() succeeds.
+  // Runs the extension function if PreRunValidation() succeeds. This should be
+  // called at most once over the lifetime of an ExtensionFunction.
   ResponseAction RunWithValidation();
 
   // Runs the function and returns the action to take when the caller is ready
-  // to respond.
+  // to respond. Callers can expect this is called at most once for the lifetime
+  // of an ExtensionFunction.
   //
   // Typical return values might be:
   //   * RespondNow(NoArguments())
@@ -516,6 +518,12 @@ class ExtensionFunction : public base::RefCountedThreadSafe<
   // returning.  Usually we want to kill the message sending process.
   bool bad_message_ = false;
 
+#if DCHECK_IS_ON()
+  // Set to true when RunWithValidation() is called, to look for callers using
+  // the method more than once on a single ExtensionFunction.
+  bool did_run_ = false;
+#endif
+
   // The sample value to record with the histogram API when the function
   // is invoked.
   extensions::functions::HistogramValue histogram_value_ =

extensions/browser/requirements_checker.cc

@@ -39,11 +39,12 @@ void RequirementsChecker::Start(ResultCallback callback) {
 
   callback_ = std::move(callback);
   if (requirements.webgl) {
-    webgl_checker_ = content::GpuFeatureChecker::Create(
-        gpu::GPU_FEATURE_TYPE_ACCELERATED_WEBGL,
-        base::Bind(&RequirementsChecker::VerifyWebGLAvailability,
-                   weak_ptr_factory_.GetWeakPtr()));
-    webgl_checker_->CheckGpuFeatureAvailability();
+    scoped_refptr<content::GpuFeatureChecker> webgl_checker =
+        content::GpuFeatureChecker::Create(
+            gpu::GPU_FEATURE_TYPE_ACCELERATED_WEBGL,
+            base::BindOnce(&RequirementsChecker::VerifyWebGLAvailability,
+                           weak_ptr_factory_.GetWeakPtr()));
+    webgl_checker->CheckGpuFeatureAvailability();
   } else {
     PostRunCallback();
   }

extensions/browser/requirements_checker.h

@@ -41,8 +41,6 @@ class RequirementsChecker : public PreloadCheck {
   // Helper function to run the callback.
   void RunCallback();
 
-  scoped_refptr<content::GpuFeatureChecker> webgl_checker_;
-
   ResultCallback callback_;
   Errors errors_;