Move prefix_set and parts of s_b_util into a new component safe_browsing_db.

Move prefix_set and parts of safe_browsing_util into a new component safe_browsing_db.

This component will hold the new implementation for the
next version (pver4) of the update protocol and DB.

BUG=543161
Committed: https://crrev.com/607ee298239a88fa0823c3aea10bcee83eff855a
Cr-Commit-Position: refs/heads/master@{#355654}

[vakh (old account. dont use), 2015-10-10 01:39:42]

vakh@google.com changed reviewers:
+ noelutz@chromium.org, nparker@chromium.org

[Nathan Parker, 2015-10-14 15:55:24]

nparker@chromium.org changed reviewers:
+ gsennton@chromium.org

[Nathan Parker, 2015-10-14 17:37:10]

https://codereview.chromium.org/1399843003/diff/1/chrome/browser/safe_browsin...
File chrome/browser/safe_browsing/safe_browsing_util.cc (left):

https://codereview.chromium.org/1399843003/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/safe_browsing_util.cc:494: GURL
GeneratePhishingReportUrl(const std::string& report_page,
meta note: This is being removed because it is dead code.

https://codereview.chromium.org/1399843003/diff/1/components/safe_browsing_db...
File components/safe_browsing_db/OWNERS (right):

https://codereview.chromium.org/1399843003/diff/1/components/safe_browsing_db...
components/safe_browsing_db/OWNERS:2: vakh@google.com
I think this won't work unless you're a committer.  Let's add mattm since he
supports the existing code that'll move here.

https://codereview.chromium.org/1399843003/diff/1/components/safe_browsing_db...
File components/safe_browsing_db/README (right):

https://codereview.chromium.org/1399843003/diff/1/components/safe_browsing_db...
components/safe_browsing_db/README:1: SafeBrowsing is in the process of becoming
a layered component to provide it as
How about:

safe_browsing_db/ will contain the database-related code from existing
safe_browsing/ directories, and will house the new Pver4 database+update
implementation.

https://codereview.chromium.org/1399843003/diff/1/components/safe_browsing_db...
File components/safe_browsing_db/safe_browsing_db_util.h (right):

https://codereview.chromium.org/1399843003/diff/1/components/safe_browsing_db...
components/safe_browsing_db/safe_browsing_db_util.h:14: typedef uint32 SBPrefix;
Note: Long term I think we should put this all in safe_browsing namespace,
though I think that'd be better as a separate CL since it'll touch a LOT of code
and should be fairly mechanical.  We should put new code in a namespace.

[Nathan Parker, 2015-10-14 17:38:39]

+jialiul so she's aware of code moving around.  vakh is setting up
safe_browsing_db as a place to implement pver4 DB code, and some existing code
will move there as well.

[Nathan Parker, 2015-10-15 00:33:33]

nparker@chromium.org changed reviewers:
+ droger@chromium.org
- gsennton@chromium.org

[Nathan Parker, 2015-10-15 00:33:33]

+droger for feedback on structure of new component.

[droger, 2015-10-15 09:10:27]

From a high (directory structure and DEPS) this seems fine, if you don't
anticipate componentizing any renderer code.
Otherwise consider putting this code in //components/safe_browsing_db/browser/

https://codereview.chromium.org/1399843003/diff/1/components/safe_browsing_db...
File components/safe_browsing_db.gypi (right):

https://codereview.chromium.org/1399843003/diff/1/components/safe_browsing_db...
components/safe_browsing_db.gypi:12: '../base/base.gyp:base',
Missing dependency on crypto, because of the crypto/sha2.h include.

[vakh (old account. dont use), 2015-10-15 21:07:08]

https://codereview.chromium.org/1399843003/diff/1/chrome/browser/safe_browsin...
File chrome/browser/safe_browsing/safe_browsing_util.cc (left):

https://codereview.chromium.org/1399843003/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/safe_browsing_util.cc:494: GURL
GeneratePhishingReportUrl(const std::string& report_page,
On 2015/10/14 17:37:10, nparker wrote:
> meta note: This is being removed because it is dead code.

Acknowledged.

https://codereview.chromium.org/1399843003/diff/1/components/safe_browsing_db...
File components/safe_browsing_db.gypi (right):

https://codereview.chromium.org/1399843003/diff/1/components/safe_browsing_db...
components/safe_browsing_db.gypi:12: '../base/base.gyp:base',
On 2015/10/15 09:10:27, droger wrote:
> Missing dependency on crypto, because of the crypto/sha2.h include.

Done.

https://codereview.chromium.org/1399843003/diff/1/components/safe_browsing_db...
File components/safe_browsing_db/OWNERS (right):

https://codereview.chromium.org/1399843003/diff/1/components/safe_browsing_db...
components/safe_browsing_db/OWNERS:2: vakh@google.com
On 2015/10/14 17:37:10, nparker wrote:
> I think this won't work unless you're a committer.  Let's add mattm since he
> supports the existing code that'll move here.

Done. Placed him first to keep the list sorted.

https://codereview.chromium.org/1399843003/diff/1/components/safe_browsing_db...
File components/safe_browsing_db/README (right):

https://codereview.chromium.org/1399843003/diff/1/components/safe_browsing_db...
components/safe_browsing_db/README:1: SafeBrowsing is in the process of becoming
a layered component to provide it as
On 2015/10/14 17:37:10, nparker wrote:
> How about:
> 
> safe_browsing_db/ will contain the database-related code from existing
> safe_browsing/ directories, and will house the new Pver4 database+update
> implementation.

Done.

https://codereview.chromium.org/1399843003/diff/1/components/safe_browsing_db...
File components/safe_browsing_db/safe_browsing_db_util.h (right):

https://codereview.chromium.org/1399843003/diff/1/components/safe_browsing_db...
components/safe_browsing_db/safe_browsing_db_util.h:14: typedef uint32 SBPrefix;
On 2015/10/14 17:37:10, nparker wrote:
> Note: Long term I think we should put this all in safe_browsing namespace,
> though I think that'd be better as a separate CL since it'll touch a LOT of
code
> and should be fairly mechanical.  We should put new code in a namespace.

Acknowledged.

[Nathan Parker, 2015-10-15 22:55:32]

nparker@chromium.org changed reviewers:
+ blundell@chromium.org, thestig@chromium.org

[Nathan Parker, 2015-10-15 22:55:33]

lgtm

thestig@chromium.org: Please review changes in 
  chrome/ DEPS and build

blundell@chromium.org: Please review changes in 
  /components/

[Lei Zhang, 2015-10-15 23:01:09]

On 2015/10/15 22:55:33, nparker wrote:
> mailto:thestig@chromium.org: Please review changes in 
>   chrome/ DEPS and build

lgtm

[blundell, 2015-10-16 07:29:25]

lgtm with question resolved

What's the story for this component on iOS in the end? That is to say, in this
CL it's shared with iOS, which from my perspective is great :). However, it also
means it can't use //content (or must be refactored into a layered component in
order to use //content, or must stop being shared with iOS in order to use
//content). Do you envision this component growing //content dependencies, or
no?

If the answer is "no", my LGTM stands. Otherwise, we should discuss what
direction we see this component going in.

[chromium-reviews, 2015-10-16 14:28:57]

This would not be used in iOS, since we don't have plans to support safe
browsing databases there.  When iOS provides a platform-based Safe Browsing
service, we'll loop into that.

How should this be different to _not_ share with iOS?

 -- Nathan

On Fri, Oct 16, 2015 at 12:29 AM, <blundell@chromium.org> wrote:

> lgtm with question resolved
>
> What's the story for this component on iOS in the end? That is to say, in
> this
> CL it's shared with iOS, which from my perspective is great :). However,
> it also
> means it can't use //content (or must be refactored into a layered
> component in
> order to use //content, or must stop being shared with iOS in order to use
> //content). Do you envision this component growing //content dependencies,
> or
> no?
>
> If the answer is "no", my LGTM stands. Otherwise, we should discuss what
> direction we see this component going in.
>
> https://codereview.chromium.org/1399843003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[blundell, 2015-10-16 14:31:45]

On 2015/10/16 14:28:57, chromium-reviews wrote:
> This would not be used in iOS, since we don't have plans to support safe
> browsing databases there.  When iOS provides a platform-based Safe Browsing
> service, we'll loop into that.
> 
> How should this be different to _not_ share with iOS?

In this CL the component is being added as part of the list that is shared
across all platforms. If the intent is to avoid sharing with (and being
restricted by) iOS, then it should be moved to the !iOS section.

> 
>  -- Nathan
> 
> On Fri, Oct 16, 2015 at 12:29 AM, <mailto:blundell@chromium.org> wrote:
> 
> > lgtm with question resolved
> >
> > What's the story for this component on iOS in the end? That is to say, in
> > this
> > CL it's shared with iOS, which from my perspective is great :). However,
> > it also
> > means it can't use //content (or must be refactored into a layered
> > component in
> > order to use //content, or must stop being shared with iOS in order to use
> > //content). Do you envision this component growing //content dependencies,
> > or
> > no?
> >
> > If the answer is "no", my LGTM stands. Otherwise, we should discuss what
> > direction we see this component going in.
> >
> > https://codereview.chromium.org/1399843003/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[vakh (old account. dont use), 2015-10-16 21:44:15]

On 2015/10/16 14:31:45, blundell wrote:
> On 2015/10/16 14:28:57, chromium-reviews wrote:
> > This would not be used in iOS, since we don't have plans to support safe
> > browsing databases there.  When iOS provides a platform-based Safe Browsing
> > service, we'll loop into that.
> > 
> > How should this be different to _not_ share with iOS?
> 
> In this CL the component is being added as part of the list that is shared
> across all platforms. If the intent is to avoid sharing with (and being
> restricted by) iOS, then it should be moved to the !iOS section.
> 
> > 
> >  -- Nathan
> > 
> > On Fri, Oct 16, 2015 at 12:29 AM, <mailto:blundell@chromium.org> wrote:
> > 
> > > lgtm with question resolved
> > >
> > > What's the story for this component on iOS in the end? That is to say, in
> > > this
> > > CL it's shared with iOS, which from my perspective is great :). However,
> > > it also
> > > means it can't use //content (or must be refactored into a layered
> > > component in
> > > order to use //content, or must stop being shared with iOS in order to use
> > > //content). Do you envision this component growing //content dependencies,
> > > or
> > > no?
> > >
> > > If the answer is "no", my LGTM stands. Otherwise, we should discuss what
> > > direction we see this component going in.
> > >
> > > https://codereview.chromium.org/1399843003/
> > >
> > 
> > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.

I don't have the answer to the //content question but may be Nathan knows. If
not, I'll look into it and reply back.

Meanwhile, I have corrected the BUILD and gyp[i] files so that this component is
not included in iOS builds.
Thanks for pointing that out.

[chromium-reviews, 2015-10-16 21:58:01]

I think that's all you needed.  Now it won't be built for iOS, and it can
(and will) include //content.

On Fri, Oct 16, 2015 at 2:44 PM, <vakh@google.com> wrote:

> On 2015/10/16 14:31:45, blundell wrote:
>
>> On 2015/10/16 14:28:57, chromium-reviews wrote:
>> > This would not be used in iOS, since we don't have plans to support safe
>> > browsing databases there.  When iOS provides a platform-based Safe
>> Browsing
>> > service, we'll loop into that.
>> >
>> > How should this be different to _not_ share with iOS?
>>
>
> In this CL the component is being added as part of the list that is shared
>> across all platforms. If the intent is to avoid sharing with (and being
>> restricted by) iOS, then it should be moved to the !iOS section.
>>
>
> >
>> >  -- Nathan
>> >
>> > On Fri, Oct 16, 2015 at 12:29 AM, <mailto:blundell@chromium.org> wrote:
>> >
>> > > lgtm with question resolved
>> > >
>> > > What's the story for this component on iOS in the end? That is to
>> say, in
>> > > this
>> > > CL it's shared with iOS, which from my perspective is great :).
>> However,
>> > > it also
>> > > means it can't use //content (or must be refactored into a layered
>> > > component in
>> > > order to use //content, or must stop being shared with iOS in order
>> to use
>> > > //content). Do you envision this component growing //content
>> dependencies,
>> > > or
>> > > no?
>> > >
>> > > If the answer is "no", my LGTM stands. Otherwise, we should discuss
>> what
>> > > direction we see this component going in.
>> > >
>> > > https://codereview.chromium.org/1399843003/
>> > >
>> >
>> > To unsubscribe from this group and stop receiving emails from it, send
>> an
>> email
>> > to mailto:chromium-reviews+unsubscribe@chromium.org.
>>
>
> I don't have the answer to the //content question but may be Nathan knows.
> If
> not, I'll look into it and reply back.
>
> Meanwhile, I have corrected the BUILD and gyp[i] files so that this
> component is
> not included in iOS builds.
> Thanks for pointing that out.
>
> https://codereview.chromium.org/1399843003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[vakh (old account. dont use), 2015-10-16 23:36:23]

The CQ bit was checked by vakh@google.com

[vakh (old account. dont use), 2015-10-16 23:36:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
nparker@chromium.org, thestig@chromium.org, blundell@chromium.org
Link to the patchset: https://codereview.chromium.org/1399843003/#ps80001
(title: "Make safe_browsing_db a non-iOS component.")

[commit-bot: I haz the power, 2015-10-16 23:37:18]

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

[commit-bot: I haz the power, 2015-10-16 23:42:30]

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

[commit-bot: I haz the power, 2015-10-16 23:42:31]

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...)
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)

[vakh (old account. dont use), 2015-10-16 23:52:26]

vakh@google.com changed reviewers:
+ rsleevi@chromium.org

[vakh (old account. dont use), 2015-10-16 23:54:21]

+rsleevi for allowing components/safe_browsing_db/ to depend on +crypto

[Ryan Sleevi, 2015-10-16 23:55:51]

//crypto DEP LGTM

[vakh (old account. dont use), 2015-10-17 00:37:20]

The CQ bit was checked by vakh@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-10-17 00:37:53]

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

[commit-bot: I haz the power, 2015-10-17 00:49:35]

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

[commit-bot: I haz the power, 2015-10-17 00:49:36]

Dry run: 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...)

[vakh (old account. dont use), 2015-10-22 21:22:59]

The CQ bit was checked by vakh@google.com

[vakh (old account. dont use), 2015-10-22 21:22:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
rsleevi@chromium.org, thestig@chromium.org, nparker@chromium.org,
blundell@chromium.org
Link to the patchset: https://codereview.chromium.org/1399843003/#ps120001
(title: "Use group instead of a static_library for targets that have no
sources.")

[commit-bot: I haz the power, 2015-10-22 21:23:20]

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

[commit-bot: I haz the power, 2015-10-22 22:10:37]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-10-22 22:11:32]

Patchset 7 (id:??) landed as
https://crrev.com/607ee298239a88fa0823c3aea10bcee83eff855a
Cr-Commit-Position: refs/heads/master@{#355654}

[DaleCurtis, 2015-10-22 22:36:15]

A revert of this CL (patchset #7 id:120001) has been created in
https://codereview.chromium.org/1420143002/ by dalecurtis@chromium.org.

The reason for reverting is: Broke the Windows x64 GN builders:

http://build.chromium.org/p/chromium.win/buildstatus?builder=Win%20x64%20GN&n....