Refactor safe-browsing build-config definitions

Refactor safe-browsing build-config definitions.

Centralize the configuration and break things down into individual sub-systems to enable, rather than referring to feature bundles.

I will in a later CL add safe_browsing==3 for enabling XLB on Android, which will define SAFE_BROWSING_DB_REMOTE.  When launched, this will replace mode 2.

SAFE_BROWSING_SERVICE - enable the browser to talk to the server.
SAFE_BROWSING_DB_LOCAL - maintain local database(s) synced from the server.
SAFE_BROWSING_DB_REMOTE - talk via API to an external DB (i.e. XLB).
SAFE_BROWSING_CSD - client-side phishing detection support.

This is nearly a copy of shess@'s 8-month-old
https://codereview.chromium.org/397853002/ which was LGTMed.  Diffs: Mostly merge conflicts, a rename, and few files added

BUG=394379, 474608
Committed: https://crrev.com/333f169bf09e1c455ea821520010ec8cc9658990
Cr-Commit-Position: refs/heads/master@{#325775}

[Nathan Parker, 2015-04-15 22:00:36]

nparker@chromium.org changed reviewers:
+ brettw@chromium.org, mattm@chromium.org, shess@chromium.org,
thestig@chromium.org

[Nathan Parker, 2015-04-16 16:48:10]

I'm leaning towards "local / remote" rather than "internal / external."  lmo
what you think.

[Scott Hess - ex-Googler, 2015-04-16 19:34:22]

I think LGTM, though I no longer have the mental model in place to really
understand what I might be missing.

I'm sad that FULL_SAFE_BROWSING and MOBILE_SAFE_BROWSING aren't entirely gone,
though :-).

https://codereview.chromium.org/1081403002/diff/60001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/1081403002/diff/60001/chrome/chrome_tests.gyp...
chrome/chrome_tests.gypi:2200: # TODO(sgurun) enable tests for safe_browsing==2.
I see that the OLD version is different than it is for my stale CL, there are no
longer cases for 0 and 2..  I wonder if this change still makes sense?

[Nathan Parker, 2015-04-16 19:59:12]

Fix chrome_tests.gypi, per comment

[Nathan Parker, 2015-04-16 20:01:53]

I agree on the desire to get rid of the other two DEFINES.  I plan to do that as
I do some more refactoring, but not immediately.

https://codereview.chromium.org/1081403002/diff/60001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/1081403002/diff/60001/chrome/chrome_tests.gyp...
chrome/chrome_tests.gypi:2200: # TODO(sgurun) enable tests for safe_browsing==2.
On 2015/04/16 19:34:22, Scott Hess wrote:
> I see that the OLD version is different than it is for my stale CL, there are
no
> longer cases for 0 and 2..  I wonder if this change still makes sense?

I think it should be just sb==1.  Fixed.  The tests it runs are related to
database updates, etc.

I'll leave the todo since we need some tests for mode 2, but not precisely
these.

[Nathan Parker, 2015-04-16 20:02:18]

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

[Nathan Parker, 2015-04-16 20:02:19]

The patchset sent to the CQ was uploaded after l-g-t-m from shess@chromium.org
Link to the patchset: https://codereview.chromium.org/1081403002/#ps80001
(title: "Fix chrome_tests.gypi, per comment")

[commit-bot: I haz the power, 2015-04-16 20:03:58]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1081403002/80001

[commit-bot: I haz the power, 2015-04-17 05:47:06]

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

[commit-bot: I haz the power, 2015-04-17 05:47:07]

Dry run: This issue passed the CQ dry run.

[Scott Hess - ex-Googler, 2015-04-17 15:01:41]

https://codereview.chromium.org/1081403002/diff/60001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/1081403002/diff/60001/chrome/chrome_tests.gyp...
chrome/chrome_tests.gypi:2200: # TODO(sgurun) enable tests for safe_browsing==2.
On 2015/04/16 20:01:53, nparker wrote:
> On 2015/04/16 19:34:22, Scott Hess wrote:
> > I see that the OLD version is different than it is for my stale CL, there
are
> no
> > longer cases for 0 and 2..  I wonder if this change still makes sense?
> 
> I think it should be just sb==1.  Fixed.  The tests it runs are related to
> database updates, etc.
> 
> I'll leave the todo since we need some tests for mode 2, but not precisely
> these.

Acknowledged.

[Nathan Parker, 2015-04-17 17:13:06]

Rename INTERNAL/EXTERNAL to LOCAL/REMOTE

[Lei Zhang (Do not use), 2015-04-17 18:04:47]

thestig@google.com changed reviewers:
+ thestig@google.com

[Lei Zhang (Do not use), 2015-04-17 18:04:48]

https://codereview.chromium.org/1081403002/diff/100001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/1081403002/diff/100001/build/common.gypi#newc...
build/common.gypi:2987: 'FULL_SAFE_BROWSING',
Are there still uses of these FULL_SAFE_BROWSING and MOBILE_SAFE_BROWSING
outside of this CL that you haven't gotten rid of?

[Lei Zhang (Do not use), 2015-04-17 18:29:13]

On 2015/04/17 18:04:48, Lei Zhang (Do not use) wrote:
> https://codereview.chromium.org/1081403002/diff/100001/build/common.gypi
> File build/common.gypi (right):
> 
>
https://codereview.chromium.org/1081403002/diff/100001/build/common.gypi#newc...
> build/common.gypi:2987: 'FULL_SAFE_BROWSING',
> Are there still uses of these FULL_SAFE_BROWSING and MOBILE_SAFE_BROWSING
> outside of this CL that you haven't gotten rid of?

And to answer my own question, yes... can you add a TODO to get rid of these 2
#defines?

[Lei Zhang (Do not use), 2015-04-17 18:30:51]

lgtm

https://codereview.chromium.org/1081403002/diff/100001/chrome/browser/rendere...
File chrome/browser/renderer_host/safe_browsing_resource_throttle_factory.cc
(right):

https://codereview.chromium.org/1081403002/diff/100001/chrome/browser/rendere...
chrome/browser/renderer_host/safe_browsing_resource_throttle_factory.cc:48:
#error Need to define safe_browsing mode.
I'd put this check at the top of the file.

[Lei Zhang, 2015-04-17 18:40:39]

thestig@chromium.org changed reviewers:
- thestig@google.com

[Lei Zhang, 2015-04-17 18:40:39]

Oh, and the CL description needs to be updated.

This lgtm is from the right account.

[Nathan Parker, 2015-04-17 18:51:09]

rm redundant defs, add todos

[Nathan Parker, 2015-04-17 19:00:02]

Good catch. I've removed defs that aren't needed, and added TODOs.  I'll fix the
TODOs when I make a smaller pass through all of this to add the new XLB defs.

[Nathan Parker, 2015-04-17 19:00:08]

The CQ bit was checked by nparker@chromium.org

[Nathan Parker, 2015-04-17 19:00:09]

The patchset sent to the CQ was uploaded after l-g-t-m from shess@chromium.org,
thestig@chromium.org, thestig@google.com
Link to the patchset: https://codereview.chromium.org/1081403002/#ps120001
(title: "rm redundant defs, add todos")

[commit-bot: I haz the power, 2015-04-17 19:01:19]

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

[commit-bot: I haz the power, 2015-04-17 19:13:35]

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

[commit-bot: I haz the power, 2015-04-17 19:13:36]

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

[Martin Barbella, 2015-04-17 20:06:15]

mbarbella@chromium.org changed reviewers:
+ mbarbella@chromium.org

[Martin Barbella, 2015-04-17 20:06:15]

LGTM for ipc_fuzzer.

[Nathan Parker, 2015-04-17 20:53:26]

nparker@chromium.org changed reviewers:
+ scottmg@chromium.org, tsepez@chromium.org

[Nathan Parker, 2015-04-17 20:53:27]

tsepez: Please review changes in 
chrome/common/safe_browsing/safebrowsing_messages.h

scottmg: Please review changes in build/common.gypi and  build/config/BUILD.gn

Thanks!

[Tom Sepez, 2015-04-17 20:55:19]

RS LGTM on changing macro names in messages file.

[scottmg, 2015-04-17 21:03:25]

lgtm

[Nathan Parker, 2015-04-17 21:09:02]

Thanks everyone!  Submitting.

[Nathan Parker, 2015-04-17 21:09:08]

The CQ bit was checked by nparker@chromium.org

[Nathan Parker, 2015-04-17 21:09:09]

The patchset sent to the CQ was uploaded after l-g-t-m from thestig@google.com,
thestig@chromium.org, mbarbella@chromium.org, tsepez@chromium.org,
shess@chromium.org
Link to the patchset: https://codereview.chromium.org/1081403002/#ps140001
(title: "move #error up, per suggestion")

[commit-bot: I haz the power, 2015-04-17 21:10:35]

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

[commit-bot: I haz the power, 2015-04-17 21:16:49]

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

[commit-bot: I haz the power, 2015-04-17 21:16:51]

Try jobs failed on following builders:
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[Nathan Parker, 2015-04-17 21:44:07]

The CQ bit was checked by nparker@chromium.org

[Nathan Parker, 2015-04-17 21:44:07]

The patchset sent to the CQ was uploaded after l-g-t-m from thestig@google.com,
thestig@chromium.org, mbarbella@chromium.org, scottmg@chromium.org,
tsepez@chromium.org, shess@chromium.org
Link to the patchset: https://codereview.chromium.org/1081403002/#ps160001
(title: "rebase to fix conflict")

[commit-bot: I haz the power, 2015-04-17 21:45:33]

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

[commit-bot: I haz the power, 2015-04-17 21:54:07]

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

[commit-bot: I haz the power, 2015-04-17 21:54:08]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

[Nathan Parker, 2015-04-17 22:48:07]

The CQ bit was checked by nparker@chromium.org

[commit-bot: I haz the power, 2015-04-17 22:50:48]

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

[commit-bot: I haz the power, 2015-04-17 23:00:38]

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

[commit-bot: I haz the power, 2015-04-17 23:00:39]

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

[Nathan Parker, 2015-04-17 23:25:08]

The CQ bit was checked by nparker@chromium.org

[Nathan Parker, 2015-04-17 23:25:08]

The patchset sent to the CQ was uploaded after l-g-t-m from thestig@google.com,
thestig@chromium.org, mbarbella@chromium.org, scottmg@chromium.org,
tsepez@chromium.org, shess@chromium.org
Link to the patchset: https://codereview.chromium.org/1081403002/#ps180001
(title: "rebase again to fix collision")

[commit-bot: I haz the power, 2015-04-17 23:25:56]

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

[commit-bot: I haz the power, 2015-04-18 13:33:31]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2015-04-18 13:34:19]

Patchset 10 (id:??) landed as
https://crrev.com/333f169bf09e1c455ea821520010ec8cc9658990
Cr-Commit-Position: refs/heads/master@{#325775}