Unify safe_browsing* components [1]: move safe_browsing_prefs*

Unify safe_browsing* components [1]: move safe_browsing_prefs*

This patch performs a step towards unification of safe_browsing_db
and safe_browsing components by moving safe_browsing_prefs* to
safe_browsing/common/ as discussed in the design doc
(https://goo.gl/2R6K3G).

BUG=668515
TBR=thakis@chromium.org

Review-Url: https://codereview.chromium.org/2869383002
Cr-Commit-Position: refs/heads/master@{#471824}
Committed: https://chromium.googlesource.com/chromium/src/+/89cf1171a6451dea5588e7b053a24d3a64a77f82

[timvolodine, 2017-05-10 13:07:22]

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

[commit-bot: I haz the power, 2017-05-10 13:07:48]

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-05-10 13:12:00]

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

[commit-bot: I haz the power, 2017-05-10 13:12:01]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[timvolodine, 2017-05-11 11:36:02]

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

[commit-bot: I haz the power, 2017-05-11 11:36:39]

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-05-11 11:40:57]

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

[commit-bot: I haz the power, 2017-05-11 11:40:58]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[timvolodine, 2017-05-11 11:50:11]

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

[commit-bot: I haz the power, 2017-05-11 11:50:28]

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-05-11 12:28:03]

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

[commit-bot: I haz the power, 2017-05-11 12:28:04]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[timvolodine, 2017-05-11 12:37:26]

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

[commit-bot: I haz the power, 2017-05-11 12:37:55]

Dry run: CQ is trying da patch.

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

[timvolodine, 2017-05-11 13:55:35]

timvolodine@chromium.org changed reviewers:
+ lpz@chromium.org, vakh@chromium.org

[timvolodine, 2017-05-11 13:55:36]

lpz@: for safe_browsing_prefs*
vakh@: as owner of safe_browsing_db

+cc:nparker@,jialiu@

[timvolodine, 2017-05-11 13:58:00]

Description was changed from

==========
Unify safe_browsing components [1]: move safe_browsing_prefs*

This patch performs a step towards unification of safe_browsing_db
and safe_browsing components by moving safe_browsing_prefs* to
safe_browsing/common/ as discussed in the design doc
(https://goo.gl/2R6K3G).

BUG=668515
==========

to

==========
Unify safe_browsing* components [1]: move safe_browsing_prefs*

This patch performs a step towards unification of safe_browsing_db
and safe_browsing components by moving safe_browsing_prefs* to
safe_browsing/common/ as discussed in the design doc
(https://goo.gl/2R6K3G).

BUG=668515
==========

[commit-bot: I haz the power, 2017-05-11 14:35:26]

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

[commit-bot: I haz the power, 2017-05-11 14:35:27]

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

[lpz, 2017-05-11 18:17:36]

Whew, thanks for tracking down all these references!

https://codereview.chromium.org/2869383002/diff/60001/components/safe_browsin...
File components/safe_browsing/common/safe_browsing_prefs.cc (right):

https://codereview.chromium.org/2869383002/diff/60001/components/safe_browsin...
components/safe_browsing/common/safe_browsing_prefs.cc:5: #include
"components/safe_browsing/common/safe_browsing_prefs.h"
nit: 1 blank line after

https://codereview.chromium.org/2869383002/diff/60001/components/safe_browsin...
File components/safe_browsing_db/android/safe_browsing_api_handler_bridge.h
(right):

https://codereview.chromium.org/2869383002/diff/60001/components/safe_browsin...
components/safe_browsing_db/android/safe_browsing_api_handler_bridge.h:7:
#ifndef COMPONENTS_SAFE_BROWSING_ANDROID_SAFE_BROWSING_API_HANDLER_BRIDGE_H_
should this change be made even if the file didn't move?

[timvolodine, 2017-05-12 11:26:53]

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

[commit-bot: I haz the power, 2017-05-12 11:27:14]

Dry run: CQ is trying da patch.

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

[timvolodine, 2017-05-12 11:31:09]

Thanks for the review Luke, PTAL.

https://codereview.chromium.org/2869383002/diff/60001/components/safe_browsin...
File components/safe_browsing/common/safe_browsing_prefs.cc (right):

https://codereview.chromium.org/2869383002/diff/60001/components/safe_browsin...
components/safe_browsing/common/safe_browsing_prefs.cc:5: #include
"components/safe_browsing/common/safe_browsing_prefs.h"
On 2017/05/11 18:17:35, lpz wrote:
> nit: 1 blank line after

Done.

https://codereview.chromium.org/2869383002/diff/60001/components/safe_browsin...
File components/safe_browsing_db/android/safe_browsing_api_handler_bridge.h
(right):

https://codereview.chromium.org/2869383002/diff/60001/components/safe_browsin...
components/safe_browsing_db/android/safe_browsing_api_handler_bridge.h:7:
#ifndef COMPONENTS_SAFE_BROWSING_ANDROID_SAFE_BROWSING_API_HANDLER_BRIDGE_H_
On 2017/05/11 18:17:36, lpz wrote:
> should this change be made even if the file didn't move?

ah yes, sorry, thanks for catching this, this shouldn't change (yet). reverted.
done.

[commit-bot: I haz the power, 2017-05-12 13:35:43]

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

[commit-bot: I haz the power, 2017-05-12 13:35:44]

Dry run: This issue passed the CQ dry run.

[lpz, 2017-05-12 13:45:52]

LGTM aside from the c/s_b_db/android build file question.

https://codereview.chromium.org/2869383002/diff/80001/components/safe_browsin...
File components/safe_browsing_db/android/BUILD.gn (right):

https://codereview.chromium.org/2869383002/diff/80001/components/safe_browsin...
components/safe_browsing_db/android/BUILD.gn:23: jni_package =
"components/safe_browsing/android"
Should we undo this as well?

[timvolodine, 2017-05-12 15:02:00]

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

[commit-bot: I haz the power, 2017-05-12 15:02:45]

Dry run: CQ is trying da patch.

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

[timvolodine, 2017-05-12 15:04:14]

thanks!

https://codereview.chromium.org/2869383002/diff/80001/components/safe_browsin...
File components/safe_browsing_db/android/BUILD.gn (right):

https://codereview.chromium.org/2869383002/diff/80001/components/safe_browsin...
components/safe_browsing_db/android/BUILD.gn:23: jni_package =
"components/safe_browsing/android"
On 2017/05/12 13:45:52, lpz wrote:
> Should we undo this as well?

Yes, of course, sorry the few android/ related changes got mixed up in this
patch.

Done.

[timvolodine, 2017-05-12 15:05:06]

vakh@ : does this look ok to you?

[timvolodine, 2017-05-12 15:24:32]

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

[timvolodine, 2017-05-12 15:24:33]

+jialiul@: for chrome/browser/safe_browsing and
components/security_interstitials/

[Jialiu Lin, 2017-05-12 16:59:22]

LGTM for c/b/safe_browsing and components/security_interstitials

[commit-bot: I haz the power, 2017-05-12 17:22:44]

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

[commit-bot: I haz the power, 2017-05-12 17:22:45]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[vakh (use Gerrit instead), 2017-05-12 17:28:14]

lgtm

https://codereview.chromium.org/2869383002/diff/100001/components/safe_browsi...
File components/safe_browsing_db/DEPS (right):

https://codereview.chromium.org/2869383002/diff/100001/components/safe_browsi...
components/safe_browsing_db/DEPS:4: "+components/safe_browsing",
it seems odd to me that safe_browsing_db depends on safe_browsing. This could
end up in a cyclic dependency.

[timvolodine, 2017-05-12 17:40:12]

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

[commit-bot: I haz the power, 2017-05-12 17:40:58]

Dry run: CQ is trying da patch.

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

[timvolodine, 2017-05-12 17:50:51]

https://codereview.chromium.org/2869383002/diff/100001/components/safe_browsi...
File components/safe_browsing_db/DEPS (right):

https://codereview.chromium.org/2869383002/diff/100001/components/safe_browsi...
components/safe_browsing_db/DEPS:4: "+components/safe_browsing",
On 2017/05/12 17:28:14, vakh (Varun Khaneja) wrote:
> it seems odd to me that safe_browsing_db depends on safe_browsing. This could
> end up in a cyclic dependency.

yes this is obviously a temporary situation, I've also modified it to be more
specific to avoid misuse:
  "+components/safe_browsing/common/safe_browsing_prefs.h"

[timvolodine, 2017-05-12 17:59:42]

timvolodine@chromium.org changed reviewers:
+ jam@chromium.org

[timvolodine, 2017-05-12 17:59:43]

+jam@: for
chrome/browser/android/preferences/pref_service_bridge.cc
chrome/browser/extensions/BUILD.gn
components/BUILD.gn
chrome/browser/profiles/profile.cc

I'll probably TBR chrome/browser/ssl/, chrome/browser/ui/,
chrome/browser/extensions/ since the change there is only the include path.

[timvolodine, 2017-05-12 18:02:53]

timvolodine@chromium.org changed reviewers:
+ blundell@chromium.org

[timvolodine, 2017-05-12 18:02:53]

correction:
+blundell@: for components/BUILD.gn

[commit-bot: I haz the power, 2017-05-12 19:12:36]

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

[commit-bot: I haz the power, 2017-05-12 19:12:38]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[jam, 2017-05-12 19:54:04]

On 2017/05/12 17:59:43, timvolodine wrote:
> +jam@: for
> chrome/browser/android/preferences/pref_service_bridge.cc
> chrome/browser/extensions/BUILD.gn
> components/BUILD.gn
> chrome/browser/profiles/profile.cc
> 
> I'll probably TBR chrome/browser/ssl/, chrome/browser/ui/,
> chrome/browser/extensions/ since the change there is only the include path.

lgtm for all these files

[jam, 2017-05-12 19:54:13]

The CQ bit was checked by jam@chromium.org

[jam, 2017-05-12 19:54:14]

The patchset sent to the CQ was uploaded after l-g-t-m from lpz@chromium.org,
vakh@chromium.org, jialiul@chromium.org
Link to the patchset: https://codereview.chromium.org/2869383002/#ps120001
(title: "modify safe_browsing_db/DEPS")

[commit-bot: I haz the power, 2017-05-12 19:54:51]

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-05-12 19:59:30]

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

[commit-bot: I haz the power, 2017-05-12 19:59:32]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[blundell, 2017-05-15 07:08:38]

lgtm

[timvolodine, 2017-05-15 12:54:49]

Description was changed from

==========
Unify safe_browsing* components [1]: move safe_browsing_prefs*

This patch performs a step towards unification of safe_browsing_db
and safe_browsing components by moving safe_browsing_prefs* to
safe_browsing/common/ as discussed in the design doc
(https://goo.gl/2R6K3G).

BUG=668515
==========

to

==========
Unify safe_browsing* components [1]: move safe_browsing_prefs*

This patch performs a step towards unification of safe_browsing_db
and safe_browsing components by moving safe_browsing_prefs* to
safe_browsing/common/ as discussed in the design doc
(https://goo.gl/2R6K3G).

BUG=668515
TBR=thakis@chromium.org
==========

[timvolodine, 2017-05-15 12:56:05]

thanks everybody!

+TBR: thakis@ for remaining chrome/browser files (mechanical changes)
chrome/browser/ssl, chrome/browser/ui, chrome/browser/extensions.

[timvolodine, 2017-05-15 12:56:19]

The CQ bit was checked by timvolodine@chromium.org

[timvolodine, 2017-05-15 12:56:19]

The patchset sent to the CQ was uploaded after l-g-t-m from lpz@chromium.org,
vakh@chromium.org, jam@chromium.org, jialiul@chromium.org, blundell@chromium.org
Link to the patchset: https://codereview.chromium.org/2869383002/#ps140001
(title: "rebase")

[commit-bot: I haz the power, 2017-05-15 12:56:31]

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-05-15 13:32:25]

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

[commit-bot: I haz the power, 2017-05-15 13:32:27]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[timvolodine, 2017-05-15 14:51:40]

The CQ bit was checked by timvolodine@chromium.org

[timvolodine, 2017-05-15 14:51:41]

The patchset sent to the CQ was uploaded after l-g-t-m from lpz@chromium.org,
vakh@chromium.org, jam@chromium.org, jialiul@chromium.org, blundell@chromium.org
Link to the patchset: https://codereview.chromium.org/2869383002/#ps160001
(title: "fix win bots")

[commit-bot: I haz the power, 2017-05-15 14:52:05]

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-05-15 18:05:10]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1494859900687030,
"parent_rev": "d0602af69e19b44bbbbc64165766098d9c9e36a6", "commit_rev":
"89cf1171a6451dea5588e7b053a24d3a64a77f82"}

[commit-bot: I haz the power, 2017-05-15 18:05:26]

Description was changed from

==========
Unify safe_browsing* components [1]: move safe_browsing_prefs*

This patch performs a step towards unification of safe_browsing_db
and safe_browsing components by moving safe_browsing_prefs* to
safe_browsing/common/ as discussed in the design doc
(https://goo.gl/2R6K3G).

BUG=668515
TBR=thakis@chromium.org
==========

to

==========
Unify safe_browsing* components [1]: move safe_browsing_prefs*

This patch performs a step towards unification of safe_browsing_db
and safe_browsing components by moving safe_browsing_prefs* to
safe_browsing/common/ as discussed in the design doc
(https://goo.gl/2R6K3G).

BUG=668515
TBR=thakis@chromium.org

Review-Url: https://codereview.chromium.org/2869383002
Cr-Commit-Position: refs/heads/master@{#471824}
Committed:
https://chromium.googlesource.com/chromium/src/+/89cf1171a6451dea5588e7b053a2...
==========

[commit-bot: I haz the power, 2017-05-15 18:05:30]

Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/89cf1171a6451dea5588e7b053a2...