Introduce the ChromeControllerClient class

Introduce the ChromeControllerClient class

security_interstitials::ControllerClient needs to be subclassed by a
standalone class, instead of being part of the SecurityInterstitialPage.

Eventually, future classes like security_interstitials::ClockError will
take ControllerClients as inputs. Until all of the blocking pages have
been updated to take ControllerClients, the functionality needs to
remain available from within the SecurityInterstitialPage, so I've put
in some temporary wrapper methods.

BUG=488673
TBR=cschuet@chromium.org

Committed: https://crrev.com/9de47c5151c1d75d710d22cfa8defb90ff1e7f1e
Cr-Commit-Position: refs/heads/master@{#361245}

[felt, 2015-11-22 10:21:31]

Description was changed from

==========
Introduce the ChromeControllerClient class

security_interstitials::ControllerClient needs to be subclassed by a
standalone class, instead of being part of the SecurityInterstitialPage.

Eventually, future classes like security_interstitials::ClockError will
take ControllerClients as inputs. Until all of the blocking pages have
been updated to take ControllerClients, the functionality needs to
remain available from within the SecurityInterstitialPage, so I've put
in some temporary wrapper methods.

BUG=488673
==========

to

==========
Introduce the ChromeControllerClient class

security_interstitials::ControllerClient needs to be subclassed by a
standalone class, instead of being part of the SecurityInterstitialPage.

Eventually, future classes like security_interstitials::ClockError will
take ControllerClients as inputs. Until all of the blocking pages have
been updated to take ControllerClients, the functionality needs to
remain available from within the SecurityInterstitialPage, so I've put
in some temporary wrapper methods.

BUG=488673
==========

[felt, 2015-11-22 10:21:31]

felt@chromium.org changed reviewers:
+ estark@chromium.org

[felt, 2015-11-22 10:22:32]

estark, PTAL?

[[ Sorry to mail over the weekend, I had this prepped and ready on Friday to
follow up 1462353002. ]]

[estark, 2015-11-23 16:25:23]

lgtm with a couple tiny nits/questions

https://codereview.chromium.org/1467063002/diff/1/chrome/browser/interstitial...
File chrome/browser/interstitials/chrome_controller_client.cc (right):

https://codereview.chromium.org/1467063002/diff/1/chrome/browser/interstitial...
chrome/browser/interstitials/chrome_controller_client.cc:9: #include
"chrome/browser/net/referrer.h"
Are you using this? Is it by chance supposed to be
content/public/common/referrer.h?

https://codereview.chromium.org/1467063002/diff/1/chrome/browser/interstitial...
chrome/browser/interstitials/chrome_controller_client.cc:14: using
content::OpenURLParams;
optional nit: I've been told to only use `using` when it really significantly
improves readability, but I can't find a reference for that, plus this was
pre-existing code, so, *shrug*

https://codereview.chromium.org/1467063002/diff/1/chrome/browser/interstitial...
File chrome/browser/interstitials/security_interstitial_page.h (right):

https://codereview.chromium.org/1467063002/diff/1/chrome/browser/interstitial...
chrome/browser/interstitials/security_interstitial_page.h:62: // subclasses that
don't yet have their own ChromeControllerClients.
maybe make this a TODO with the bug number?

[felt, 2015-11-24 00:18:11]

https://codereview.chromium.org/1467063002/diff/1/chrome/browser/interstitial...
File chrome/browser/interstitials/chrome_controller_client.cc (right):

https://codereview.chromium.org/1467063002/diff/1/chrome/browser/interstitial...
chrome/browser/interstitials/chrome_controller_client.cc:9: #include
"chrome/browser/net/referrer.h"
On 2015/11/23 16:25:23, estark wrote:
> Are you using this? Is it by chance supposed to be
> content/public/common/referrer.h?

ah yes! i'd just blindly copied it from the old file w/o noticing. :)

https://codereview.chromium.org/1467063002/diff/1/chrome/browser/interstitial...
chrome/browser/interstitials/chrome_controller_client.cc:14: using
content::OpenURLParams;
On 2015/11/23 16:25:23, estark wrote:
> optional nit: I've been told to only use `using` when it really significantly
> improves readability, but I can't find a reference for that, plus this was
> pre-existing code, so, *shrug*

Done.

https://codereview.chromium.org/1467063002/diff/1/chrome/browser/interstitial...
File chrome/browser/interstitials/security_interstitial_page.h (right):

https://codereview.chromium.org/1467063002/diff/1/chrome/browser/interstitial...
chrome/browser/interstitials/security_interstitial_page.h:62: // subclasses that
don't yet have their own ChromeControllerClients.
On 2015/11/23 16:25:23, estark wrote:
> maybe make this a TODO with the bug number?

Done.

[felt, 2015-11-24 00:31:58]

felt@chromium.org changed reviewers:
+ atwilson@chromium.org, nparker@chromium.org

[felt, 2015-11-24 00:32:22]

felt@chromium.org changed reviewers:
+ cschuet@chromium.org
- atwilson@chromium.org

[felt, 2015-11-24 00:32:32]

Description was changed from

==========
Introduce the ChromeControllerClient class

security_interstitials::ControllerClient needs to be subclassed by a
standalone class, instead of being part of the SecurityInterstitialPage.

Eventually, future classes like security_interstitials::ClockError will
take ControllerClients as inputs. Until all of the blocking pages have
been updated to take ControllerClients, the functionality needs to
remain available from within the SecurityInterstitialPage, so I've put
in some temporary wrapper methods.

BUG=488673
==========

to

==========
Introduce the ChromeControllerClient class

security_interstitials::ControllerClient needs to be subclassed by a
standalone class, instead of being part of the SecurityInterstitialPage.

Eventually, future classes like security_interstitials::ClockError will
take ControllerClients as inputs. Until all of the blocking pages have
been updated to take ControllerClients, the functionality needs to
remain available from within the SecurityInterstitialPage, so I've put
in some temporary wrapper methods.

BUG=488673
TBR=cschuet@chromium.org
==========

[felt, 2015-11-24 00:33:32]

additional owners, PTAL

nparker - c/b/safe_browsing/
cschuet - trivial renaming in c/b/policy (TBR)

[Nathan Parker, 2015-11-24 00:50:44]

lgtm

for c/b/safe_browsing/

[felt, 2015-11-24 00:55:00]

The CQ bit was checked by felt@chromium.org

[felt, 2015-11-24 00:55:00]

The patchset sent to the CQ was uploaded after l-g-t-m from estark@chromium.org,
nparker@chromium.org
Link to the patchset: https://codereview.chromium.org/1467063002/#ps60001
(title: "Bugfix")

[commit-bot: I haz the power, 2015-11-24 00:59:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1467063002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1467063002/60001

[commit-bot: I haz the power, 2015-11-24 03:30:04]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-11-24 03:31:03]

Patchset 4 (id:??) landed as
https://crrev.com/9de47c5151c1d75d710d22cfa8defb90ff1e7f1e
Cr-Commit-Position: refs/heads/master@{#361245}