Move SecurityInterstitialPage into component

components/security_interstitials/content/security_interstitial_controller_client.h

Index: components/security_interstitials/content/security_interstitial_controller_client.h
diff --git a/components/security_interstitials/content/security_interstitial_controller_client.h b/components/security_interstitials/content/security_interstitial_controller_client.h
new file mode 100644
index 0000000000000000000000000000000000000000..bd9c62adc7f721419d2c6e87b6d042ee420d593a
--- /dev/null
+++ b/components/security_interstitials/content/security_interstitial_controller_client.h
@@ -0,0 +1,71 @@
+// Copyright 2016 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_SECURITY_INTERSTITIALS_CONTENT_SECURITY_INTERSTITIAL_CONTROLLER_CLIENT_H_
+#define COMPONENTS_SECURITY_INTERSTITIALS_CONTENT_SECURITY_INTERSTITIAL_CONTROLLER_CLIENT_H_
+
+#include "base/macros.h"
+#include "components/security_interstitials/core/controller_client.h"
+
+namespace content {
+  class InterstitialPage;
+  class WebContents;
+}
+
+namespace security_interstitials {
+
+class MetricsHelper;
+
+// Handle commands from security interstitial pages. This class is instantiated
+// by SafeBrowsingBlockingPage.

[meacer, 2016/12/27 23:22:34]

Is the class really only instantiated by SafeBrowsingBlockingPage? Could other
interstitials also use it? (e.g. SSL)

[Jialiu Lin, 2016/12/28 18:30:00]

Yes, SSLBlockingPage and CaptivePortalBlockingPage can use
SecurityInterstitialControllerClient too in theory. 
However, I'm still in the middle of the refactoring and there will be more
changes to SafeBrowsingBlockingPage and SecurityInterstitialControllerClient. At
this moment, it might be safer for SSLBlockingPage and other interstitial pages
to continue use ChromeControllerClient. When everything settles, I'll see which
way is better. 

[meacer, 2016/12/28 19:02:22]

Sounds good. Can you add a comment or a todo saying this should only be used by
SafeBrowsingBlockingPage for the time being?

[Jialiu Lin, 2016/12/28 19:40:32]

Sure. Refined comments and added a todo.

+class SecurityInterstitialControllerClient
+    : public security_interstitials::ControllerClient {
+ public:
+  SecurityInterstitialControllerClient(
+      content::WebContents* web_contents,
+      std::unique_ptr<MetricsHelper> metrics_helper,
+      const GURL& default_safe_page);

[meacer, 2016/12/27 23:22:34]

As I commented in chrome_controller_client.cc, looks like you might not need
this constructor.

[Jialiu Lin, 2016/12/28 18:30:00]

Done.

+
+  SecurityInterstitialControllerClient(
+      content::WebContents* web_contents,
+      std::unique_ptr<MetricsHelper> metrics_helper,
+      PrefService* prefs,
+      const std::string& app_locale,
+      const GURL& default_safe_page);
+
+  ~SecurityInterstitialControllerClient() override;
+
+  void set_interstitial_page(content::InterstitialPage* interstitial_page);
+  content::InterstitialPage* interstitial_page();
+
+  // security_interstitials::ControllerClient overrides

[meacer, 2016/12/27 23:22:34]

nit: Missing dot or colon at the end of the line

[Jialiu Lin, 2016/12/28 18:30:00]

Done.

+  void GoBack() override;
+  void GoBackAfterNavigationCommitted() override;
+  void Proceed() override;
+  void Reload() override;
+  void OpenUrlInCurrentTab(const GURL& url) override;
+  PrefService* GetPrefService() override;
+  const std::string& GetApplicationLocale() override;
+  bool CanLaunchDateAndTimeSettings() override;
+  void LaunchDateAndTimeSettings() override;
+
+protected:
+  // security_interstitials::ControllerClient overrides
+  const std::string GetExtendedReportingPrefName() override;

[meacer, 2016/12/27 23:22:34]

This could be a const method in security_interstitials::ControllerClient.

[meacer, 2016/12/27 23:22:34]

nit: Missing dot or colon at the end of the line

[Jialiu Lin, 2016/12/28 18:30:00]

Done.

[Jialiu Lin, 2016/12/28 18:30:00]

Done.

+  content::WebContents* web_contents_;
+
+ private:
+  content::InterstitialPage* interstitial_page_;
+  PrefService* prefs_;
+  const std::string app_locale_;
+  // The default safe page we should go to if there is no previous page to go
+  // back to, e.g. chrome:kChromeUINewTabURL.
+  const GURL& default_safe_page_;

[meacer, 2016/12/27 23:22:34]

I think you can make this a non-reference member:
const GURL default_safe_page_;

[Jialiu Lin, 2016/12/28 18:30:00]

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(SecurityInterstitialControllerClient);
+};
+
+}  // namespace security_interstitials
+
+#endif  // COMPONENTS_SECURITY_INTERSTITIALS_CONTENT_SECURITY_INTERSTITIAL_CONTROLLER_CLIENT_H_