Move remote_db_manager into the safe_browsing_db component.

Move remote_db_manager into the safe_browsing_db component.

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

[vakh (use Gerrit instead), 2016-01-13 00:35:17]

Moving metadata.proto and safe_browsing_api_handler_ into safe_browsing_db

[vakh (use Gerrit instead), 2016-01-13 00:46:50]

//chrome/browser/chromeos:chromeos needs
//chrome/browser/safe_browsing:metadata_proto

[vakh (use Gerrit instead), 2016-01-14 22:15:22]

Add dependency on :safe_browsing_metadata_proto and remove dependency on files
moved from browser/safe_browsing/

[vakh (use Gerrit instead), 2016-01-14 23:02:06]

Correct the #include path in chrome_main_delegate_android.cc

[vakh (use Gerrit instead), 2016-01-14 23:36:23]

git rebase

[vakh (use Gerrit instead), 2016-01-15 01:20:49]

Change the location of unit test files in gyp and gn files

[vakh (use Gerrit instead), 2016-01-15 01:27:52]

unit_tests_mobile depend on :safe_browsing_api_handler

[vakh (use Gerrit instead), 2016-01-15 01:43:24]

unit_tests_mobile depend on //components/variations

[vakh (use Gerrit instead), 2016-01-15 18:54:01]

git fetch && git rebase

[vakh (use Gerrit instead), 2016-01-15 18:59:00]

Remove reference to chrome_unit_tests_mobile_safe_browsing_sources

[vakh (use Gerrit instead), 2016-01-15 20:53:34]

chrome_android depends on safe_browsing_db_mobile

[vakh (use Gerrit instead), 2016-01-15 21:27:31]

chrome_android depends on safe_browsing_db_mobile (gn)

[vakh (use Gerrit instead), 2016-01-15 22:34:37]

Add a gn dep on safe_browsing_db_mobile

[vakh (use Gerrit instead), 2016-01-15 23:18:53]

Description was changed from

==========
Move remote_db_manager into the safe_browsing_db component.

BUG=543161
==========

to

==========
Move remote_db_manager into the safe_browsing_db component.

BUG=543161
==========

[vakh (use Gerrit instead), 2016-01-15 23:18:53]

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

[Nathan Parker, 2016-01-20 23:26:54]

lgtm

https://codereview.chromium.org/1579403002/diff/260001/chrome/app/DEPS
File chrome/app/DEPS (right):

https://codereview.chromium.org/1579403002/diff/260001/chrome/app/DEPS#newcode23
chrome/app/DEPS:23: "+components/safe_browsing_db/safe_browsing_api_handler.h",
Do you know what chrome/app/ is?  I'm not sure why it needs to depend on this,
if not all of the component.

https://codereview.chromium.org/1579403002/diff/260001/chrome/chrome_android....
File chrome/chrome_android.gypi (right):

https://codereview.chromium.org/1579403002/diff/260001/chrome/chrome_android....
chrome/chrome_android.gypi:24:
'../components/components.gyp:safe_browsing_db_mobile',
How did chrome_android_core depend on this code before?

https://codereview.chromium.org/1579403002/diff/260001/components/components_...
File components/components_tests.gyp (right):

https://codereview.chromium.org/1579403002/diff/260001/components/components_...
components/components_tests.gyp:1268:
'safe_browsing_db/remote_database_manager_unittest.cc',
Can we specify a target here rather than individual sources? I do see a lot of
component/*.cc's above, but I'm not clear why those are listed in the component
instead.

https://codereview.chromium.org/1579403002/diff/260001/components/safe_browsi...
File components/safe_browsing_db/safe_browsing_api_handler_util.h (right):

https://codereview.chromium.org/1579403002/diff/260001/components/safe_browsi...
components/safe_browsing_db/safe_browsing_api_handler_util.h:12: #include
"components/safe_browsing_db/util.h"
This points to a different file, not one that you're moving.  Correct?

[vakh (use Gerrit instead), 2016-01-21 01:36:46]

https://codereview.chromium.org/1579403002/diff/260001/chrome/app/DEPS
File chrome/app/DEPS (right):

https://codereview.chromium.org/1579403002/diff/260001/chrome/app/DEPS#newcode23
chrome/app/DEPS:23: "+components/safe_browsing_db/safe_browsing_api_handler.h",
On 2016/01/20 23:26:54, nparker wrote:
> Do you know what chrome/app/ is?  I'm not sure why it needs to depend on this,
> if not all of the component.

chrome/app/android/chrome_main_delegate_android.h/cc uses SafeBrowsingApiHandler
but it's not clear why that file needs it.

chrome/app/android/DEPS is almost empty.
chrome/app/DEPS lists other .h files as a dependency and
safe_browsing_api_handler.h was the smallest "unit" that works so I went with
that.
Not sure what the right answer is here.

https://codereview.chromium.org/1579403002/diff/260001/chrome/chrome_android....
File chrome/chrome_android.gypi (right):

https://codereview.chromium.org/1579403002/diff/260001/chrome/chrome_android....
chrome/chrome_android.gypi:24:
'../components/components.gyp:safe_browsing_db_mobile',
On 2016/01/20 23:26:54, nparker wrote:
> How did chrome_android_core depend on this code before?

Likely through: 'chrome.gyp:browser'

https://codereview.chromium.org/1579403002/diff/260001/components/components_...
File components/components_tests.gyp (right):

https://codereview.chromium.org/1579403002/diff/260001/components/components_...
components/components_tests.gyp:1268:
'safe_browsing_db/remote_database_manager_unittest.cc',
On 2016/01/20 23:26:54, nparker wrote:
> Can we specify a target here rather than individual sources? I do see a lot of
> component/*.cc's above, but I'm not clear why those are listed in the
component
> instead. 

Not sure if we can specify targets for sources. The closest thing is a group,
such as: '<@(nacl_unittest_sources)'
However, for just 2 files, it probably isn't that useful.

https://codereview.chromium.org/1579403002/diff/260001/components/safe_browsi...
File components/safe_browsing_db/safe_browsing_api_handler_util.h (right):

https://codereview.chromium.org/1579403002/diff/260001/components/safe_browsi...
components/safe_browsing_db/safe_browsing_api_handler_util.h:12: #include
"components/safe_browsing_db/util.h"
On 2016/01/20 23:26:54, nparker wrote:
> This points to a different file, not one that you're moving.  Correct?

chrome/browser/safe_browsing/safe_browsing_util.h was split into two parts:
* components/safe_browsing_db/util.h -- that contained non-chunk related
structs.
* chrome/browser/safe_browsing/safe_browsing_util.h (same) -- that contained
chunk related structs. It still #includes the other file (util.h)

This file (safe_browsing_api_handler_util.h) only depends on the util.h part of
the original safe_browsing_util.h file.

[vakh (use Gerrit instead), 2016-01-21 01:42:04]

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

[vakh (use Gerrit instead), 2016-01-21 01:42:05]

Thanks nparker!

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

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

[blundell, 2016-01-21 08:51:27]

top-level //components lgtm with nit

https://codereview.chromium.org/1579403002/diff/260001/components/components_...
File components/components_tests.gyp (right):

https://codereview.chromium.org/1579403002/diff/260001/components/components_...
components/components_tests.gyp:1268:
'safe_browsing_db/remote_database_manager_unittest.cc',
On 2016/01/21 01:36:46, vakh wrote:
> On 2016/01/20 23:26:54, nparker wrote:
> > Can we specify a target here rather than individual sources? I do see a lot
of
> > component/*.cc's above, but I'm not clear why those are listed in the
> component
> > instead. 
> 
> Not sure if we can specify targets for sources. The closest thing is a group,
> such as: '<@(nacl_unittest_sources)'
> However, for just 2 files, it probably isn't that useful.

I think it would be more in the spirit of how things are generally organized in
this file to define a variable for these unittests.

[vakh (use Gerrit instead), 2016-01-21 18:15:39]

git fetch && git rebase && git cl format

[vakh (use Gerrit instead), 2016-01-21 18:34:41]

Define a variable for safe_browsing_db_mobile_unittest_sources

[vakh (use Gerrit instead), 2016-01-21 18:34:55]

vakh@chromium.org changed reviewers:
- thestig@chromium.org

[vakh (use Gerrit instead), 2016-01-21 18:48:45]

vakh@chromium.org changed reviewers:
+ jochen@chromium.org

[vakh (use Gerrit instead), 2016-01-21 18:48:46]

Thanks blundell@

jochen@chromium.org: Please review changes in
chrome/...

https://codereview.chromium.org/1579403002/diff/260001/components/components_...
File components/components_tests.gyp (right):

https://codereview.chromium.org/1579403002/diff/260001/components/components_...
components/components_tests.gyp:1268:
'safe_browsing_db/remote_database_manager_unittest.cc',
On 2016/01/21 08:51:27, blundell wrote:
> On 2016/01/21 01:36:46, vakh wrote:
> > On 2016/01/20 23:26:54, nparker wrote:
> > > Can we specify a target here rather than individual sources? I do see a
lot
> of
> > > component/*.cc's above, but I'm not clear why those are listed in the
> > component
> > > instead. 
> > 
> > Not sure if we can specify targets for sources. The closest thing is a
group,
> > such as: '<@(nacl_unittest_sources)'
> > However, for just 2 files, it probably isn't that useful.
> 
> I think it would be more in the spirit of how things are generally organized
in
> this file to define a variable for these unittests.

Done.

[jochen (gone - plz use gerrit), 2016-01-22 14:22:14]

lgtm

[vakh (use Gerrit instead), 2016-01-22 19:10:49]

The CQ bit was checked by vakh@chromium.org

[vakh (use Gerrit instead), 2016-01-22 19:10:50]

The patchset sent to the CQ was uploaded after l-g-t-m from
nparker@chromium.org, blundell@chromium.org
Link to the patchset: https://codereview.chromium.org/1579403002/#ps300001
(title: "Define a variable for safe_browsing_db_mobile_unittest_sources")

[commit-bot: I haz the power, 2016-01-22 19:11:18]

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

[commit-bot: I haz the power, 2016-01-22 21:01:41]

Description was changed from

==========
Move remote_db_manager into the safe_browsing_db component.

BUG=543161
==========

to

==========
Move remote_db_manager into the safe_browsing_db component.

BUG=543161
==========

[commit-bot: I haz the power, 2016-01-22 21:01:42]

Committed patchset #17 (id:300001)

[commit-bot: I haz the power, 2016-01-22 21:02:52]

Description was changed from

==========
Move remote_db_manager into the safe_browsing_db component.

BUG=543161
==========

to

==========
Move remote_db_manager into the safe_browsing_db component.

BUG=543161

Committed: https://crrev.com/bdd302affda19c5b2051d1e868c7045f5614c955
Cr-Commit-Position: refs/heads/master@{#371034}
==========

[commit-bot: I haz the power, 2016-01-22 21:02:53]

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

[Scott Hess - ex-Googler, 2016-01-22 21:47:04]

A revert of this CL (patchset #17 id:300001) has been created in
https://codereview.chromium.org/1620813005/ by shess@chromium.org.

The reason for reverting is: "buildbot failure in Chromium on Win"

components\safe_browsing_db\safe_browsing_api_handler_util.cc(138) : error
C2220: warning treated as error - no 'object' file generated
components\safe_browsing_db\safe_browsing_api_handler_util.cc(138) : warning
C4701: potentially uninitialized local variable 'matches' used
components\safe_browsing_db\safe_browsing_api_handler_util.cc(138) : warning
C4703: potentially uninitialized local pointer variable 'matches' used
.