Avoid showing the supervised user block interstitial more than once

chrome/browser/supervised_user/supervised_user_browsertest.cc

 // Copyright 2014 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 <utility>
 
 #include "base/command_line.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
@@ skipping 215 matching lines @@
   MockTabStripModelObserver observer(tab_strip);
   base::RunLoop run_loop;
   EXPECT_CALL(observer,
               TabClosingAt(tab_strip, tab, tab_strip->active_index()))
       .WillOnce(testing::InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
   GoBack(tab);
   run_loop.Run();
   EXPECT_EQ(prev_tab, tab_strip->GetActiveWebContents());
 }
 
+// Navigates to a new tab, then blocks the current page (which makes the

[Marc Treib, 2017/05/02 08:24:46]

nit: Maybe I'm biased, but to me "Navigates to a new tab" sounds like it opens
an NTP. Maybe "Navigates to some page in a new tab, then blocks that page ..."?

[Bernhard Bauer, 2017/05/02 09:02:53]

Done.

+// interstitial page behave differently from the preceding test, where the
+// navigation is blocked before it commits). The expected behavior is the same
+// though: the tab should be closed when going back.
+IN_PROC_BROWSER_TEST_F(SupervisedUserTest, BlockNewTabAfterLoading) {
+  TabStripModel* tab_strip = browser()->tab_strip_model();
+  WebContents* prev_tab = tab_strip->GetActiveWebContents();
+
+  // Open URL in a new tab.
+  GURL test_url("http://www.example.com/simple.html");
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), test_url, WindowOpenDisposition::NEW_FOREGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+
+  // Check that there is no interstitial.
+  WebContents* tab = tab_strip->GetActiveWebContents();
+  ASSERT_FALSE(ShownPageIsInterstitial(tab));
+
+  {
+    // Block the current URL.
+    base::RunLoop run_loop;
+    InterstitialPageObserver interstitial_observer(tab, run_loop.QuitClosure());
+
+    SupervisedUserSettingsService* supervised_user_settings_service =
+        SupervisedUserSettingsServiceFactory::GetForProfile(
+            browser()->profile());
+    supervised_user_settings_service->SetLocalSetting(
+        supervised_users::kContentPackDefaultFilteringBehavior,
+        base::MakeUnique<base::Value>(SupervisedUserURLFilter::BLOCK));
+
+    const SupervisedUserURLFilter* filter =
+        supervised_user_service_->GetURLFilter();
+    ASSERT_EQ(SupervisedUserURLFilter::BLOCK,
+              filter->GetFilteringBehaviorForURL(test_url));
+
+    content::RunThisRunLoop(&run_loop);
+
+    // Check that we got the interstitial.
+    ASSERT_TRUE(ShownPageIsInterstitial(tab));
+  }
+
+  {
+    // On pressing the "back" button, the new tab should be closed, and we
+    // should get back to the previous active tab.
+    MockTabStripModelObserver observer(tab_strip);
+    base::RunLoop run_loop;
+    EXPECT_CALL(observer,
+                TabClosingAt(tab_strip, tab, tab_strip->active_index()))
+        .WillOnce(testing::InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
+    GoBack(tab);
+    run_loop.Run();
+    EXPECT_EQ(prev_tab, tab_strip->GetActiveWebContents());
+  }
+}
+
+// Tests that we don't end up canceling an interstitial (thereby closing the
+// whole tab) by attempting to show a second one above it.
+IN_PROC_BROWSER_TEST_F(SupervisedUserTest, DontShowInterstitialTwice) {
+  TabStripModel* tab_strip = browser()->tab_strip_model();
+
+  // Open URL in a new tab.
+  GURL test_url("http://www.example.com/simple.html");
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), test_url, WindowOpenDisposition::NEW_FOREGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+
+  // Check that there is no interstitial.
+  WebContents* tab = tab_strip->GetActiveWebContents();
+  ASSERT_FALSE(ShownPageIsInterstitial(tab));
+
+  // Block the current URL.
+  SupervisedUserSettingsService* supervised_user_settings_service =
+      SupervisedUserSettingsServiceFactory::GetForProfile(browser()->profile());
+  base::RunLoop run_loop;
+  InterstitialPageObserver interstitial_observer(tab, run_loop.QuitClosure());
+  supervised_user_settings_service->SetLocalSetting(
+      supervised_users::kContentPackDefaultFilteringBehavior,
+      base::MakeUnique<base::Value>(SupervisedUserURLFilter::BLOCK));
+
+  const SupervisedUserURLFilter* filter =
+      supervised_user_service_->GetURLFilter();
+  ASSERT_EQ(SupervisedUserURLFilter::BLOCK,
+            filter->GetFilteringBehaviorForURL(test_url));
+
+  content::RunThisRunLoop(&run_loop);
+
+  // Check that we got the interstitial.
+  ASSERT_TRUE(ShownPageIsInterstitial(tab));
+
+  // Trigger a no-op change to the site lists, which will notify observers of
+  // the URL filter.
+  Profile* profile = browser()->profile();
+  SupervisedUserService* supervised_user_service =
+      SupervisedUserServiceFactory::GetForProfile(profile);
+  supervised_user_service->OnSiteListUpdated();
+
+  content::RunAllPendingInMessageLoop();
+  EXPECT_EQ(tab, tab_strip->GetActiveWebContents());
+}
+
 // Tests whether a visit attempt adds a special history entry.
 IN_PROC_BROWSER_TEST_F(SupervisedUserBlockModeTest,
                        HistoryVisitRecorded) {
   GURL allowed_url("http://www.example.com/simple.html");
 
   const SupervisedUserURLFilter* filter =
       supervised_user_service_->GetURLFilter();
 
   // Set the host as allowed.
   std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue);
@@ skipping 40 matching lines @@
   QueryHistory(history_service, "", options, &results);
 
   // Check that the entries have the correct blocked_visit value.
   ASSERT_EQ(2u, results.size());
   EXPECT_EQ(blocked_url.spec(), results[0].url().spec());
   EXPECT_TRUE(results[0].blocked_visit());
   EXPECT_EQ(allowed_url.spec(), results[1].url().spec());
   EXPECT_FALSE(results[1].blocked_visit());
 }
 
-IN_PROC_BROWSER_TEST_F(SupervisedUserTest, ImmediatelyProceed) {
-  GURL test_url("http://www.example.com/simple.html");
-  ui_test_utils::NavigateToURL(browser(), test_url);
-
-  WebContents* web_contents =
-      browser()->tab_strip_model()->GetActiveWebContents();
-  ASSERT_FALSE(ShownPageIsInterstitial(web_contents));
-
-  // Manually show an interstitial page for a URL that is allowed. This
-  // simulates the case where a network request was blocked on the IO thread
-  // because a change to the URL filter hadn't been propagated yet.
-  bool proceed = false;
-  SupervisedUserInterstitial::Show(
-      web_contents, test_url, supervised_user_error_page::MANUAL,
-      /* initial_page_load = */ true,
-      base::Bind(
-          [](bool* result_holder, bool result) {
-            *result_holder = result;
-          },
-          &proceed));
-
-  // The interstitial should not appear, and the callback should have been
-  // called immediately.
-  EXPECT_FALSE(ShownPageIsInterstitial(web_contents));
-  EXPECT_TRUE(proceed);
-}
-
 IN_PROC_BROWSER_TEST_F(SupervisedUserTest, GoBackOnDontProceed) {
   // We start out at the initial navigation.
   WebContents* web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
   ASSERT_EQ(0, web_contents->GetController().GetCurrentEntryIndex());
 
   GURL test_url("http://www.example.com/simple.html");
   ui_test_utils::NavigateToURL(browser(), test_url);
 
   ASSERT_FALSE(ShownPageIsInterstitial(web_contents));
@@ skipping 38 matching lines @@
       browser()->tab_strip_model()->GetActiveWebContents();
 
   ASSERT_FALSE(ShownPageIsInterstitial(web_contents));
 
   // Set the host as blocked and wait for the interstitial to appear.
   auto dict = base::MakeUnique<base::DictionaryValue>();
   dict->SetBooleanWithoutPathExpansion(test_url.host(), false);
   SupervisedUserSettingsService* supervised_user_settings_service =
       SupervisedUserSettingsServiceFactory::GetForProfile(
           browser()->profile());
-  auto message_loop_runner = make_scoped_refptr(new content::MessageLoopRunner);
-  InterstitialPageObserver observer(web_contents,
-                                    message_loop_runner->QuitClosure());
+  base::RunLoop run_loop;
+  InterstitialPageObserver observer(web_contents, run_loop.QuitClosure());
   supervised_user_settings_service->SetLocalSetting(
       supervised_users::kContentPackManualBehaviorHosts, std::move(dict));
 
   const SupervisedUserURLFilter* filter =
       supervised_user_service_->GetURLFilter();
   ASSERT_EQ(SupervisedUserURLFilter::BLOCK,
             filter->GetFilteringBehaviorForURL(test_url));
 
-  message_loop_runner->Run();
+  content::RunThisRunLoop(&run_loop);
   ASSERT_TRUE(ShownPageIsInterstitial(web_contents));
 
   dict = base::MakeUnique<base::DictionaryValue>();
   dict->SetBooleanWithoutPathExpansion(test_url.host(), true);
   supervised_user_settings_service->SetLocalSetting(
       supervised_users::kContentPackManualBehaviorHosts, std::move(dict));
   ASSERT_EQ(SupervisedUserURLFilter::ALLOW,
             filter->GetFilteringBehaviorForURL(test_url));
 
   ASSERT_EQ(test_url, web_contents->GetURL());
@@ skipping 26 matching lines @@
   const SupervisedUserURLFilter* filter =
       supervised_user_service_->GetURLFilter();
   EXPECT_EQ(SupervisedUserURLFilter::ALLOW,
             filter->GetFilteringBehaviorForURL(test_url.GetWithEmptyPath()));
 
   observer.Wait();
   EXPECT_EQ(test_url, web_contents->GetURL());
 }
 
 }  // namespace