New issue
Advanced search Search tips

Issue 773782 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

PVer4: Set client_states in FindFullHashesRequest

Project Member Reported by vakh@chromium.org, Oct 11 2017

Issue description

Ref: https://cs.chromium.org/chromium/src/components/safe_browsing/db/v4_get_hash_protocol_manager.cc?l=475&rcl=f9a99d0c0c87bdcb3db97977ed7f4c918b76fc53

The client_states field is not set in FindFullHashesRequest.
This is used by the backend for debugging (probably to detect any invalid states).
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 12 2017

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

commit 2222c6a37102cdba57404339c48cfb93f7719a73
Author: Varun Khaneja <vakh@chromium.org>
Date: Thu Oct 12 23:30:31 2017

PVer4: Set the client_states field in FindFullHashesRequest

The client_state is the state that Safe Browsing sent in the last
successfully applied update to each individual list being synced.

This information is used be the API team for debugging purposes.

Bug:  773782 
Change-Id: I6c7336378f0a0357f2568088128a0b854aba3ced
Reviewed-on: https://chromium-review.googlesource.com/716563
Commit-Queue: Varun Khaneja <vakh@chromium.org>
Reviewed-by: Jialiu Lin (Slow Oct 13-20, OOO Oct  21-29) <jialiul@chromium.org>
Reviewed-by: Luke Z <lpz@chromium.org>
Reviewed-by: Nathan Parker <nparker@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508533}
[modify] https://crrev.com/2222c6a37102cdba57404339c48cfb93f7719a73/components/safe_browsing/db/v4_get_hash_protocol_manager.cc
[modify] https://crrev.com/2222c6a37102cdba57404339c48cfb93f7719a73/components/safe_browsing/db/v4_get_hash_protocol_manager.h
[modify] https://crrev.com/2222c6a37102cdba57404339c48cfb93f7719a73/components/safe_browsing/db/v4_get_hash_protocol_manager_unittest.cc
[modify] https://crrev.com/2222c6a37102cdba57404339c48cfb93f7719a73/components/safe_browsing/db/v4_local_database_manager.cc
[modify] https://crrev.com/2222c6a37102cdba57404339c48cfb93f7719a73/components/safe_browsing/db/v4_local_database_manager.h
[modify] https://crrev.com/2222c6a37102cdba57404339c48cfb93f7719a73/components/safe_browsing/db/v4_local_database_manager_unittest.cc

Comment 2 by vakh@chromium.org, Oct 16 2017

Status: Fixed (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 20 2017

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

commit 6e0c02cb31234406e18df2c9fed341af1e4c47a2
Author: Varun Khaneja <vakh@chromium.org>
Date: Fri Oct 20 02:07:39 2017

Send correct client states with GetFullHashesWithApis.

This is a follow-up from https://crrev.com/c/716563.
In that CL, I incorrectly stated that the GetFullHashesWithApis call
will be removed when PVer3 is deprecated. It can't be because the
permissions code uses it on mobile also, where we don't use Chrome's
own PVer4 database fetching and management code.

This CL:
- Sends the client states of the SB lists being managed by Chrome.
  It is populated from the results returned by the virtual method
  GetStoreStateMap. This means that on mobile, it'll always return
  an empty list, but on desktop it'll return the current client states.
- Removes an incorrect comment about removing GetFullHashesWithApis
  after PVer4 rollout. It'll stay.

Bug:  773782 
Change-Id: Ic2c6f949329497d9ad28d702e9152336ac558f29
Reviewed-on: https://chromium-review.googlesource.com/723138
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Nathan Parker <nparker@chromium.org>
Commit-Queue: Varun Khaneja <vakh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510297}
[modify] https://crrev.com/6e0c02cb31234406e18df2c9fed341af1e4c47a2/components/safe_browsing/db/database_manager.cc
[modify] https://crrev.com/6e0c02cb31234406e18df2c9fed341af1e4c47a2/components/safe_browsing/db/database_manager.h
[modify] https://crrev.com/6e0c02cb31234406e18df2c9fed341af1e4c47a2/components/safe_browsing/db/v4_get_hash_protocol_manager.cc
[modify] https://crrev.com/6e0c02cb31234406e18df2c9fed341af1e4c47a2/components/safe_browsing/db/v4_get_hash_protocol_manager.h
[modify] https://crrev.com/6e0c02cb31234406e18df2c9fed341af1e4c47a2/components/safe_browsing/db/v4_get_hash_protocol_manager_unittest.cc
[modify] https://crrev.com/6e0c02cb31234406e18df2c9fed341af1e4c47a2/components/safe_browsing/db/v4_local_database_manager.cc
[modify] https://crrev.com/6e0c02cb31234406e18df2c9fed341af1e4c47a2/components/safe_browsing/db/v4_local_database_manager.h
[modify] https://crrev.com/6e0c02cb31234406e18df2c9fed341af1e4c47a2/components/safe_browsing/db/v4_protocol_manager_util.cc
[modify] https://crrev.com/6e0c02cb31234406e18df2c9fed341af1e4c47a2/components/safe_browsing/db/v4_protocol_manager_util.h
[modify] https://crrev.com/6e0c02cb31234406e18df2c9fed341af1e4c47a2/components/safe_browsing/db/v4_store.h

Sign in to add a comment