Move chrome/common/safe_browsing/csd.proto to components/safe_browsing

Move chrome/common/safe_browsing/csd.proto to components/safe_browsing

Move csd.proto to components/safe_browsing.
This CL serves 2 purpose:
(1) moving csd.proto to component enables further componentization of
safe browsing reporting functionality, that will be used by WebView
(2) It also allows PasswordProtectionService, and PasswordManager code
to use these proto directly.

BUG=698899, 688629
Review-Url: https://codereview.chromium.org/2733343002
Cr-Original-Commit-Position: refs/heads/master@{#455609}
Committed: https://chromium.googlesource.com/chromium/src/+/f3aafdc5ac1a6caf52c6c41615118e381ad9e549
Review-Url: https://codereview.chromium.org/2733343002
Cr-Commit-Position: refs/heads/master@{#455915}
Committed: https://chromium.googlesource.com/chromium/src/+/2e3385faf6bcf1cb97bd6fcc545643916c792140

[Jialiu Lin, 2017-03-08 00:56:57]

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

[commit-bot: I haz the power, 2017-03-08 00:58:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-08 02:34:05]

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

[commit-bot: I haz the power, 2017-03-08 02:34:06]

Dry run: This issue passed the CQ dry run.

[Jialiu Lin, 2017-03-08 02:55:48]

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

[Jialiu Lin, 2017-03-08 02:55:49]

nparker@, and vakh@, PTAL. Thanks!

[Nathan Parker, 2017-03-08 17:31:43]

Thanks for taking this on, Jialiu!  There are a lot of files including the .h!


A few thoughts to consider, which I don't know the answer to:
* Should we move all the safebrowsing protos at once?  Some of them are
definitely not usable w/o stuff in /chrome, so maybe not.
* Should we generalize csd_proto into just "safe_browsing:proto" so we're not
exposing the specifics of csd? I'm wondering if there are future changes that
might need to touch the same BUILD and DEPS files, for which we could simplify
by generalizing this a bit.
* Where should safe_browsing_db:hit_report live in relation to this?

https://codereview.chromium.org/2733343002/diff/1/chrome/common/DEPS
File chrome/common/DEPS (right):

https://codereview.chromium.org/2733343002/diff/1/chrome/common/DEPS#newcode26
chrome/common/DEPS:26: "+components/safe_browsing/csd.pb.h",
This seems rather specific for what's otherwise a list of commonly used
includes. Does this file decide what chrome/common/* can include? And do they
not already need things from components/safe_browsing/common/*.h?

https://codereview.chromium.org/2733343002/diff/1/chrome/renderer/BUILD.gn
File chrome/renderer/BUILD.gn (right):

https://codereview.chromium.org/2733343002/diff/1/chrome/renderer/BUILD.gn#ne...
chrome/renderer/BUILD.gn:220: "//components/safe_browsing:csd_proto",
Is this not needed on Android? (sb_mode == 2)

[Jialiu Lin, 2017-03-08 18:31:33]

nparker@ and I discussed in person.
We decided not moving other protos in chrome/common/safe_browsing since all the
code they interact with is in chrome/browser.

https://codereview.chromium.org/2733343002/diff/1/chrome/common/DEPS
File chrome/common/DEPS (right):

https://codereview.chromium.org/2733343002/diff/1/chrome/common/DEPS#newcode26
chrome/common/DEPS:26: "+components/safe_browsing/csd.pb.h",
On 2017/03/08 17:31:43, Nathan Parker wrote:
> This seems rather specific for what's otherwise a list of commonly used
> includes. Does this file decide what chrome/common/* can include? And do they
> not already need things from components/safe_browsing/common/*.h?
> 
chrome/common folder does not need components/safe_browsing/common/*. Since I
moved the csd.proto to components/safe_browsing, I add csd.pb.h's new location
to this DEPS file. This is the smallest scope to include.

https://codereview.chromium.org/2733343002/diff/1/chrome/renderer/BUILD.gn
File chrome/renderer/BUILD.gn (right):

https://codereview.chromium.org/2733343002/diff/1/chrome/renderer/BUILD.gn#ne...
chrome/renderer/BUILD.gn:220: "//components/safe_browsing:csd_proto",
On 2017/03/08 17:31:43, Nathan Parker wrote:
> Is this not needed on Android? (sb_mode == 2)

The places in chrome/renderer need csd.proto are all related to client phishing
detection, therefore it is not needed on Android.

[Nathan Parker, 2017-03-08 18:33:32]

lgtm

[Jialiu Lin, 2017-03-08 19:01:22]

jialiul@chromium.org changed reviewers:
+ sky@chromium.org

[Jialiu Lin, 2017-03-08 19:01:23]

+sky@ for owner review of
chrome/common/DEPS
chrome/tools/safe_browsing/DEPS
chrome/tools/safe_browsing/sb_sigutil.cc

[sky, 2017-03-08 21:03:45]

LGTM

[Jialiu Lin, 2017-03-08 22:19:07]

jialiul@chromium.org changed reviewers:
+ pkasting@chromium.org

[Jialiu Lin, 2017-03-08 22:19:08]

I need one more approval from pkasting@, since "third_party/protobuf" is added
to components/safe_browsing/DEPS.

Thanks!

[Peter Kasting, 2017-03-08 22:22:24]

LGTM

[Jialiu Lin, 2017-03-08 22:24:35]

The CQ bit was checked by jialiul@chromium.org

[commit-bot: I haz the power, 2017-03-08 22:25:02]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[vakh (use Gerrit instead), 2017-03-08 22:25:24]

lgtm

[commit-bot: I haz the power, 2017-03-08 22:43:29]

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

[commit-bot: I haz the power, 2017-03-08 22:43:32]

Failed to apply patch for
chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc:
While running git apply --index -p1;
  error: patch failed:
chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc:34
  error:
chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc:
patch does not apply

Patch:      
chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc
Index:
chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc
diff --git
a/chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc
b/chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc
index
2444e83e060fd11626b25a2dbe1fd45021da0ff5..29034e19301a22e98b3591de5328db8e88142371
100644
---
a/chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc
+++
b/chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc
@@ -34,8 +34,8 @@
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/pref_names.h"
-#include "chrome/common/safe_browsing/csd.pb.h"
 #include "components/prefs/pref_service.h"
+#include "components/safe_browsing/csd.pb.h"
 #include "components/safe_browsing_db/database_manager.h"
 #include "components/safe_browsing_db/safe_browsing_prefs.h"
 #include
"components/user_prefs/tracked/tracked_preference_validation_delegate.h"

[Jialiu Lin, 2017-03-08 23:12:00]

The CQ bit was checked by jialiul@chromium.org

[Jialiu Lin, 2017-03-08 23:12:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
nparker@chromium.org, vakh@chromium.org, pkasting@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2733343002/#ps20001
(title: "rebase")

[commit-bot: I haz the power, 2017-03-08 23:12:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-09 00:24:20]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1489014720845250,
"parent_rev": "1fa94875ba2905c83f0dc328d5672c720b932a75", "commit_rev":
"f3aafdc5ac1a6caf52c6c41615118e381ad9e549"}

[commit-bot: I haz the power, 2017-03-09 00:26:06]

Description was changed from

==========
Move chrome/common/safe_browsing/csd.proto to components/safe_browsing

Move csd.proto to components/safe_browsing.
This CL serves 2 purpose:
(1) moving csd.proto to component enables further componentization of
safe browsing reporting functionality, that will be used by WebView
(2) It also allows PasswordProtectionService, and PasswordManager code
to use these proto directly.

BUG=698899,688629
==========

to

==========
Move chrome/common/safe_browsing/csd.proto to components/safe_browsing

Move csd.proto to components/safe_browsing.
This CL serves 2 purpose:
(1) moving csd.proto to component enables further componentization of
safe browsing reporting functionality, that will be used by WebView
(2) It also allows PasswordProtectionService, and PasswordManager code
to use these proto directly.

BUG=698899,688629

Review-Url: https://codereview.chromium.org/2733343002
Cr-Commit-Position: refs/heads/master@{#455609}
Committed:
https://chromium.googlesource.com/chromium/src/+/f3aafdc5ac1a6caf52c6c4161511...
==========

[commit-bot: I haz the power, 2017-03-09 00:26:08]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/f3aafdc5ac1a6caf52c6c4161511...

[xhwang, 2017-03-09 00:51:10]

On 2017/03/09 00:26:08, commit-bot: I haz the power wrote:
> Committed patchset #2 (id:20001) as
>
https://chromium.googlesource.com/chromium/src/+/f3aafdc5ac1a6caf52c6c4161511...

This seems to be breaking the linux bot:

https://build.chromium.org/p/chromium/builders/Linux%20x64/builds/34711

fatal error: 'chrome/common/safe_browsing/csd.pb.h' file not found

[Jialiu Lin, 2017-03-09 00:55:04]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2743483002/ by jialiul@chromium.org.

The reason for reverting is: revert this CL since it broke linux bots..

[Jialiu Lin, 2017-03-09 00:55:29]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2744503002/ by jialiul@chromium.org.

The reason for reverting is: revert this CL since it broke linux bots..

[Scott Hess - ex-Googler, 2017-03-09 01:00:13]

On 2017/03/09 00:55:29, Jialiu Lin wrote:
> A revert of this CL (patchset #2 id:20001) has been created in
> https://codereview.chromium.org/2744503002/ by mailto:jialiul@chromium.org.
> 
> The reason for reverting is: revert this CL since it broke linux bots..

Bridging my IRC comment: I find myself wondering if a stale file in the
incremental build masked a problem, there.  I don’t see a change in the CL for
extensions/BUILD.gn (compile break is in
chrome/browser/extensions/api/downloads/downloads_api.h).

[Jialiu Lin, 2017-03-09 01:12:43]

Description was changed from

==========
Move chrome/common/safe_browsing/csd.proto to components/safe_browsing

Move csd.proto to components/safe_browsing.
This CL serves 2 purpose:
(1) moving csd.proto to component enables further componentization of
safe browsing reporting functionality, that will be used by WebView
(2) It also allows PasswordProtectionService, and PasswordManager code
to use these proto directly.

BUG=698899,688629

Review-Url: https://codereview.chromium.org/2733343002
Cr-Commit-Position: refs/heads/master@{#455609}
Committed:
https://chromium.googlesource.com/chromium/src/+/f3aafdc5ac1a6caf52c6c4161511...
==========

to

==========
Move chrome/common/safe_browsing/csd.proto to components/safe_browsing

Move csd.proto to components/safe_browsing.
This CL serves 2 purpose:
(1) moving csd.proto to component enables further componentization of
safe browsing reporting functionality, that will be used by WebView
(2) It also allows PasswordProtectionService, and PasswordManager code
to use these proto directly.

BUG=698899,688629

Review-Url: https://codereview.chromium.org/2733343002
Cr-Commit-Position: refs/heads/master@{#455609}
Committed:
https://chromium.googlesource.com/chromium/src/+/f3aafdc5ac1a6caf52c6c4161511...
==========

[Scott Hess - ex-Googler, 2017-03-09 01:30:19]

On 2017/03/09 01:00:13, Scott Hess wrote:
> On 2017/03/09 00:55:29, Jialiu Lin wrote:
> > A revert of this CL (patchset #2 id:20001) has been created in
> > https://codereview.chromium.org/2744503002/ by mailto:jialiul@chromium.org.
> > 
> > The reason for reverting is: revert this CL since it broke linux bots..
> 
> Bridging my IRC comment: I find myself wondering if a stale file in the
> incremental build masked a problem, there.  I don’t see a change in the CL for
> extensions/BUILD.gn (compile break is in
> chrome/browser/extensions/api/downloads/downloads_api.h).

If this turns out to be the case, maybe update http://crbug.com/509811 (or maybe
I'm just pattern-matching poorly).

[Jialiu Lin, 2017-03-09 02:10:47]

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

[commit-bot: I haz the power, 2017-03-09 02:11:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-09 02:57:49]

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

[commit-bot: I haz the power, 2017-03-09 02:57:50]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Jialiu Lin, 2017-03-09 05:28:23]

I missed a couple of files that depends on csd.pb.h, which caused waterfall
breakage after CL commit.
Since trybots are using increment builds, the stale
chrome/common/safe_browsing/csd.pb.h was still there, hence this problem got
masked.  I revert the previous commit. 
And reopen this CL and uploaded a new patch, which should fix this problem.

[Jialiu Lin, 2017-03-09 18:06:33]

The CQ bit was checked by jialiul@chromium.org

[Jialiu Lin, 2017-03-09 18:06:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
nparker@chromium.org, vakh@chromium.org, pkasting@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2733343002/#ps40001
(title: "some missed files")

[commit-bot: I haz the power, 2017-03-09 18:06:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-09 19:25:30]

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

[commit-bot: I haz the power, 2017-03-09 19:25:32]

Failed to apply patch for
chrome/browser/safe_browsing/download_feedback_unittest.cc:
While running git apply --index -p1;
  error: patch failed:
chrome/browser/safe_browsing/download_feedback_unittest.cc:11
  error: chrome/browser/safe_browsing/download_feedback_unittest.cc: patch does
not apply

Patch:       chrome/browser/safe_browsing/download_feedback_unittest.cc
Index: chrome/browser/safe_browsing/download_feedback_unittest.cc
diff --git a/chrome/browser/safe_browsing/download_feedback_unittest.cc
b/chrome/browser/safe_browsing/download_feedback_unittest.cc
index
8af5c16e75e274dee5387b600db6e09af77592b3..1b465b285c10ddcb22aade851da2af47ce60ebc2
100644
--- a/chrome/browser/safe_browsing/download_feedback_unittest.cc
+++ b/chrome/browser/safe_browsing/download_feedback_unittest.cc
@@ -11,7 +11,7 @@
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
 #include "chrome/browser/safe_browsing/two_phase_uploader.h"
-#include "chrome/common/safe_browsing/csd.pb.h"
+#include "components/safe_browsing/csd.pb.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "net/url_request/url_request_test_util.h"

[Jialiu Lin, 2017-03-09 19:41:01]

The CQ bit was checked by jialiul@chromium.org

[Jialiu Lin, 2017-03-09 19:41:01]

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

[commit-bot: I haz the power, 2017-03-09 19:41:50]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Jialiu Lin, 2017-03-09 19:47:34]

The CQ bit was unchecked by jialiul@chromium.org

[Jialiu Lin, 2017-03-09 22:32:27]

The CQ bit was checked by jialiul@chromium.org

[Jialiu Lin, 2017-03-09 22:32:27]

The patchset sent to the CQ was uploaded after l-g-t-m from
nparker@chromium.org, vakh@chromium.org, pkasting@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2733343002/#ps80001
(title: "rebase again")

[commit-bot: I haz the power, 2017-03-09 22:33:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-10 00:09:38]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1489098747383060,
"parent_rev": "38c3be3e1d35d839e87eba82c38459fbb971c9aa", "commit_rev":
"2e3385faf6bcf1cb97bd6fcc545643916c792140"}

[commit-bot: I haz the power, 2017-03-10 00:10:16]

Description was changed from

==========
Move chrome/common/safe_browsing/csd.proto to components/safe_browsing

Move csd.proto to components/safe_browsing.
This CL serves 2 purpose:
(1) moving csd.proto to component enables further componentization of
safe browsing reporting functionality, that will be used by WebView
(2) It also allows PasswordProtectionService, and PasswordManager code
to use these proto directly.

BUG=698899,688629

Review-Url: https://codereview.chromium.org/2733343002
Cr-Commit-Position: refs/heads/master@{#455609}
Committed:
https://chromium.googlesource.com/chromium/src/+/f3aafdc5ac1a6caf52c6c4161511...
==========

to

==========
Move chrome/common/safe_browsing/csd.proto to components/safe_browsing

Move csd.proto to components/safe_browsing.
This CL serves 2 purpose:
(1) moving csd.proto to component enables further componentization of
safe browsing reporting functionality, that will be used by WebView
(2) It also allows PasswordProtectionService, and PasswordManager code
to use these proto directly.

BUG=698899,688629

Review-Url: https://codereview.chromium.org/2733343002
Cr-Original-Commit-Position: refs/heads/master@{#455609}
Committed:
https://chromium.googlesource.com/chromium/src/+/f3aafdc5ac1a6caf52c6c4161511...
Review-Url: https://codereview.chromium.org/2733343002
Cr-Commit-Position: refs/heads/master@{#455915}
Committed:
https://chromium.googlesource.com/chromium/src/+/2e3385faf6bcf1cb97bd6fcc5456...
==========

[commit-bot: I haz the power, 2017-03-10 00:10:17]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/2e3385faf6bcf1cb97bd6fcc5456...