Use the previews black list for offline previews

chrome/browser/android/offline_pages/offline_page_request_job.cc

Index: chrome/browser/android/offline_pages/offline_page_request_job.cc
diff --git a/chrome/browser/android/offline_pages/offline_page_request_job.cc b/chrome/browser/android/offline_pages/offline_page_request_job.cc
index bc8ad7460fb11f40c8896f8a48d1557f85c5c02c..d626fda90605d2f1d37db37fb767adae0b0547fc 100644
--- a/chrome/browser/android/offline_pages/offline_page_request_job.cc
+++ b/chrome/browser/android/offline_pages/offline_page_request_job.cc
@@ -12,21 +12,23 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/android/offline_pages/offline_page_model_factory.h"
 #include "chrome/browser/android/offline_pages/offline_page_tab_helper.h"
 #include "chrome/browser/android/offline_pages/offline_page_utils.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "components/offline_pages/offline_page_model.h"
 #include "components/offline_pages/request_header/offline_page_header.h"
+#include "components/previews/core/previews_black_list.h"
 #include "components/previews/core/previews_experiments.h"
+#include "components/previews/core/previews_opt_out_store.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/resource_request_info.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/resource_type.h"
 #include "net/base/network_change_notifier.h"
 #include "net/http/http_request_headers.h"
 #include "net/nqe/network_quality_estimator.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context.h"
 
@@ -101,50 +103,61 @@ class DefaultDelegate : public OfflinePageRequestJob::Delegate {
   }
 
  private:
   static bool GetTabId(content::WebContents* web_contents, int* tab_id) {
     return OfflinePageUtils::GetTabId(web_contents, tab_id);
   }
 
   DISALLOW_COPY_AND_ASSIGN(DefaultDelegate);
 };
 
-bool IsNetworkProhibitivelySlow(net::URLRequest* request) {
+bool IsNetworkProhibitivelySlow(
+    net::URLRequest* request,
+    previews::PreviewsBlackList* previews_black_list) {
   // NetworkQualityEstimator only works when it is enabled.
   if (!previews::IsOfflinePreviewsEnabled())
     return false;
 
+  // If |previews_black_list| is non-null and the URL is not allowed for offline
+  // previews, do not allow the offline page to be shown.
+  if (previews_black_list &&

[jianli, 2016/10/03 22:56:05]

Is it possible to merge this black list into NetworkQualityEstimator or
introduce another new thing like PreviewEstimator that provides both NQE, black
list and any other future addition?

Otherwise for every place that calls NetworkQualityEstimator, we also need to
pass this new black list. For example, do we also need to pass the black list to
RequestCoordinator?

 

[RyanSturm, 2016/10/03 23:54:39]

NQE is global, the blacklist is per profile.

Embedding the blacklist in URLRequestContext would be a layering violation, and
I'm not really sure how else we could get this here.
It shouldn't need to be plumbed anywhere else in OfflinePages. I haven't looked
extensively at RequestCoordinator, but the NQE usage in it isn't related to
previews AFAIK.

+      !previews_black_list->IsLoadedAndAllowed(

[tbansal1, 2016/10/04 12:38:04]

I suppose the part where the black list is notified of navigation completions
(and opt-outs) will come later?

[RyanSturm, 2016/10/04 21:36:38]

Yes.

+          request->url(), previews::PreviewsType::OFFLINE)) {
+    return false;
+  }
+
   if (!request->context())
     return false;
 
   net::NetworkQualityEstimator* network_quality_estimator =
       request->context()->network_quality_estimator();
   if (!network_quality_estimator)
     return false;
 
   net::EffectiveConnectionType effective_connection_type =
       network_quality_estimator->GetEffectiveConnectionType();
   return effective_connection_type >= net::EFFECTIVE_CONNECTION_TYPE_OFFLINE &&
          effective_connection_type <= net::EFFECTIVE_CONNECTION_TYPE_SLOW_2G;
 }
 
 NetworkState GetNetworkState(net::URLRequest* request,
-                             const OfflinePageHeader& offline_header) {
+                             const OfflinePageHeader& offline_header,
+                             previews::PreviewsBlackList* previews_black_list) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 
   if (offline_header.reason == OfflinePageHeader::Reason::NET_ERROR)
     return NetworkState::FLAKY_NETWORK;
 
   if (net::NetworkChangeNotifier::IsOffline())
     return NetworkState::DISCONNECTED_NETWORK;
 
-  if (IsNetworkProhibitivelySlow(request))
+  if (IsNetworkProhibitivelySlow(request, previews_black_list))
     return NetworkState::PROHIBITIVELY_SLOW_NETWORK;
 
   // If offline header contains a reason other than RELOAD, the offline page
   // should be forced to load even when the network is connected.
   return (offline_header.reason != OfflinePageHeader::Reason::NONE &&
           offline_header.reason != OfflinePageHeader::Reason::RELOAD)
              ? NetworkState::FORCE_OFFLINE_ON_CONNECTED_NETWORK
              : NetworkState::CONNECTED_NETWORK;
 }
 
@@ -448,21 +461,22 @@ void SelectPage(
 void OfflinePageRequestJob::ReportAggregatedRequestResult(
     AggregatedRequestResult result) {
   UMA_HISTOGRAM_ENUMERATION("OfflinePages.AggregatedRequestResult",
       static_cast<int>(result),
       static_cast<int>(AggregatedRequestResult::AGGREGATED_REQUEST_RESULT_MAX));
 }
 
 // static
 OfflinePageRequestJob* OfflinePageRequestJob::Create(
     net::URLRequest* request,
-    net::NetworkDelegate* network_delegate) {
+    net::NetworkDelegate* network_delegate,
+    previews::PreviewsBlackList* previews_black_list) {
   const content::ResourceRequestInfo* resource_request_info =
       content::ResourceRequestInfo::ForRequest(request);
   if (!resource_request_info)
     return nullptr;
 
   // Ignore the requests not for the main resource.
   if (resource_request_info->GetResourceType() !=
       content::RESOURCE_TYPE_MAIN_FRAME) {
     return nullptr;
   }
@@ -479,55 +493,58 @@ OfflinePageRequestJob* OfflinePageRequestJob::Create(
       OfflinePageRequestInfo::GetFromRequest(request);
   if (info) {
     // Fall back to default which is set when an offline page cannot be served,
     // either page not found or online version desired.
     if (info->use_default())
       return nullptr;
   } else {
     request->SetUserData(&kUserDataKey, new OfflinePageRequestInfo());
   }
 
-  return new OfflinePageRequestJob(request, network_delegate);
+  return new OfflinePageRequestJob(request, network_delegate,
+                                   previews_black_list);
 }
 
 OfflinePageRequestJob::OfflinePageRequestJob(
     net::URLRequest* request,
-    net::NetworkDelegate* network_delegate)
+    net::NetworkDelegate* network_delegate,
+    previews::PreviewsBlackList* previews_black_list)
     : net::URLRequestFileJob(
           request,
           network_delegate,
           base::FilePath(),
-          content::BrowserThread::GetBlockingPool()->
-              GetTaskRunnerWithShutdownBehavior(
+          content::BrowserThread::GetBlockingPool()
+              ->GetTaskRunnerWithShutdownBehavior(
                   base::SequencedWorkerPool::SKIP_ON_SHUTDOWN)),
       delegate_(new DefaultDelegate()),
-      weak_ptr_factory_(this) {
-}
+      previews_black_list_(previews_black_list),
+      weak_ptr_factory_(this) {}
 
 OfflinePageRequestJob::~OfflinePageRequestJob() {
 }
 
 void OfflinePageRequestJob::Start() {
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::Bind(&OfflinePageRequestJob::StartAsync,
                             weak_ptr_factory_.GetWeakPtr()));
 }
 
 void OfflinePageRequestJob::StartAsync() {
   std::string offline_header_value;
   request()->extra_request_headers().GetHeader(
       kOfflinePageHeader, &offline_header_value);
   // Note that |offline_header| will be empty if parsing from the header value
   // fails.
   OfflinePageHeader offline_header(offline_header_value);
 
-  NetworkState network_state = GetNetworkState(request(), offline_header);
+  NetworkState network_state =
+      GetNetworkState(request(), offline_header, previews_black_list_);
   if (network_state == NetworkState::CONNECTED_NETWORK) {
     FallbackToDefault();
     return;
   }
 
   content::BrowserThread::PostTask(
       content::BrowserThread::UI,
       FROM_HERE,
       base::Bind(&SelectPage,
                  request()->url(),