Componentize SafeBrowsingBlockingPage for WebView use

components/safe_browsing/base_blocking_page.h

Index: components/safe_browsing/base_blocking_page.h
diff --git a/components/safe_browsing/base_blocking_page.h b/components/safe_browsing/base_blocking_page.h
new file mode 100644
index 0000000000000000000000000000000000000000..cb81d56279b25fc26f053df32db33f35f520e9a3
--- /dev/null
+++ b/components/safe_browsing/base_blocking_page.h
@@ -0,0 +1,139 @@
+// Copyright 2017 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.
+
+#ifndef COMPONENTS_SAFE_BROWSING_BASE_BLOCKING_PAGE_H_
+#define COMPONENTS_SAFE_BROWSING_BASE_BLOCKING_PAGE_H_
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "base/macros.h"
+#include "components/safe_browsing/base_ui_manager.h"
+#include "components/security_interstitials/content/security_interstitial_page.h"
+#include "components/security_interstitials/core/safe_browsing_error_ui.h"
+#include "content/public/browser/interstitial_page_delegate.h"
+#include "url/gurl.h"
+
+namespace history {
+class HistoryService;
+}
+
+namespace safe_browsing {
+
+// Base class for managing the SafeBrowsing interstitial pages.
+class BaseBlockingPage
+    : public security_interstitials::SecurityInterstitialPage {
+ public:
+  typedef security_interstitials::UnsafeResource UnsafeResource;
+  typedef security_interstitials::SafeBrowsingErrorUI SafeBrowsingErrorUI;
+  typedef std::vector<UnsafeResource> UnsafeResourceList;
+  typedef std::unordered_map<content::WebContents*, UnsafeResourceList>
+      UnsafeResourceMap;
+
+  ~BaseBlockingPage() override;
+
+  // Creates a blocking page. Use ShowBlockingPage if you don't need to access
+  // the blocking page directly.
+  static BaseBlockingPage* CreateBlockingPage(

[meacer, 2017/01/11 01:33:31]

This seems to be called only from BaseBlockingPage::ShowBlockingPage, can it be
private? Or maybe you can call the constructor directly and remove this method?

[Jialiu Lin, 2017/01/11 02:53:15]

Removed this function and directly call constructor in ShowBlowckingPage(..)

+      BaseUIManager* ui_manager,
+      content::WebContents* web_contents,
+      const GURL& main_frame_url,
+      const UnsafeResource& unsafe_resource);
+
+  // Shows a blocking page warning the user about phishing/malware for a
+  // specific resource.
+  // You can call this method several times, if an interstitial is already

[meacer, 2017/01/11 01:33:31]

tiny nit: might want to reword this sentence a bit since it has two if
conditions:

"This can be called several times. If an interstitial is already showing and the
user decides to proceed, it will be discarded and a new one will displayed"

[Jialiu Lin, 2017/01/11 02:53:16]

Done.

+  // showing, the new one will be queued and displayed if the user decides
+  // to proceed on the currently showing interstitial.
+  static void ShowBlockingPage(BaseUIManager* ui_manager,
+                               const UnsafeResource& resource);
+
+  // Returns true if the passed |unsafe_resources| is blocking the load of
+  // the main page.
+  static bool IsMainPageLoadBlocked(const UnsafeResourceList& unsafe_resources);
+
+  // InterstitialPageDelegate method:

[meacer, 2017/01/11 01:33:31]

nit: methods -> methods?

[Jialiu Lin, 2017/01/11 02:53:16]

Done.

+  void OnProceed() override;
+  void OnDontProceed() override;
+  void CommandReceived(const std::string& command) override;
+
+ protected:
+  // Don't instantiate this class directly, use ShowBlockingPage instead.
+  BaseBlockingPage(
+      BaseUIManager* ui_manager,
+      content::WebContents* web_contents,
+      const GURL& main_frame_url,
+      const UnsafeResourceList& unsafe_resources,
+      std::unique_ptr<
+          security_interstitials::SecurityInterstitialControllerClient>
+          controller_client,
+      SafeBrowsingErrorUI::SBErrorDisplayOptions* display_options);
+
+  // SecurityInterstitialPage methods:
+  bool ShouldCreateNewNavigation() const override;
+  void PopulateInterstitialStrings(
+      base::DictionaryValue* load_time_data) override;
+
+  // Called when the interstitial is going away. Intentionally do nothing in
+  // this base class. Overrided by SafeBrowsingBlockingPage class.

[meacer, 2017/01/11 01:33:31]

nit: Overrided -> Overridden (though I think only keeping the first sentence is
sufficient)

[Jialiu Lin, 2017/01/11 02:53:16]

Done.

+  virtual void FinishThreatDetails(int64_t delay_ms,
+                                   bool did_proceed,
+                                   int num_visits);
+
+  // A list of SafeBrowsingUIManager::UnsafeResource for a tab that the user
+  // should be warned about.  They are queued when displaying more than one

[meacer, 2017/01/11 01:33:31]

nit: Double spaces after period

[Jialiu Lin, 2017/01/11 02:53:16]

Done.

+  // interstitial at a time.
+  static UnsafeResourceMap* GetUnsafeResourcesMap();
+
+  static std::string GetMetricPrefix(
+      const UnsafeResourceList& unsafe_resources,
+      SafeBrowsingErrorUI::SBInterstitialReason interstitial_reason);
+
+  static std::string GetExtraMetricsSuffix(
+      const UnsafeResourceList& unsafe_resources);
+
+  static SafeBrowsingErrorUI::SBInterstitialReason GetInterstitialReason(
+      const UnsafeResourceList& unsafe_resources);
+
+  // For reporting back user actions.
+  BaseUIManager* ui_manager_;

[meacer, 2017/01/11 01:33:31]

In general fields shouldn't directly made available to subclasses
(https://google.github.io/styleguide/cppguide.html#Access_Control).

So you can move them to the private section and add protected getters for them.

[Jialiu Lin, 2017/01/11 02:53:16]

Done.

+
+  // The URL of the main frame that caused the warning.
+  GURL main_frame_url_;
+
+  // The index of a navigation entry that should be removed when DontProceed()
+  // is invoked, -1 if entry should not be removed.
+  const int navigation_entry_index_to_remove_;
+
+  // The list of unsafe resources this page is warning about.
+  UnsafeResourceList unsafe_resources_;
+
+  // For displaying safe browsing interstitial.
+  std::unique_ptr<SafeBrowsingErrorUI> sb_error_ui_;
+
+  // Indicate whether user has proceeded this blocking page.
+  bool proceeded_;
+
+  // Which type of Safe Browsing interstitial this is.
+  SafeBrowsingErrorUI::SBInterstitialReason interstitial_reason_;
+
+ private:
+  static std::unique_ptr<
+      security_interstitials::SecurityInterstitialControllerClient>
+  CreateControllerClient(content::WebContents* web_contents,
+                         const UnsafeResourceList& unsafe_resources,
+                         history::HistoryService* history_service,
+                         const std::string& app_locale,
+                         const GURL& default_safe_page);
+
+  static SafeBrowsingErrorUI::SBErrorDisplayOptions*
+  CreateDefaultDisplayOptions(const UnsafeResourceList& unsafe_resources);
+
+  DISALLOW_COPY_AND_ASSIGN(BaseBlockingPage);
+};
+
+}  // namespace safe_browsing
+
+#endif  // COMPONENTS_SAFE_BROWSING_BASE_BLOCKING_PAGE_H_