Added mojo interface for supervised user commands.

Added mojo interface to handle interstitial commands and implemented it
in SupervisedUserNavigationObserver which passes the commands to
SupervisedUserInterstitial when committed interstitials are enabled.

In a future CL an extra class will be added which will listen to JS
commands from the interstitial and passes them via mojo to the
observer.

Bug: 817099
Change-Id: I5285e1fdaa16ee4e96ec7167d470b8ada9b73685
Reviewed-on: https://chromium-review.googlesource.com/950413
Commit-Queue: Carlos IL <carlosil@chromium.org>
Reviewed-by: Emily Stark <estark@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542848}

chrome/browser/BUILD.gn

@@ -4033,6 +4033,7 @@ jumbo_split_static_library("browser") {
       "supervised_user/supervised_user_whitelist_service.h",
       "supervised_user/supervised_users.h",
     ]
+    deps += [ "//chrome/common:supervised_user_commands_mojom" ]
   }
   if (enable_supervised_users && !is_android) {
     sources += [

chrome/browser/supervised_user/supervised_user_interstitial.h

@@ -53,6 +53,12 @@ class SupervisedUserInterstitial : public content::InterstitialPageDelegate,
       Profile* profile,
       supervised_user_error_page::FilteringBehaviorReason reason);
 
+  // InterstitialPageDelegate implementation. This method was made public while
+  // both committed and non-committed interstitials are supported. Once
+  // committed interstitials are the only codepath, this method will be removed
+  // and replaced with separate handlers for go back and request permission.
+  void CommandReceived(const std::string& command) override;
+
  private:
   SupervisedUserInterstitial(
       content::WebContents* web_contents,
@@ -65,7 +71,6 @@ class SupervisedUserInterstitial : public content::InterstitialPageDelegate,
 
   // InterstitialPageDelegate implementation.
   std::string GetHTMLContents() override;
-  void CommandReceived(const std::string& command) override;
   void OnProceed() override;
   void OnDontProceed() override;
   content::InterstitialPageDelegate::TypeID GetTypeForTesting() const override;

chrome/browser/supervised_user/supervised_user_navigation_observer.cc

@@ -34,7 +34,9 @@ SupervisedUserNavigationObserver::~SupervisedUserNavigationObserver() {
 
 SupervisedUserNavigationObserver::SupervisedUserNavigationObserver(
     content::WebContents* web_contents)
-    : content::WebContentsObserver(web_contents), weak_ptr_factory_(this) {
+    : content::WebContentsObserver(web_contents),
+      binding_(web_contents, this),
+      weak_ptr_factory_(this) {
   Profile* profile =
       Profile::FromBrowserContext(web_contents->GetBrowserContext());
   supervised_user_service_ =
@@ -48,6 +50,7 @@ void SupervisedUserNavigationObserver::OnRequestBlocked(
     content::WebContents* web_contents,
     const GURL& url,
     supervised_user_error_page::FilteringBehaviorReason reason,
+    int64_t navigation_id,
     const base::Callback<
         void(SupervisedUserNavigationThrottle::CallbackActions)>& callback) {
   SupervisedUserNavigationObserver* navigation_observer =
@@ -60,24 +63,31 @@ void SupervisedUserNavigationObserver::OnRequestBlocked(
     return;
   }
 
-  navigation_observer->OnRequestBlockedInternal(url, reason, callback);
+  navigation_observer->OnRequestBlockedInternal(url, reason, navigation_id,
+                                                callback);
 }
 
 void SupervisedUserNavigationObserver::DidFinishNavigation(
       content::NavigationHandle* navigation_handle) {
+  // With committed interstitials on, if this is a different navigation than the
+  // one that triggered the interstitial, clear is_showing_interstitial_
+  if (is_showing_interstitial_ &&
+      navigation_handle->GetNavigationId() != interstitial_navigation_id_ &&
+      base::FeatureList::IsEnabled(
+          features::kSupervisedUserCommittedInterstitials)) {
+    is_showing_interstitial_ = false;
+  }
+
   // Only filter same page navigations (eg. pushState/popState); others will
   // have been filtered by the NavigationThrottle.
-  if (!navigation_handle->IsSameDocument())
-    return;
-
-  if (!navigation_handle->IsInMainFrame())
-    return;
-
-  url_filter_->GetFilteringBehaviorForURLWithAsyncChecks(
-      web_contents()->GetLastCommittedURL(),
-      base::BindOnce(&SupervisedUserNavigationObserver::URLFilterCheckCallback,
-                     weak_ptr_factory_.GetWeakPtr(),
-                     navigation_handle->GetURL()));
+  if (navigation_handle->IsSameDocument() &&
+      navigation_handle->IsInMainFrame()) {
+    url_filter_->GetFilteringBehaviorForURLWithAsyncChecks(
+        web_contents()->GetLastCommittedURL(),
+        base::BindOnce(
+            &SupervisedUserNavigationObserver::URLFilterCheckCallback,
+            weak_ptr_factory_.GetWeakPtr(), navigation_handle->GetURL()));
+  }
 }
 
 void SupervisedUserNavigationObserver::OnURLFilterChanged() {
@@ -91,6 +101,7 @@ void SupervisedUserNavigationObserver::OnURLFilterChanged() {
 void SupervisedUserNavigationObserver::OnRequestBlockedInternal(
     const GURL& url,
     supervised_user_error_page::FilteringBehaviorReason reason,
+    int64_t navigation_id,
     const base::Callback<
         void(SupervisedUserNavigationThrottle::CallbackActions)>& callback) {
   // TODO(bauerb): Use SaneTime when available.
@@ -128,7 +139,8 @@ void SupervisedUserNavigationObserver::OnRequestBlockedInternal(
 
   // Show the interstitial.
   const bool initial_page_load = true;
-  MaybeShowInterstitial(url, reason, initial_page_load, callback);
+  MaybeShowInterstitial(url, reason, initial_page_load, navigation_id,
+                        callback);
 }
 
 void SupervisedUserNavigationObserver::URLFilterCheckCallback(
@@ -141,14 +153,17 @@ void SupervisedUserNavigationObserver::URLFilterCheckCallback(
     return;
 
   if (!is_showing_interstitial_ &&
-      behavior == SupervisedUserURLFilter::FilteringBehavior::BLOCK) {
-    // TODO(carlosil): Handle this case for committed interstitials. For this we
-    // will check if the current page is an error page since
-    // is_showing_interstitial_ does not get reset when navigating through the
-    // back button.
+      behavior == SupervisedUserURLFilter::FilteringBehavior::BLOCK &&
+      !base::FeatureList::IsEnabled(
+          features::kSupervisedUserCommittedInterstitials)) {
+    // TODO(carlosil): Handle this case for committed interstitials. For now, we
+    // pass a 0 as the navigation id causing the interstitial for this case
+    // since we don't have the real id here, this doesn't cause issues since the
+    // navigation id is not used when committed interstitials are not enabled.
+    // This will be removed once committed interstitials are the only code path.
     const bool initial_page_load = false;
     MaybeShowInterstitial(
-        url, reason, initial_page_load,
+        url, reason, initial_page_load, 0,
         base::Callback<void(
             SupervisedUserNavigationThrottle::CallbackActions)>());
   }
@@ -158,8 +173,10 @@ void SupervisedUserNavigationObserver::MaybeShowInterstitial(
     const GURL& url,
     supervised_user_error_page::FilteringBehaviorReason reason,
     bool initial_page_load,
+    int64_t navigation_id,
     const base::Callback<
         void(SupervisedUserNavigationThrottle::CallbackActions)>& callback) {
+  interstitial_navigation_id_ = navigation_id;
   is_showing_interstitial_ = true;
   base::Callback<void(bool)> wrapped_callback =
       base::Bind(&SupervisedUserNavigationObserver::OnInterstitialResult,
@@ -190,3 +207,24 @@ void SupervisedUserNavigationObserver::OnInterstitialResult(
                         : SupervisedUserNavigationThrottle::CallbackActions::
                               kCancelNavigation);
 }
+
+void SupervisedUserNavigationObserver::GoBack() {
+  DCHECK(base::FeatureList::IsEnabled(
+      features::kSupervisedUserCommittedInterstitials));
+  if (interstitial_ && is_showing_interstitial_)
+    interstitial_->CommandReceived("\"back\"");
+}
+
+void SupervisedUserNavigationObserver::RequestPermission() {
+  DCHECK(base::FeatureList::IsEnabled(
+      features::kSupervisedUserCommittedInterstitials));
+  if (interstitial_ && is_showing_interstitial_)
+    interstitial_->CommandReceived("\"request\"");
+}
+
+void SupervisedUserNavigationObserver::Feedback() {
+  DCHECK(base::FeatureList::IsEnabled(
+      features::kSupervisedUserCommittedInterstitials));
+  if (interstitial_ && is_showing_interstitial_)
+    interstitial_->CommandReceived("\"feedback\"");
+}

chrome/browser/supervised_user/supervised_user_navigation_observer.h

@@ -12,8 +12,10 @@
 #include "chrome/browser/supervised_user/supervised_user_service_observer.h"
 #include "chrome/browser/supervised_user/supervised_user_url_filter.h"
 #include "chrome/browser/supervised_user/supervised_users.h"
+#include "chrome/common/supervised_user_commands.mojom.h"
 #include "components/sessions/core/serialized_navigation_entry.h"
 #include "components/supervised_user_error_page/supervised_user_error_page.h"
+#include "content/public/browser/web_contents_binding_set.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_user_data.h"
 
@@ -28,7 +30,8 @@ class WebContents;
 class SupervisedUserNavigationObserver
     : public content::WebContentsUserData<SupervisedUserNavigationObserver>,
       public content::WebContentsObserver,
-      public SupervisedUserServiceObserver {
+      public SupervisedUserServiceObserver,
+      public supervised_user::mojom::SupervisedUserCommands {
  public:
   ~SupervisedUserNavigationObserver() override;
 
@@ -42,6 +45,7 @@ class SupervisedUserNavigationObserver
       content::WebContents* web_contents,
       const GURL& url,
       supervised_user_error_page::FilteringBehaviorReason reason,
+      int64_t navigation_id,
       const base::Callback<
           void(SupervisedUserNavigationThrottle::CallbackActions)>& callback);
 
@@ -60,6 +64,7 @@ class SupervisedUserNavigationObserver
   void OnRequestBlockedInternal(
       const GURL& url,
       supervised_user_error_page::FilteringBehaviorReason reason,
+      int64_t navigation_id,
       const base::Callback<
           void(SupervisedUserNavigationThrottle::CallbackActions)>& callback);
 
@@ -73,6 +78,7 @@ class SupervisedUserNavigationObserver
       const GURL& url,
       supervised_user_error_page::FilteringBehaviorReason reason,
       bool initial_page_load,
+      int64_t navigation_id,
       const base::Callback<
           void(SupervisedUserNavigationThrottle::CallbackActions)>& callback);
 
@@ -81,12 +87,22 @@ class SupervisedUserNavigationObserver
           void(SupervisedUserNavigationThrottle::CallbackActions)>& callback,
       bool result);
 
+  // supervised_user::mojom::SupervisedUserCommands implementation. Should not
+  // be called when an interstitial is no longer showing. This should be
+  // enforced by the mojo caller.
+  void GoBack() override;
+  void RequestPermission() override;
+  void Feedback() override;
+
   // Owned by SupervisedUserService.
   const SupervisedUserURLFilter* url_filter_;
 
   // Owned by SupervisedUserServiceFactory (lifetime of Profile).
   SupervisedUserService* supervised_user_service_;
 
+  // Navigation ID of the navigation that triggered the last interstitial.
+  int64_t interstitial_navigation_id_;
+
   bool is_showing_interstitial_ = false;
 
   std::vector<std::unique_ptr<const sessions::SerializedNavigationEntry>>
@@ -94,6 +110,10 @@ class SupervisedUserNavigationObserver
 
   std::unique_ptr<SupervisedUserInterstitial> interstitial_;
 
+  content::WebContentsFrameBindingSet<
+      supervised_user::mojom::SupervisedUserCommands>
+      binding_;
+
   base::WeakPtrFactory<SupervisedUserNavigationObserver> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(SupervisedUserNavigationObserver);

chrome/browser/supervised_user/supervised_user_navigation_throttle.cc

@@ -187,7 +187,7 @@ void SupervisedUserNavigationThrottle::ShowInterstitialAsync(
   DCHECK(deferred_);
   SupervisedUserNavigationObserver::OnRequestBlocked(
       navigation_handle()->GetWebContents(), navigation_handle()->GetURL(),
-      reason,
+      reason, navigation_handle()->GetNavigationId(),
       base::Bind(&SupervisedUserNavigationThrottle::OnInterstitialResult,
                  weak_ptr_factory_.GetWeakPtr()));
 }

chrome/common/BUILD.gn

@@ -210,6 +210,7 @@ static_library("common") {
     ":ini_parser",
     ":mojo_bindings",
     ":page_load_metrics_mojom",
+    ":supervised_user_commands_mojom",
     "//base:base",
     "//base:base_static",
     "//base:i18n",
@@ -755,3 +756,9 @@ if (enable_print_preview && !is_chromeos) {
     ]
   }
 }
+
+mojom("supervised_user_commands_mojom") {
+  sources = [
+    "supervised_user_commands.mojom",
+  ]
+}

chrome/common/supervised_user_commands.mojom

+// Copyright 2018 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.
+
+module supervised_user.mojom;
+
+// Used to deliver commands from the supervised user interstitial when
+// committed interstitials are enabled.
+interface SupervisedUserCommands {
+  // Go back to the previous page.
+  GoBack();
+  // Request permission from supervised user administrator to view the current
+  // URL.
+  RequestPermission();
+  // Send feedback about the decision to block the current URL.
+  Feedback();
+};
\ No newline at end of file