Implement Web Restrictions in WebView.

components/web_restrictions/browser/web_restrictions_client.cc

Index: components/web_restrictions/browser/web_restrictions_client.cc
diff --git a/components/web_restrictions/browser/web_restrictions_client.cc b/components/web_restrictions/browser/web_restrictions_client.cc
index f263743b2992fb79fbbf72d125184cb77d25a167..5465632b60180fd57eec591158cd5cd8e6c785cf 100644
--- a/components/web_restrictions/browser/web_restrictions_client.cc
+++ b/components/web_restrictions/browser/web_restrictions_client.cc
@@ -20,12 +20,12 @@ namespace {
 const size_t kMaxCacheSize = 100;
 
 bool RequestPermissionTask(
-    const GURL& url,
+    const std::string& url,
     const base::android::JavaRef<jobject>& java_provider) {
   JNIEnv* env = base::android::AttachCurrentThread();
   return Java_WebRestrictionsClient_requestPermission(
       env, java_provider.obj(),
-      base::android::ConvertUTF8ToJavaString(env, url.spec()).obj());
+      base::android::ConvertUTF8ToJavaString(env, url).obj());
 }
 
 bool CheckSupportsRequestTask(
@@ -43,7 +43,8 @@ bool WebRestrictionsClient::Register(JNIEnv* env) {
 
 WebRestrictionsClient::WebRestrictionsClient()
     : initialized_(false), supports_request_(false) {
-  single_thread_task_runner_ = base::ThreadTaskRunnerHandle::Get();
+  io_thread_task_runner_ = content::BrowserThread::GetMessageLoopProxyForThread(

[Bernhard Bauer, 2016/04/18 14:48:34]

If you are not injecting this as a dependency, just directly post to the IO
thread using BrowserThread::PostTask. That way you can also check thread usage
with DCHECK_CURRENTLY_ON(), which gives nicer error messages.

[aberent, 2016/05/18 20:06:50]

Done.

+      content::BrowserThread::IO);
   base::SequencedWorkerPool* worker_pool =
       content::BrowserThread::GetBlockingPool();
   background_task_runner_ =
@@ -62,7 +63,22 @@ WebRestrictionsClient::~WebRestrictionsClient() {
 
 void WebRestrictionsClient::SetAuthority(
     const std::string& content_provider_authority) {
-  DCHECK(single_thread_task_runner_->BelongsToCurrentThread());
+  // This is called from the UI thread in normal use.
+  if (io_thread_task_runner_->BelongsToCurrentThread()) {
+    // Only used for testing. Avoids need for complex multithreading in tests.
+    SetAuthorityTask(content_provider_authority);

[Bernhard Bauer, 2016/04/18 14:48:34]

You could just friend the test class and call SetAuthorityTask() directly in the
test, no?

[aberent, 2016/05/18 20:06:50]

Done.

+  } else {
+    // Run a task on the IO thread to do the real work.
+    io_thread_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&WebRestrictionsClient::SetAuthorityTask,
+                   base::Unretained(this), content_provider_authority));
+  }
+}
+
+void WebRestrictionsClient::SetAuthorityTask(
+    const std::string& content_provider_authority) {
+  DCHECK(io_thread_task_runner_->BelongsToCurrentThread());
   // Destroy any existing content resolver.
   JNIEnv* env = base::android::AttachCurrentThread();
   if (!java_provider_.is_null()) {
@@ -91,18 +107,17 @@ void WebRestrictionsClient::SetAuthority(
 
 UrlAccess WebRestrictionsClient::ShouldProceed(
     bool is_main_frame,
-    const GURL& url,
+    const std::string& url,
     const base::Callback<void(bool)>& callback) {
-  DCHECK(single_thread_task_runner_->BelongsToCurrentThread());
+  DCHECK(io_thread_task_runner_->BelongsToCurrentThread());
   if (!initialized_)
     return ALLOW;
-  auto iter = cache_.find(url);
-  if (iter != cache_.end()) {
+
+  std::unique_ptr<const WebRestrictionsClientResult> result =
+      cache_.GetCacheEntry(url);
+  if (result.get()) {

[Bernhard Bauer, 2016/04/18 14:48:34]

The .get() is unnecessary -- unique_ptr has a bool overload.

[aberent, 2016/05/18 20:06:50]

Done.

     RecordURLAccess(url);
-    JNIEnv* env = base::android::AttachCurrentThread();
-    return Java_ShouldProceedResult_shouldProceed(env, iter->second.obj())
-               ? ALLOW
-               : DISALLOW;
+    return result->ShouldProceed() ? ALLOW : DISALLOW;
   }
   base::PostTaskAndReplyWithResult(
       background_task_runner_.get(), FROM_HERE,
@@ -118,62 +133,8 @@ bool WebRestrictionsClient::SupportsRequest() const {
   return initialized_ && supports_request_;
 }
 
-int WebRestrictionsClient::GetResultColumnCount(const GURL& url) const {
-  DCHECK(single_thread_task_runner_->BelongsToCurrentThread());
-  if (!initialized_)
-    return 0;
-  auto iter = cache_.find(url);
-  if (iter == cache_.end())
-    return 0;
-  return Java_ShouldProceedResult_getColumnCount(
-      base::android::AttachCurrentThread(), iter->second.obj());
-}
-
-std::string WebRestrictionsClient::GetResultColumnName(const GURL& url,
-                                                       int column) const {
-  DCHECK(single_thread_task_runner_->BelongsToCurrentThread());
-  if (!initialized_)
-    return std::string();
-  auto iter = cache_.find(url);
-  if (iter == cache_.end())
-    return std::string();
-
-  JNIEnv* env = base::android::AttachCurrentThread();
-  return base::android::ConvertJavaStringToUTF8(
-      env,
-      Java_ShouldProceedResult_getColumnName(env, iter->second.obj(), column)
-          .obj());
-}
-
-int WebRestrictionsClient::GetResultIntValue(const GURL& url,
-                                             int column) const {
-  DCHECK(single_thread_task_runner_->BelongsToCurrentThread());
-  if (!initialized_)
-    return 0;
-  auto iter = cache_.find(url);
-  if (iter == cache_.end())
-    return 0;
-  return Java_ShouldProceedResult_getInt(base::android::AttachCurrentThread(),
-                                         iter->second.obj(), column);
-}
-
-std::string WebRestrictionsClient::GetResultStringValue(const GURL& url,
-                                                        int column) const {
-  DCHECK(single_thread_task_runner_->BelongsToCurrentThread());
-  if (!initialized_)
-    return std::string();
-  auto iter = cache_.find(url);
-  if (iter == cache_.end())
-    return std::string();
-
-  JNIEnv* env = base::android::AttachCurrentThread();
-  return base::android::ConvertJavaStringToUTF8(
-      env, Java_ShouldProceedResult_getString(env, iter->second.obj(), column)
-               .obj());
-}
-
 void WebRestrictionsClient::RequestPermission(
-    const GURL& url,
+    const std::string& url,
     const base::Callback<void(bool)>& request_success) {
   if (!initialized_) {
     request_success.Run(false);
@@ -185,39 +146,39 @@ void WebRestrictionsClient::RequestPermission(
 }
 
 void WebRestrictionsClient::OnWebRestrictionsChanged() {
-  single_thread_task_runner_->PostTask(
+  io_thread_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&WebRestrictionsClient::ClearCache, base::Unretained(this)));
 }
 
-void WebRestrictionsClient::RecordURLAccess(const GURL& url) {
-  DCHECK(single_thread_task_runner_->BelongsToCurrentThread());
+void WebRestrictionsClient::RecordURLAccess(const std::string& url) {
+  DCHECK(io_thread_task_runner_->BelongsToCurrentThread());
   // Move the URL to the front of the cache.
   recent_urls_.remove(url);
   recent_urls_.push_front(url);
 }
 
 void WebRestrictionsClient::UpdateCache(std::string provider_authority,
-                                        GURL url,
+                                        std::string url,

[Bernhard Bauer, 2016/04/18 14:48:34]

This should be const-ref (as should have been the GURL previously).

[aberent, 2016/05/18 20:06:50]

Done.

                                         ScopedJavaGlobalRef<jobject> result) {
-  DCHECK(single_thread_task_runner_->BelongsToCurrentThread());
+  DCHECK(io_thread_task_runner_->BelongsToCurrentThread());
   // If the webrestrictions provider changed when the old one was being queried,
   // do not update the cache for the new provider.
   if (provider_authority != provider_authority_)
     return;
   RecordURLAccess(url);
   if (recent_urls_.size() >= kMaxCacheSize) {
-    cache_.erase(recent_urls_.back());
+    cache_.RemoveCacheEntry(recent_urls_.back());
     recent_urls_.pop_back();
   }
-  cache_[url] = result;
+  cache_.SetCacheEntry(url, WebRestrictionsClientResult(result));
 }
 
 void WebRestrictionsClient::RequestSupportKnown(std::string provider_authority,
                                                 bool supports_request) {
   // |supports_request_| is initialized to false.
   DCHECK(!supports_request_);
-  DCHECK(single_thread_task_runner_->BelongsToCurrentThread());
+  DCHECK(io_thread_task_runner_->BelongsToCurrentThread());
   // If the webrestrictions provider changed when the old one was being queried,
   // ignore the result.
   if (provider_authority != provider_authority_)
@@ -227,32 +188,68 @@ void WebRestrictionsClient::RequestSupportKnown(std::string provider_authority,
 
 void WebRestrictionsClient::OnShouldProceedComplete(
     std::string provider_authority,
-    const GURL& url,
+    const std::string& url,
     const base::Callback<void(bool)>& callback,
     const ScopedJavaGlobalRef<jobject>& result) {
   UpdateCache(provider_authority, url, result);
-  JNIEnv* env = base::android::AttachCurrentThread();
-  callback.Run(Java_ShouldProceedResult_shouldProceed(env, result.obj()));
+  callback.Run(cache_.GetCacheEntry(url)->ShouldProceed());
 }
 
 void WebRestrictionsClient::ClearCache() {
-  DCHECK(single_thread_task_runner_->BelongsToCurrentThread());
-  cache_.clear();
+  DCHECK(io_thread_task_runner_->BelongsToCurrentThread());
+  cache_.Clear();
   recent_urls_.clear();
 }
 
+std::unique_ptr<const WebRestrictionsClientResult>
+WebRestrictionsClient::GetCachedWebRestrictionsResult(const std::string& url) {
+  return cache_.GetCacheEntry(url);
+}
+
 // static
 ScopedJavaGlobalRef<jobject> WebRestrictionsClient::ShouldProceedTask(
-    const GURL& url,
+    const std::string& url,
     const base::android::JavaRef<jobject>& java_provider) {
   JNIEnv* env = base::android::AttachCurrentThread();
   base::android::ScopedJavaGlobalRef<jobject> result(
       Java_WebRestrictionsClient_shouldProceed(
           env, java_provider.obj(),
-          base::android::ConvertUTF8ToJavaString(env, url.spec()).obj()));
+          base::android::ConvertUTF8ToJavaString(env, url).obj()));
   return result;
 }
 
+std::unique_ptr<const WebRestrictionsClientResult>
+WebRestrictionsCache::GetCacheEntry(const std::string& url) {
+  lock_.Acquire();

[Bernhard Bauer, 2016/04/18 14:48:34]

Use base::AutoLock for this. That way you can also return nullptr if the entry
is not in the cache, so you don't need the local variable for |result|.

[aberent, 2016/05/18 20:06:50]

Done.

+  std::unique_ptr<const WebRestrictionsClientResult> result;
+  auto iter = cache_data_.find(url);
+  if (iter != cache_data_.end())
+    // This has to be thread-safe, so copy the data.
+    result.reset(new WebRestrictionsClientResult(iter->second));
+  lock_.Release();
+  return result;
+}
+
+void WebRestrictionsCache::SetCacheEntry(
+    const std::string& url,
+    const WebRestrictionsClientResult& entry) {
+  lock_.Acquire();
+  cache_data_[url] = entry;
+  lock_.Release();
+}
+
+void WebRestrictionsCache::RemoveCacheEntry(const std::string& url) {
+  lock_.Acquire();
+  cache_data_.erase(url);
+  lock_.Release();
+}
+
+void WebRestrictionsCache::Clear() {
+  lock_.Acquire();
+  cache_data_.clear();
+  lock_.Release();
+}
+
 void NotifyWebRestrictionsChanged(

[Bernhard Bauer, 2016/04/18 14:48:34]

Actually, we could remove this and just directly call OnWebRestrictionsChanged()
from Java (with the appropriate signa ture changes), no?

[aberent, 2016/05/18 20:06:50]

Done.

     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& clazz,