Move SecurityInterstitialPage into component

Move SecurityInterstitialPage into components/security_interstitial/content.
And resolve all the dependencies and tests and so on.

BUG=666100
Review-Url: https://codereview.chromium.org/2603433002
Cr-Commit-Position: refs/heads/master@{#441701}
Committed: https://chromium.googlesource.com/chromium/src/+/a2c0692563f58517c32e839312695e91f45971a3

[Jialiu Lin, 2016-12-22 18:54:56]

The CQ bit was checked by jialiul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-22 18:55:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-22 18:57:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-22 18:57:22]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[Jialiu Lin, 2016-12-22 19:58:40]

The CQ bit was checked by jialiul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-22 19:58:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-22 20:03:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-22 20:03:30]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Jialiu Lin, 2016-12-22 20:42:15]

The CQ bit was checked by jialiul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-22 20:42:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Jialiu Lin, 2016-12-22 22:14:48]

The CQ bit was checked by jialiul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-22 22:14:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Jialiu Lin, 2016-12-22 22:20:49]

Patchset #1 (id:1) has been deleted

[Jialiu Lin, 2016-12-22 22:21:01]

Patchset #1 (id:20001) has been deleted

[Jialiu Lin, 2016-12-22 22:22:52]

Description was changed from

==========
Draft CL Don't Submit
move security_interstitial_page into component

BUG=
==========

to

==========
Move SecurityInterstitialPage into components/security_interstitial/content

BUG=666100
==========

[Jialiu Lin, 2016-12-22 22:23:21]

Description was changed from

==========
Move SecurityInterstitialPage into components/security_interstitial/content

BUG=666100
==========

to

==========
Move SecurityInterstitialPage into components/security_interstitial/content.
And resolve all the dependencies and tests and so on.

BUG=666100
==========

[Jialiu Lin, 2016-12-27 22:26:21]

jialiul@chromium.org changed reviewers:
+ meacer@chromium.org

[Jialiu Lin, 2016-12-27 22:26:22]

Hi meacer@, 
Could you take a look at this componentization/refactoring CL?  Thanks!

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

Looking good, some questions and nits.

https://codereview.chromium.org/2603433002/diff/80001/chrome/browser/intersti...
File chrome/browser/interstitials/chrome_controller_client.cc (right):

https://codereview.chromium.org/2603433002/diff/80001/chrome/browser/intersti...
chrome/browser/interstitials/chrome_controller_client.cc:159: return
profile->GetPrefs();
Seems like you can get rid of these two overrides by calling the other
SecurityInterstitialControllerClient constructor and passing these values:

ChromeControllerClient::ChromeControllerClient(...)
  : SecurityInterstitialControllerClient(
        web_contents, std::move(metrics_helper),
       
Profile::FromBrowserContext(web_contents->GetBrowserContext())->GetPrefs(),
        g_browser_process->GetApplicationLocale(),
        GURL(chrome::kChromeUINewTabURL)) {}

And then perhaps you might not need the other
SecurityInterstitialControllerClient constructor? (i.e. the one that this class
currently calls)

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
File
components/security_interstitials/content/security_interstitial_controller_client.cc
(right):

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
components/security_interstitials/content/security_interstitial_controller_client.cc:61:
// commits.
nit: This comment seems to be specific to the implementation, so perhaps move
inside the method?

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
File
components/security_interstitials/content/security_interstitial_controller_client.h
(right):

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
components/security_interstitials/content/security_interstitial_controller_client.h:21:
// by SafeBrowsingBlockingPage.
Is the class really only instantiated by SafeBrowsingBlockingPage? Could other
interstitials also use it? (e.g. SSL)

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
components/security_interstitials/content/security_interstitial_controller_client.h:28:
const GURL& default_safe_page);
As I commented in chrome_controller_client.cc, looks like you might not need
this constructor.

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
components/security_interstitials/content/security_interstitial_controller_client.h:42:
// security_interstitials::ControllerClient overrides
nit: Missing dot or colon at the end of the line

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
components/security_interstitials/content/security_interstitial_controller_client.h:55:
const std::string GetExtendedReportingPrefName() override;
nit: Missing dot or colon at the end of the line

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
components/security_interstitials/content/security_interstitial_controller_client.h:55:
const std::string GetExtendedReportingPrefName() override;
This could be a const method in security_interstitials::ControllerClient.

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
components/security_interstitials/content/security_interstitial_controller_client.h:64:
const GURL& default_safe_page_;
I think you can make this a non-reference member:
const GURL default_safe_page_;

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
File components/security_interstitials/core/controller_client.h (right):

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
components/security_interstitials/core/controller_client.h:92: virtual const
std::string& GetApplicationLocale() = 0;
nit: const method

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
components/security_interstitials/core/controller_client.h:95: virtual const
std::string GetExtendedReportingPrefName() = 0;
const method

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
File components/security_interstitials/core/safe_browsing_error_ui.h (right):

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
components/security_interstitials/core/safe_browsing_error_ui.h:98: const
std::string app_locale() {
nit: These can all be const methods

[Jialiu Lin, 2016-12-28 18:26:33]

The CQ bit was checked by jialiul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-28 18:26:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Jialiu Lin, 2016-12-28 18:30:01]

Thanks, meacer@!  All your comments/questions are addressed.

https://codereview.chromium.org/2603433002/diff/80001/chrome/browser/intersti...
File chrome/browser/interstitials/chrome_controller_client.cc (right):

https://codereview.chromium.org/2603433002/diff/80001/chrome/browser/intersti...
chrome/browser/interstitials/chrome_controller_client.cc:159: return
profile->GetPrefs();
On 2016/12/27 23:22:34, Mustafa Emre Acer wrote:
> Seems like you can get rid of these two overrides by calling the other
> SecurityInterstitialControllerClient constructor and passing these values:
> 
> ChromeControllerClient::ChromeControllerClient(...)
>   : SecurityInterstitialControllerClient(
>         web_contents, std::move(metrics_helper),
>        
> Profile::FromBrowserContext(web_contents->GetBrowserContext())->GetPrefs(),
>         g_browser_process->GetApplicationLocale(),
>         GURL(chrome::kChromeUINewTabURL)) {}
> 
> And then perhaps you might not need the other
> SecurityInterstitialControllerClient constructor? (i.e. the one that this
class
> currently calls)

Yes, that's right. Done.

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
File
components/security_interstitials/content/security_interstitial_controller_client.cc
(right):

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
components/security_interstitials/content/security_interstitial_controller_client.cc:61:
// commits.
On 2016/12/27 23:22:34, Mustafa Emre Acer wrote:
> nit: This comment seems to be specific to the implementation, so perhaps move
> inside the method?

Done.

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
File
components/security_interstitials/content/security_interstitial_controller_client.h
(right):

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
components/security_interstitials/content/security_interstitial_controller_client.h:21:
// by SafeBrowsingBlockingPage.
On 2016/12/27 23:22:34, Mustafa Emre Acer wrote:
> Is the class really only instantiated by SafeBrowsingBlockingPage? Could other
> interstitials also use it? (e.g. SSL)

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.

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
components/security_interstitials/content/security_interstitial_controller_client.h:28:
const GURL& default_safe_page);
On 2016/12/27 23:22:34, Mustafa Emre Acer wrote:
> As I commented in chrome_controller_client.cc, looks like you might not need
> this constructor.

Done.

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
components/security_interstitials/content/security_interstitial_controller_client.h:42:
// security_interstitials::ControllerClient overrides
On 2016/12/27 23:22:34, Mustafa Emre Acer wrote:
> nit: Missing dot or colon at the end of the line

Done.

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
components/security_interstitials/content/security_interstitial_controller_client.h:55:
const std::string GetExtendedReportingPrefName() override;
On 2016/12/27 23:22:34, Mustafa Emre Acer wrote:
> nit: Missing dot or colon at the end of the line

Done.

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
components/security_interstitials/content/security_interstitial_controller_client.h:55:
const std::string GetExtendedReportingPrefName() override;
On 2016/12/27 23:22:34, Mustafa Emre Acer wrote:
> This could be a const method in security_interstitials::ControllerClient.

Done.

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
components/security_interstitials/content/security_interstitial_controller_client.h:64:
const GURL& default_safe_page_;
On 2016/12/27 23:22:34, Mustafa Emre Acer wrote:
> I think you can make this a non-reference member:
> const GURL default_safe_page_;

Done.

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
File components/security_interstitials/core/controller_client.h (right):

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
components/security_interstitials/core/controller_client.h:92: virtual const
std::string& GetApplicationLocale() = 0;
On 2016/12/27 23:22:34, Mustafa Emre Acer wrote:
> nit: const method

Done.

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
components/security_interstitials/core/controller_client.h:95: virtual const
std::string GetExtendedReportingPrefName() = 0;
On 2016/12/27 23:22:34, Mustafa Emre Acer wrote:
> const method

Done.

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
File components/security_interstitials/core/safe_browsing_error_ui.h (right):

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
components/security_interstitials/core/safe_browsing_error_ui.h:98: const
std::string app_locale() {
On 2016/12/27 23:22:34, Mustafa Emre Acer wrote:
> nit: These can all be const methods

Indeed. Done.

[Jialiu Lin, 2016-12-28 18:33:14]

jialiul@chromium.org changed reviewers:
+ mmenke@chromium.org, tnagel@chromium.org

[Jialiu Lin, 2016-12-28 18:33:15]

+ mmenke@chromium.org: Please review changes in c/b/captive_portal

+ tnagel@chromium.org: Please review changes in c/b/policy


Thanks!

[commit-bot: I haz the power, 2016-12-28 18:33:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-28 18:33:33]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[mmenke, 2016-12-28 18:42:46]

On 2016/12/28 18:33:15, Jialiu Lin wrote:
> + mailto:mmenke@chromium.org: Please review changes in c/b/captive_portal
> 
> + mailto:tnagel@chromium.org: Please review changes in c/b/policy
> 
> 
> Thanks!

chrome/browser/captive_portal LGTM.

[Jialiu Lin, 2016-12-28 18:44:59]

The CQ bit was checked by jialiul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-28 18:45:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Jialiu Lin, 2016-12-28 18:52:31]

jialiul@chromium.org changed reviewers:
+ eugenebut@chromium.org

[Jialiu Lin, 2016-12-28 18:52:32]

+ eugenebut@chromium.org Could you take a look at changes in
ios/chrome/browser/*?

Thanks!

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

LGTM

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
File
components/security_interstitials/content/security_interstitial_controller_client.h
(right):

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
components/security_interstitials/content/security_interstitial_controller_client.h:21:
// by SafeBrowsingBlockingPage.
On 2016/12/28 18:30:00, Jialiu Lin wrote:
> On 2016/12/27 23:22:34, Mustafa Emre Acer wrote:
> > Is the class really only instantiated by SafeBrowsingBlockingPage? Could
other
> > interstitials also use it? (e.g. SSL)
> 
> 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. 

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

[Eugene But (OOO till 7-30), 2016-12-28 19:02:52]

lgtm

https://codereview.chromium.org/2603433002/diff/120001/ios/chrome/browser/int...
File ios/chrome/browser/interstitials/ios_chrome_controller_client.h (right):

https://codereview.chromium.org/2603433002/diff/120001/ios/chrome/browser/int...
ios/chrome/browser/interstitials/ios_chrome_controller_client.h:45: PrefService*
GetPrefService() const override;
Should |PrefService*| be also const? What if ControllerClient owns PrefService*
object?

[Jialiu Lin, 2016-12-28 19:24:30]

The CQ bit was checked by jialiul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-28 19:24:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-28 19:28:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-28 19:28:51]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[Jialiu Lin, 2016-12-28 19:39:17]

The CQ bit was checked by jialiul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-28 19:39:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Jialiu Lin, 2016-12-28 19:39:53]

Patchset #6 (id:140001) has been deleted

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

Thanks!

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
File
components/security_interstitials/content/security_interstitial_controller_client.h
(right):

https://codereview.chromium.org/2603433002/diff/80001/components/security_int...
components/security_interstitials/content/security_interstitial_controller_client.h:21:
// by SafeBrowsingBlockingPage.
On 2016/12/28 19:02:22, Mustafa Emre Acer wrote:
> On 2016/12/28 18:30:00, Jialiu Lin wrote:
> > On 2016/12/27 23:22:34, Mustafa Emre Acer wrote:
> > > Is the class really only instantiated by SafeBrowsingBlockingPage? Could
> other
> > > interstitials also use it? (e.g. SSL)
> > 
> > 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. 
> 
> Sounds good. Can you add a comment or a todo saying this should only be used
by
> SafeBrowsingBlockingPage for the time being?

Sure. Refined comments and added a todo.

https://codereview.chromium.org/2603433002/diff/120001/ios/chrome/browser/int...
File ios/chrome/browser/interstitials/ios_chrome_controller_client.h (right):

https://codereview.chromium.org/2603433002/diff/120001/ios/chrome/browser/int...
ios/chrome/browser/interstitials/ios_chrome_controller_client.h:45: PrefService*
GetPrefService() const override;
On 2016/12/28 19:02:52, Eugene But wrote:
> Should |PrefService*| be also const? What if ControllerClient owns
PrefService*
> object?

We cannot do "const PrefService* GetPrefService() const" here ... Since, certain
user actions on interstitial need to modify PrefService (e.g. check/uncheck
extended reporting opt-in checkbox)

[Jialiu Lin, 2016-12-28 19:46:00]

jialiul@chromium.org changed reviewers:
+ nasko@chromium.org, pam@chromium.org

[Jialiu Lin, 2016-12-28 19:46:01]

+nasko@ because I need LGTM from owners of content/public/common in DEPS that
was modified in this CL

+gab@ because I need LGTM from owners of components/prefs in DEPSs that was
modified in this CL

Thanks!

[commit-bot: I haz the power, 2016-12-28 19:47:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-28 19:47:11]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Jialiu Lin, 2016-12-28 19:52:08]

The CQ bit was checked by jialiul@chromium.org to run a CQ dry run

[Jialiu Lin, 2016-12-28 19:52:15]

Patchset #6 (id:160001) has been deleted

[commit-bot: I haz the power, 2016-12-28 19:52:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Eugene But (OOO till 7-30), 2016-12-28 20:01:34]

https://codereview.chromium.org/2603433002/diff/120001/ios/chrome/browser/int...
File ios/chrome/browser/interstitials/ios_chrome_controller_client.h (right):

https://codereview.chromium.org/2603433002/diff/120001/ios/chrome/browser/int...
ios/chrome/browser/interstitials/ios_chrome_controller_client.h:45: PrefService*
GetPrefService() const override;
On 2016/12/28 19:40:32, Jialiu Lin wrote:
> On 2016/12/28 19:02:52, Eugene But wrote:
> > Should |PrefService*| be also const? What if ControllerClient owns
> PrefService*
> > object?
> 
> We cannot do "const PrefService* GetPrefService() const" here ... Since,
certain
> user actions on interstitial need to modify PrefService (e.g. check/uncheck
> extended reporting opt-in checkbox) 
nit: If that's the case then is it fair to make |GetPrefService| method const?
It was not const before, which is reasonable, because returning non-const
PrefService* allows changing the state of ControllerClient.

[Jialiu Lin, 2016-12-28 20:13:40]

https://codereview.chromium.org/2603433002/diff/120001/ios/chrome/browser/int...
File ios/chrome/browser/interstitials/ios_chrome_controller_client.h (right):

https://codereview.chromium.org/2603433002/diff/120001/ios/chrome/browser/int...
ios/chrome/browser/interstitials/ios_chrome_controller_client.h:45: PrefService*
GetPrefService() const override;
On 2016/12/28 20:01:34, Eugene But wrote:
> On 2016/12/28 19:40:32, Jialiu Lin wrote:
> > On 2016/12/28 19:02:52, Eugene But wrote:
> > > Should |PrefService*| be also const? What if ControllerClient owns
> > PrefService*
> > > object?
> > 
> > We cannot do "const PrefService* GetPrefService() const" here ... Since,
> certain
> > user actions on interstitial need to modify PrefService (e.g. check/uncheck
> > extended reporting opt-in checkbox) 
> nit: If that's the case then is it fair to make |GetPrefService| method const?
> It was not const before, which is reasonable, because returning non-const
> PrefService* allows changing the state of ControllerClient.

You're right. Changed GetPrefService() back to non-const method.

[Jialiu Lin, 2017-01-03 17:53:12]

jialiul@chromium.org changed reviewers:
+ pastarmovj@chromium.org
- tnagel@chromium.org

[Jialiu Lin, 2017-01-03 17:55:06]

jialiul@chromium.org changed reviewers:
+ gab@chromium.org
- pam@chromium.org

[Jialiu Lin, 2017-01-03 17:57:33]

Gentle ping for owner and dependency review. Thanks! 

+nasko@ because I need LGTM from owners of content/public/common in DEPS that
was modified in this CL

+gab@ because I need LGTM from owners of components/prefs in DEPSs that was
modified in this CL

+pastarmovj@  for owner review of chrome/browser/policy/*

[nasko, 2017-01-04 01:03:04]

I'm not sure you need my stamp, but nevertheless
components/security_interstitials/content/DEPS LGTM.

[pastarmovj, 2017-01-04 10:20:06]

policy lgtm

[pastarmovj, 2017-01-04 10:20:06]

policy lgtm

[Jialiu Lin, 2017-01-04 16:52:04]

The CQ bit was checked by jialiul@chromium.org

[Jialiu Lin, 2017-01-04 16:52:04]

The patchset sent to the CQ was uploaded after l-g-t-m from mmenke@chromium.org,
eugenebut@chromium.org, meacer@chromium.org
Link to the patchset: https://codereview.chromium.org/2603433002/#ps200001
(title: "revert GetPrefService to non-const")

[commit-bot: I haz the power, 2017-01-04 16:52:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-04 16:54:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-04 16:54:14]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[Jialiu Lin, 2017-01-04 18:43:48]

The CQ bit was checked by jialiul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-04 18:44:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-04 18:48:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-04 18:48:42]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)

[Jialiu Lin, 2017-01-04 18:53:30]

The CQ bit was checked by jialiul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-04 18:53:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-04 19:07:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-04 19:07:02]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Jialiu Lin, 2017-01-04 19:31:50]

The CQ bit was checked by jialiul@chromium.org

[Jialiu Lin, 2017-01-04 19:31:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
eugenebut@chromium.org, mmenke@chromium.org, nasko@chromium.org,
pastarmovj@chromium.org, meacer@chromium.org
Link to the patchset: https://codereview.chromium.org/2603433002/#ps260001
(title: "missed some changes in rebase")

[commit-bot: I haz the power, 2017-01-04 19:32:26]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-04 19:40:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-04 19:40:54]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Jialiu Lin, 2017-01-04 19:44:33]

jialiul@chromium.org changed reviewers:
+ battre@chromium.org
- gab@chromium.org

[Jialiu Lin, 2017-01-04 19:44:34]

Need one more LGTM. 

+battre@ because I need LGTM from owners of components/prefs in DEPS that was
modified in this CL

[Jialiu Lin, 2017-01-05 01:11:59]

+cc lpz@

[battre, 2017-01-05 08:20:59]

dependence on components/prefs LGTM

[Jialiu Lin, 2017-01-05 18:07:50]

The CQ bit was checked by jialiul@chromium.org

[commit-bot: I haz the power, 2017-01-05 18:08:27]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-05 18:13:08]

CQ is committing da patch.

Bot data: {"patchset_id": 260001, "attempt_start_ts": 1483639670475720,
"parent_rev": "98a412efa58cb8aa48d70b473c38bce2cd6391ea", "commit_rev":
"a2c0692563f58517c32e839312695e91f45971a3"}

[commit-bot: I haz the power, 2017-01-05 18:13:39]

Description was changed from

==========
Move SecurityInterstitialPage into components/security_interstitial/content.
And resolve all the dependencies and tests and so on.

BUG=666100
==========

to

==========
Move SecurityInterstitialPage into components/security_interstitial/content.
And resolve all the dependencies and tests and so on.

BUG=666100

Review-Url: https://codereview.chromium.org/2603433002
Cr-Commit-Position: refs/heads/master@{#441701}
Committed:
https://chromium.googlesource.com/chromium/src/+/a2c0692563f58517c32e83931269...
==========

[commit-bot: I haz the power, 2017-01-05 18:13:41]

Committed patchset #10 (id:260001) as
https://chromium.googlesource.com/chromium/src/+/a2c0692563f58517c32e83931269...