Adding offline navigations to the previews blacklist

chrome/browser/previews/previews_infobar_tab_helper.cc

Index: chrome/browser/previews/previews_infobar_tab_helper.cc
diff --git a/chrome/browser/previews/previews_infobar_tab_helper.cc b/chrome/browser/previews/previews_infobar_tab_helper.cc
index 870d22e2c540b770286cbac46762734d97a694e2..dcca36013ffe673e520f57e62189758221cab5b1 100644
--- a/chrome/browser/previews/previews_infobar_tab_helper.cc
+++ b/chrome/browser/previews/previews_infobar_tab_helper.cc
@@ -4,9 +4,17 @@
 
 #include "chrome/browser/previews/previews_infobar_tab_helper.h"
 
+#include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "chrome/browser/previews/previews_infobar_delegate.h"
+#include "chrome/browser/previews/previews_service.h"
+#include "chrome/browser/previews/previews_service_factory.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/common/features.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_headers.h"
+#include "components/previews/core/previews_experiments.h"
+#include "components/previews/core/previews_ui_service.h"
+#include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
@@ -16,6 +24,25 @@
 
 #if BUILDFLAG(ANDROID_JAVA_UI)
 #include "chrome/browser/android/offline_pages/offline_page_tab_helper.h"
+
+namespace {
+
+// Adds the preview navigation to the black list. This method is only used on
+// android currently.
+void AddPreviewNavigationCallback(content::BrowserContext* browser_context,
+                                  const GURL& url,
+                                  previews::PreviewsType type,
+                                  bool opt_out) {
+  PreviewsService* previews_service = PreviewsServiceFactory::GetForProfile(
+      Profile::FromBrowserContext(browser_context));
+  if (previews_service && previews_service->previews_ui_service()) {
+    previews_service->previews_ui_service()->AddPreviewNavigation(url, type,
+                                                                  opt_out);
+  }
+}
+
+}  // namespace
+
 #endif  // BUILDFLAG(ANDROID_JAVA_UI)
 
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(PreviewsInfoBarTabHelper);
@@ -49,8 +76,11 @@ void PreviewsInfoBarTabHelper::DidFinishNavigation(
       return;
     }
     is_showing_offline_preview_ = true;
-    PreviewsInfoBarDelegate::Create(web_contents(),
-                                    PreviewsInfoBarDelegate::OFFLINE);
+    PreviewsInfoBarDelegate::Create(
+        web_contents(), PreviewsInfoBarDelegate::OFFLINE,
+        base::Bind(
+            &AddPreviewNavigationCallback, web_contents()->GetBrowserContext(),
+            navigation_handle->GetURL(), previews::PreviewsType::OFFLINE));

[megjablon, 2016/11/09 19:35:05]

Can you add a test to previews_infobar_tab_helper_unittests.cc for this?

[RyanSturm, 2016/11/09 20:45:47]

I wanted to avoid it due to the nature of KeyedService initialization. This
would need a browser test to test anything meaningful. Based on the infobar
delegate test we know the callback mechanism functions correctly, so we need to
test getting the PreviewsService and accessing previews_ui_service() on it.

I'll add a browser test to verify that PreviewsService is created and
initialized properly, but I think a browser test that runs the tab helper code
is excessive. The previews_io_data_unittest and other tests cover all the cases.
With this browser test the only thing that is untested about the callback is
running the code itself, but all of its functionality is tested.

[megjablon, 2016/11/09 22:17:54]

Ok sounds reasonable.

     // Don't try to show other infobars if this is an offline preview.
     return;
   }
@@ -59,8 +89,9 @@ void PreviewsInfoBarTabHelper::DidFinishNavigation(
   const net::HttpResponseHeaders* headers =
       navigation_handle->GetResponseHeaders();
   if (headers && data_reduction_proxy::IsLitePagePreview(*headers)) {
-    PreviewsInfoBarDelegate::Create(web_contents(),
-                                    PreviewsInfoBarDelegate::LITE_PAGE);
+    PreviewsInfoBarDelegate::Create(
+        web_contents(), PreviewsInfoBarDelegate::LITE_PAGE,
+        PreviewsInfoBarDelegate::OnDismissPreviewsInfobarCallback());
   }
 }