Refactor SafeBrowsing and SSL interstitials to have a type variable

Refactor SafeBrowsing and SSL interstitials to have a type variable

We'll be adding a new type of interstitial for captive portals. This
patch is to accomodate that change.

BUG=384667
Committed: https://crrev.com/ab0b014226df255d297af51832f73e0f99ecaca7
Cr-Commit-Position: refs/heads/master@{#297090}

[meacer, 2014-09-25 19:46:00]

meacer@chromium.org changed reviewers:
+ bauerb@chromium.org, mattm@chromium.org, palmer@chromium.org

[meacer, 2014-09-25 19:46:00]

Owners, could you please take a look?

palmer for ssl_blocking_page.cc
mattm for safe_browsing_blocking_page.cc
bauerb for chrome/browser/resources

Thanks!

[palmer, 2014-09-25 20:10:52]

lgtm

[mattm, 2014-09-25 21:27:26]

https://codereview.chromium.org/600153004/diff/1/chrome/browser/safe_browsing...
File chrome/browser/safe_browsing/safe_browsing_blocking_page.cc (right):

https://codereview.chromium.org/600153004/diff/1/chrome/browser/safe_browsing...
chrome/browser/safe_browsing/safe_browsing_blocking_page.cc:692:
load_time_data.SetString("type", "safeBrowsing");
Lowercase s seems a little weird. Is there some style this is following, or
could it be SafeBrowsing or safe_browsing instead?

[meacer, 2014-09-25 21:46:10]

https://codereview.chromium.org/600153004/diff/1/chrome/browser/safe_browsing...
File chrome/browser/safe_browsing/safe_browsing_blocking_page.cc (right):

https://codereview.chromium.org/600153004/diff/1/chrome/browser/safe_browsing...
chrome/browser/safe_browsing/safe_browsing_blocking_page.cc:692:
load_time_data.SetString("type", "safeBrowsing");
On 2014/09/25 21:27:26, mattm wrote:
> Lowercase s seems a little weird. Is there some style this is following, or
> could it be SafeBrowsing or safe_browsing instead?

I wanted to follow the convention with other variable names below this line
(tabTitle, openDetails etc), even though this is actually the value and not the
variable name. I don't mind changing it though.

[mattm, 2014-09-25 21:49:57]

https://codereview.chromium.org/600153004/diff/1/chrome/browser/safe_browsing...
File chrome/browser/safe_browsing/safe_browsing_blocking_page.cc (right):

https://codereview.chromium.org/600153004/diff/1/chrome/browser/safe_browsing...
chrome/browser/safe_browsing/safe_browsing_blocking_page.cc:692:
load_time_data.SetString("type", "safeBrowsing");
On 2014/09/25 21:46:09, Mustafa Emre Acer wrote:
> On 2014/09/25 21:27:26, mattm wrote:
> > Lowercase s seems a little weird. Is there some style this is following, or
> > could it be SafeBrowsing or safe_browsing instead?
> 
> I wanted to follow the convention with other variable names below this line
> (tabTitle, openDetails etc), even though this is actually the value and not
the
> variable name. I don't mind changing it though.

In that case, it seems more like it should follow the style of constant or maybe
enum naming rather than a variable.

[meacer, 2014-09-25 21:58:33]

https://codereview.chromium.org/600153004/diff/1/chrome/browser/safe_browsing...
File chrome/browser/safe_browsing/safe_browsing_blocking_page.cc (right):

https://codereview.chromium.org/600153004/diff/1/chrome/browser/safe_browsing...
chrome/browser/safe_browsing/safe_browsing_blocking_page.cc:692:
load_time_data.SetString("type", "safeBrowsing");
On 2014/09/25 21:49:57, mattm wrote:
> On 2014/09/25 21:46:09, Mustafa Emre Acer wrote:
> > On 2014/09/25 21:27:26, mattm wrote:
> > > Lowercase s seems a little weird. Is there some style this is following,
or
> > > could it be SafeBrowsing or safe_browsing instead?
> > 
> > I wanted to follow the convention with other variable names below this line
> > (tabTitle, openDetails etc), even though this is actually the value and not
> the
> > variable name. I don't mind changing it though.
> 
> In that case, it seems more like it should follow the style of constant or
maybe
> enum naming rather than a variable.

Done, I assumed Safebrowsing is a single word.

[mattm, 2014-09-25 22:22:11]

lgtm

[meacer, 2014-09-26 18:48:08]

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

[meacer, 2014-09-26 18:48:08]

Looks like Bernhard is OOO.

estade: Can you please take a look at chrome/browser/resources/*? Thanks.

[Evan Stade, 2014-09-26 19:28:16]

lgtm

[meacer, 2014-09-26 19:34:37]

The CQ bit was checked by meacer@chromium.org

[commit-bot: I haz the power, 2014-09-26 19:36:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/600153004/20001

[commit-bot: I haz the power, 2014-09-26 19:41:06]

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

[commit-bot: I haz the power, 2014-09-26 19:41:07]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/61365)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/66232)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_chromium_gn_compile_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[meacer, 2014-09-27 00:51:37]

The CQ bit was checked by meacer@chromium.org

[commit-bot: I haz the power, 2014-09-27 00:52:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/600153004/40001

[commit-bot: I haz the power, 2014-09-27 01:46:20]

Committed patchset #3 (id:40001) as 7f1c2928b28f5311b50ec3ea1b6866fed43bf59d

[commit-bot: I haz the power, 2014-09-27 01:47:05]

Patchset 3 (id:??) landed as
https://crrev.com/ab0b014226df255d297af51832f73e0f99ecaca7
Cr-Commit-Position: refs/heads/master@{#297090}