New issue
Advanced search Search tips

Issue 621647 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocking:
issue 695081



Sign in to add a comment

v4_local_database_manager.cc is getting built on Android (GN)

Project Member Reported by vakh@chromium.org, Jun 20 2016

Issue description

v4_local_database_manager.cc is meant to be build only for desktop platforms but is getting built for Android GN platforms.

 

Comment 1 by vakh@chromium.org, Jun 20 2016

And when you do, also remove the comment from the top of that file.

Comment 2 by huape...@amazon.com, Nov 12 2016

Hi, I am currently using this file for our V4 safe browsing implementation (Android). I made a few patches(instead of using remote db manager, I stub it with v4 local db manager in chrome/browser/safe_browsing/services_delegate_stub.cc). This v4 sb component works perfectly on Android(update and hash find). 

We don't have any play service and GSM on the device. Without this file, the component will no longer work on Android anymore. Will there be any alternative solution?

Comment 3 by vakh@chromium.org, Nov 28 2016

Yes v4_local_database_manager.cc works on Android since there is no platform specific code in it and I don't foresee adding any code that's platform specific.

However, please do not rely on it working on Android always since that's not supported on Android, so it may stop working in the future.

That being said, I'd welcome any patches that fix any breakages that happen on Android in the future.

Comment 4 by huape...@amazon.com, Dec 20 2016

Thanks!
Please keep this class. I will upstream any fixes if it breaks on Android.

Comment 5 by vakh@chromium.org, Jun 6 2017

Status: Started (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/25810dbb9351ffa60b96b1acc4ca92140179d09b

commit 25810dbb9351ffa60b96b1acc4ca92140179d09b
Author: Varun Khaneja <vakh@chromium.org>
Date: Mon Jun 12 18:16:39 2017

Re-factor the targets within safe_browsing_db for mobile and desktop.

The main change is to move most of the v4_* unit_test targets out of
the runners for mobile since v4_* is specifically targeting desktop
platforms.

Bug:  621647 
Change-Id: I8a8b1c28231b424c116b87c23659f18b1e5873c6
Reviewed-on: https://chromium-review.googlesource.com/524988
Commit-Queue: Varun Khaneja <vakh@chromium.org>
Reviewed-by: Jialiu Lin <jialiul@chromium.org>
Reviewed-by: Luke Z <lpz@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#478684}
[modify] https://crrev.com/25810dbb9351ffa60b96b1acc4ca92140179d09b/components/BUILD.gn
[modify] https://crrev.com/25810dbb9351ffa60b96b1acc4ca92140179d09b/components/safe_browsing_db/BUILD.gn

Comment 8 by vakh@chromium.org, Jun 12 2017

Blocking: 695081
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9c87bf21f52885c923c067ca043c97864ce70614

commit 9c87bf21f52885c923c067ca043c97864ce70614
Author: Varun Khaneja <vakh@chromium.org>
Date: Tue Jun 13 15:03:31 2017

Re-factor the targets within safe_browsing_db for mobile and desktop.

Include the //components/safe_browsing_db:safe_browsing_db target in
DEPS only for desktop platforms (i.e. safe_browsing_mode == 1), not for
mobile platforms (safe_browsing_mode == 2).

Bug:  590389 ,  621647 
Change-Id: Iad25c93861ee34603d24d925569a86c6a9b60538
Reviewed-on: https://chromium-review.googlesource.com/531786
Commit-Queue: Varun Khaneja <vakh@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479019}
[modify] https://crrev.com/9c87bf21f52885c923c067ca043c97864ce70614/chrome/browser/BUILD.gn
[modify] https://crrev.com/9c87bf21f52885c923c067ca043c97864ce70614/components/safe_browsing_db/BUILD.gn

Comment 10 by vakh@chromium.org, Jun 13 2017

Status: Fixed (was: Started)
For Android:
$ gn desc out/Debug components:components_unittests deps --tree | grep "g_db:v4_" | sed -e 's/^ *//g' -e 's/\.\.\.//g' | sort | uniq
//components/safe_browsing_db:v4_feature_list
//components/safe_browsing_db:v4_get_hash_protocol_manager
//components/safe_browsing_db:v4_protocol_manager_util
//components/safe_browsing_db:v4_test_util


$ gn desc out/Debug chrome/android:chrome deps --tree | grep "g_db:v4_" | sed -e 's/^ *//g' -e 's/\.\.\.//g' | sort | uniq
//components/safe_browsing_db:v4_feature_list
//components/safe_browsing_db:v4_get_hash_protocol_manager
//components/safe_browsing_db:v4_protocol_manager_util

Sign in to add a comment