Componentize ping_manager

Componentize ping_manager

Consolidate functions in ProtocolManagerHelper and put them inside
components/safe_browsing_db/util.*.
Separate BasePingManager and SafeBrowsingPingManager, where:
BasePingManager handles basic safe browsing pings such as HitReport,
ThreatDetails report etc.
Leave NotificationImageReport and PermissionReport to
SafeBrowsingPingManager due to their chrome/browser/ dependencies.
BUG=684805
Review-Url: https://codereview.chromium.org/2650973005
Cr-Commit-Position: refs/heads/master@{#453300}
Committed: https://chromium.googlesource.com/chromium/src/+/d06419a9457679d7daf3e513c3f72431233ca708

[Jialiu Lin, 2017-02-07 00:44:43]

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

[commit-bot: I haz the power, 2017-02-07 00:45:52]

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-02-07 00:51:37]

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

[commit-bot: I haz the power, 2017-02-07 00:51:38]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_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-02-07 00:56:15]

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

[Jialiu Lin, 2017-02-07 00:56:30]

Patchset #2 (id:20001) has been deleted

[commit-bot: I haz the power, 2017-02-07 00:57:08]

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-02-07 01:03:55]

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

[commit-bot: I haz the power, 2017-02-07 01:03:57]

Dry run: Try jobs failed on following builders:
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  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_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_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_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Jialiu Lin, 2017-02-07 01:08:23]

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

[commit-bot: I haz the power, 2017-02-07 01:09:33]

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-02-07 02:33:31]

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

[commit-bot: I haz the power, 2017-02-07 02:33:32]

Dry run: This issue passed the CQ dry run.

[Jialiu Lin, 2017-02-07 21:40:32]

Patchset #2 (id:40001) has been deleted

[Jialiu Lin, 2017-02-07 21:41:04]

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

[Jialiu Lin, 2017-02-07 21:41:05]

Hi vakh@, take your time with this CL. It is not urgent.

[vakh (use Gerrit instead), 2017-02-14 02:33:32]

lgtm modulo namespace comment.

The CL does what it says.

https://codereview.chromium.org/2650973005/diff/60001/components/safe_browsin...
File components/safe_browsing_db/util.h (right):

https://codereview.chromium.org/2650973005/diff/60001/components/safe_browsin...
components/safe_browsing_db/util.h:185: std::string Version();
The function name is very generic. I think it would be good to have this
function, may be even all of this code being moved, into a namespace such as
ProtocolManagerHelper.

[Jialiu Lin, 2017-02-14 18:43:01]

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

[commit-bot: I haz the power, 2017-02-14 18:43:26]

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

[Jialiu Lin, 2017-02-14 18:44:07]

Thanks vakh@!

https://codereview.chromium.org/2650973005/diff/60001/components/safe_browsin...
File components/safe_browsing_db/util.h (right):

https://codereview.chromium.org/2650973005/diff/60001/components/safe_browsin...
components/safe_browsing_db/util.h:185: std::string Version();
On 2017/02/14 02:33:31, vakh (Varun Khaneja) wrote:
> The function name is very generic. I think it would be good to have this
> function, may be even all of this code being moved, into a namespace such as
> ProtocolManagerHelper.

Sounds good. Namespace added.

[Jialiu Lin, 2017-02-14 18:51:36]

jialiul@chromium.org changed reviewers:
+ bengr@chromium.org, rdevlin.cronin@chromium.org

[Jialiu Lin, 2017-02-14 18:51:37]

+rdevlin.cronin@ for owner review of minor change in
c/b/extensions/blacklist_state_fetcher.*

+bengr@ for adding components/data_use_measurement/core to
components/safe_browsing/DEPS

+thestig@ for adding components/version_info to components/safe_browsing/DEPS

+rogerta@chromium.org for adding google_apis to  components/safe_browsing/DEPS

+phajdan.jr@ for adding testing/gtest to components/safe_browsing/DEPS

[Jialiu Lin, 2017-02-14 18:53:19]

jialiul@chromium.org changed reviewers:
+ phajdan.jr@chromium.org, rogerta@chromium.org, thakis@chromium.org

[Jialiu Lin, 2017-02-14 18:53:19]

+thakis@ for adding components/version_info to components/safe_browsing/DEPS

+rogerta@ for adding google_apis to components/safe_browsing/DEPS

+phajdan.jr@ for adding testing/gtest to components/safe_browsing/DEPS

[Nico, 2017-02-14 18:58:41]

https://codereview.chromium.org/2650973005/diff/80001/components/safe_browsin...
File components/safe_browsing_db/util.cc (right):

https://codereview.chromium.org/2650973005/diff/80001/components/safe_browsin...
components/safe_browsing_db/util.cc:229: return "0.1";
Huh? Why even have this function? Why is 0.1 a good version if we don't have a
version number?

[Jialiu Lin, 2017-02-14 19:01:19]

https://codereview.chromium.org/2650973005/diff/80001/components/safe_browsin...
File components/safe_browsing_db/util.cc (right):

https://codereview.chromium.org/2650973005/diff/80001/components/safe_browsin...
components/safe_browsing_db/util.cc:229: return "0.1";
On 2017/02/14 18:58:40, Nico wrote:
> Huh? Why even have this function? Why is 0.1 a good version if we don't have a
> version number?

Honestly, I don't know. I simply copied this function down from the deleted
chrome/browser/safe_browsing/protocol_manager_helper.cc. I guess the Safe
Browsing server end may do something with the "0.1"?

[Nico, 2017-02-14 19:05:07]

dep lgtm

[Paweł Hajdan Jr., 2017-02-14 19:19:22]

Adding testing/gtest LGTM .

[Roger Tawa OOO till Jul 10th, 2017-02-14 19:22:31]

lgtm google_apis to components/safe_browsing/DEPS

[commit-bot: I haz the power, 2017-02-14 20:51:46]

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

[commit-bot: I haz the power, 2017-02-14 20:51:47]

Dry run: This issue passed the CQ dry run.

[bengr, 2017-02-15 00:13:16]

What would you like me to review?

[Jialiu Lin, 2017-02-15 00:17:59]

Hi bengr@, 
This CL added adding components/data_use_measurement/core to
components/safe_browsing/DEPS. And I believe you're a owner of
data_use_measurement component. I need your approval to proceed. :-)

[Devlin, 2017-02-15 00:28:48]

extensions lgtm

[Jialiu Lin, 2017-02-15 23:46:13]

The CQ bit was checked by jialiul@chromium.org

[Jialiu Lin, 2017-02-15 23:46:14]

The patchset sent to the CQ was uploaded after l-g-t-m from vakh@chromium.org
Link to the patchset: https://codereview.chromium.org/2650973005/#ps80001
(title: "address comment from vakh")

[commit-bot: I haz the power, 2017-02-15 23:47:18]

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-02-16 00:50:28]

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

[commit-bot: I haz the power, 2017-02-16 00:50:29]

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

[Jialiu Lin, 2017-02-16 01:08:49]

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

[Jialiu Lin, 2017-02-16 01:08:51]

Hi tbansal@chromium.org,
Need your owner approval since I added 
+components/data_use_measurement/core to components/safe_browsing/DEPS file. 

Thanks!

[Jialiu Lin, 2017-02-17 18:19:38]

Friendly ping bengr@ or tbansal@ for owner approval of adding
components/data_use_measurement/core to components/safe_browsing/DEPS file.

[tbansal1, 2017-02-17 18:25:14]

lgtm for adding
components/data_use_measurement/core to components/safe_browsing/DEPS. Thanks.

[bengr, 2017-02-24 19:35:44]

lgtm

[Jialiu Lin, 2017-02-24 19:44:28]

The CQ bit was checked by jialiul@chromium.org

[commit-bot: I haz the power, 2017-02-24 19:45:10]

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-02-24 19:49:35]

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

[commit-bot: I haz the power, 2017-02-24 19:49:36]

Try jobs failed on following builders:
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_ozone_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_...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Jialiu Lin, 2017-02-25 00:20:28]

The CQ bit was checked by jialiul@chromium.org

[Jialiu Lin, 2017-02-25 00:20:28]

The patchset sent to the CQ was uploaded after l-g-t-m from
rdevlin.cronin@chromium.org, rogerta@chromium.org, bengr@chromium.org,
thakis@chromium.org, phajdan.jr@chromium.org, vakh@chromium.org,
tbansal@chromium.org
Link to the patchset: https://codereview.chromium.org/2650973005/#ps100001
(title: "rebase")

[commit-bot: I haz the power, 2017-02-25 00:21:00]

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-02-25 00:23:52]

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

[commit-bot: I haz the power, 2017-02-25 00:23:55]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Jialiu Lin, 2017-02-27 18:19:44]

The CQ bit was checked by jialiul@chromium.org

[commit-bot: I haz the power, 2017-02-27 18:20:18]

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-02-27 19:40:16]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1488219584923060,
"parent_rev": "cb3da9c98f24a2fe73a40d490548e1a9acf1df1d", "commit_rev":
"d06419a9457679d7daf3e513c3f72431233ca708"}

[commit-bot: I haz the power, 2017-02-27 19:41:35]

Description was changed from

==========
Componentize ping_manager

Consolidate functions in ProtocolManagerHelper and put them inside
components/safe_browsing_db/util.*.
Separate BasePingManager and SafeBrowsingPingManager, where:
BasePingManager handles basic safe browsing pings such as HitReport,
ThreatDetails report etc.
Leave NotificationImageReport and PermissionReport to
SafeBrowsingPingManager due to their chrome/browser/ dependencies.
BUG=684805
==========

to

==========
Componentize ping_manager

Consolidate functions in ProtocolManagerHelper and put them inside
components/safe_browsing_db/util.*.
Separate BasePingManager and SafeBrowsingPingManager, where:
BasePingManager handles basic safe browsing pings such as HitReport,
ThreatDetails report etc.
Leave NotificationImageReport and PermissionReport to
SafeBrowsingPingManager due to their chrome/browser/ dependencies.
BUG=684805

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

[commit-bot: I haz the power, 2017-02-27 19:41:37]

Committed patchset #4 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/d06419a9457679d7daf3e513c3f7...