New issue
Advanced search Search tips

Issue 682495 link

Starred by 1 user
Status: Fixed
Owner:
vakh@chromium.org
Closed: Feb 2017
Cc:
vakh@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocked on:
issue 689672


Sign in to add a comment

Ensure Pver4 has test coverage >= Pver3

Project Member Reported by nparker@chromium.org, Jan 19 2017 
The safe_Browsing_service_browsertest.cc code currently has parts that only work with Pver3 database manager.  We should switch that to work with Pver4, and ensure Pver4 test coverage is at least as good as Pver3.

One way to measure Pver3 dependencies is to hardcode  IsV4OnlyEnabled()==true and then run all the tests.  We should ensure pver4 tests are running on the bots before we roll out.

Comment 1 by vakh@chromium.org, Jan 26 2017

Status: Started (was: Untriaged)

Comment 2 by vakh@chromium.org, Jan 27 2017

Labels: SafeBrowsing-Triaged

Comment 3 by vakh@chromium.org, Feb 7 2017

Blockedon: 689672
Project Member

Comment 4 by bugdroid1@chromium.org, Feb 10 2017 

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

commit d9bfc537eac0e031058b6df63b4290a645318cf8
Author: vakh <vakh@chromium.org>
Date: Fri Feb 10 19:50:43 2017

Adding browser tests for PVer4.

Just adding some basic tests for now.
More coming soon (http://crrev.com/2684663002)

BUG= 682495 

Review-Url: https://codereview.chromium.org/2675063002
Cr-Commit-Position: refs/heads/master@{#449703}

[modify] https://crrev.com/d9bfc537eac0e031058b6df63b4290a645318cf8/chrome/browser/download/download_browsertest.cc
[modify] https://crrev.com/d9bfc537eac0e031058b6df63b4290a645318cf8/chrome/browser/loader/safe_browsing_resource_throttle.cc
[modify] https://crrev.com/d9bfc537eac0e031058b6df63b4290a645318cf8/chrome/browser/safe_browsing/safe_browsing_service.cc
[modify] https://crrev.com/d9bfc537eac0e031058b6df63b4290a645318cf8/chrome/browser/safe_browsing/safe_browsing_service.h
[modify] https://crrev.com/d9bfc537eac0e031058b6df63b4290a645318cf8/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
[modify] https://crrev.com/d9bfc537eac0e031058b6df63b4290a645318cf8/chrome/browser/safe_browsing/services_delegate.h
[modify] https://crrev.com/d9bfc537eac0e031058b6df63b4290a645318cf8/chrome/browser/safe_browsing/services_delegate_impl.cc
[modify] https://crrev.com/d9bfc537eac0e031058b6df63b4290a645318cf8/chrome/browser/safe_browsing/services_delegate_impl.h
[modify] https://crrev.com/d9bfc537eac0e031058b6df63b4290a645318cf8/chrome/browser/safe_browsing/services_delegate_stub.cc
[modify] https://crrev.com/d9bfc537eac0e031058b6df63b4290a645318cf8/chrome/browser/safe_browsing/services_delegate_stub.h
[modify] https://crrev.com/d9bfc537eac0e031058b6df63b4290a645318cf8/chrome/browser/safe_browsing/test_safe_browsing_service.cc
[modify] https://crrev.com/d9bfc537eac0e031058b6df63b4290a645318cf8/chrome/browser/safe_browsing/test_safe_browsing_service.h
[modify] https://crrev.com/d9bfc537eac0e031058b6df63b4290a645318cf8/components/safe_browsing_db/v4_database.cc
[modify] https://crrev.com/d9bfc537eac0e031058b6df63b4290a645318cf8/components/safe_browsing_db/v4_database.h
[modify] https://crrev.com/d9bfc537eac0e031058b6df63b4290a645318cf8/components/safe_browsing_db/v4_database_unittest.cc
[modify] https://crrev.com/d9bfc537eac0e031058b6df63b4290a645318cf8/components/safe_browsing_db/v4_feature_list.cc
[modify] https://crrev.com/d9bfc537eac0e031058b6df63b4290a645318cf8/components/safe_browsing_db/v4_feature_list.h
[modify] https://crrev.com/d9bfc537eac0e031058b6df63b4290a645318cf8/components/safe_browsing_db/v4_get_hash_protocol_manager.h
[modify] https://crrev.com/d9bfc537eac0e031058b6df63b4290a645318cf8/components/safe_browsing_db/v4_local_database_manager.cc
[modify] https://crrev.com/d9bfc537eac0e031058b6df63b4290a645318cf8/components/safe_browsing_db/v4_store.h
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 10 2017 

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

commit 97c42c445521b0f145afb0603bf7a38d4266e7dd
Author: hiroshige <hiroshige@chromium.org>
Date: Fri Feb 10 20:59:16 2017

Revert of Browser tests for using the new SafeBrowsing protocol (v4) (patchset #8 id:220001 of https://codereview.chromium.org/2675063002/ )

Reason for revert:
Suspected to cause sizes test failing due to static
initializers added to v4_database.cc.

BUG= 691069 

Original issue's description:
> Adding browser tests for PVer4.
>
> Just adding some basic tests for now.
> More coming soon (http://crrev.com/2684663002)
>
> BUG= 682495 
>
> Review-Url: https://codereview.chromium.org/2675063002
> Cr-Commit-Position: refs/heads/master@{#449703}
> Committed: https://chromium.googlesource.com/chromium/src/+/d9bfc537eac0e031058b6df63b4290a645318cf8

TBR=asanka@chromium.org,csharrison@chromium.org,nparker@chromium.org,shess@chromium.org,vakh@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 682495 

Review-Url: https://codereview.chromium.org/2687023007
Cr-Commit-Position: refs/heads/master@{#449731}

[modify] https://crrev.com/97c42c445521b0f145afb0603bf7a38d4266e7dd/chrome/browser/download/download_browsertest.cc
[modify] https://crrev.com/97c42c445521b0f145afb0603bf7a38d4266e7dd/chrome/browser/loader/safe_browsing_resource_throttle.cc
[modify] https://crrev.com/97c42c445521b0f145afb0603bf7a38d4266e7dd/chrome/browser/safe_browsing/safe_browsing_service.cc
[modify] https://crrev.com/97c42c445521b0f145afb0603bf7a38d4266e7dd/chrome/browser/safe_browsing/safe_browsing_service.h
[modify] https://crrev.com/97c42c445521b0f145afb0603bf7a38d4266e7dd/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
[modify] https://crrev.com/97c42c445521b0f145afb0603bf7a38d4266e7dd/chrome/browser/safe_browsing/services_delegate.h
[modify] https://crrev.com/97c42c445521b0f145afb0603bf7a38d4266e7dd/chrome/browser/safe_browsing/services_delegate_impl.cc
[modify] https://crrev.com/97c42c445521b0f145afb0603bf7a38d4266e7dd/chrome/browser/safe_browsing/services_delegate_impl.h
[modify] https://crrev.com/97c42c445521b0f145afb0603bf7a38d4266e7dd/chrome/browser/safe_browsing/services_delegate_stub.cc
[modify] https://crrev.com/97c42c445521b0f145afb0603bf7a38d4266e7dd/chrome/browser/safe_browsing/services_delegate_stub.h
[modify] https://crrev.com/97c42c445521b0f145afb0603bf7a38d4266e7dd/chrome/browser/safe_browsing/test_safe_browsing_service.cc
[modify] https://crrev.com/97c42c445521b0f145afb0603bf7a38d4266e7dd/chrome/browser/safe_browsing/test_safe_browsing_service.h
[modify] https://crrev.com/97c42c445521b0f145afb0603bf7a38d4266e7dd/components/safe_browsing_db/v4_database.cc
[modify] https://crrev.com/97c42c445521b0f145afb0603bf7a38d4266e7dd/components/safe_browsing_db/v4_database.h
[modify] https://crrev.com/97c42c445521b0f145afb0603bf7a38d4266e7dd/components/safe_browsing_db/v4_database_unittest.cc
[modify] https://crrev.com/97c42c445521b0f145afb0603bf7a38d4266e7dd/components/safe_browsing_db/v4_feature_list.cc
[modify] https://crrev.com/97c42c445521b0f145afb0603bf7a38d4266e7dd/components/safe_browsing_db/v4_feature_list.h
[modify] https://crrev.com/97c42c445521b0f145afb0603bf7a38d4266e7dd/components/safe_browsing_db/v4_get_hash_protocol_manager.h
[modify] https://crrev.com/97c42c445521b0f145afb0603bf7a38d4266e7dd/components/safe_browsing_db/v4_local_database_manager.cc
[modify] https://crrev.com/97c42c445521b0f145afb0603bf7a38d4266e7dd/components/safe_browsing_db/v4_store.h
Project Member

Comment 6 by bugdroid1@chromium.org, Feb 13 2017 

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

commit b397adc6b277d907e4c313797b44c78171d4d793
Author: vakh <vakh@chromium.org>
Date: Mon Feb 13 23:09:02 2017

Adding browser tests for PVer4.

Part 1: http://crrev.com/2675063002 (this CL)
Part 2: http://crrev.com/2684663002
Part 3: http://crrev.com/2683813002

BUG= 682495 

Review-Url: https://codereview.chromium.org/2675063002
Cr-Original-Commit-Position: refs/heads/master@{#449703}
Committed: https://chromium.googlesource.com/chromium/src/+/d9bfc537eac0e031058b6df63b4290a645318cf8
Review-Url: https://codereview.chromium.org/2675063002
Cr-Commit-Position: refs/heads/master@{#450138}

[modify] https://crrev.com/b397adc6b277d907e4c313797b44c78171d4d793/chrome/browser/download/download_browsertest.cc
[modify] https://crrev.com/b397adc6b277d907e4c313797b44c78171d4d793/chrome/browser/loader/safe_browsing_resource_throttle.cc
[modify] https://crrev.com/b397adc6b277d907e4c313797b44c78171d4d793/chrome/browser/safe_browsing/safe_browsing_service.cc
[modify] https://crrev.com/b397adc6b277d907e4c313797b44c78171d4d793/chrome/browser/safe_browsing/safe_browsing_service.h
[modify] https://crrev.com/b397adc6b277d907e4c313797b44c78171d4d793/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
[modify] https://crrev.com/b397adc6b277d907e4c313797b44c78171d4d793/chrome/browser/safe_browsing/services_delegate.h
[modify] https://crrev.com/b397adc6b277d907e4c313797b44c78171d4d793/chrome/browser/safe_browsing/services_delegate_impl.cc
[modify] https://crrev.com/b397adc6b277d907e4c313797b44c78171d4d793/chrome/browser/safe_browsing/services_delegate_impl.h
[modify] https://crrev.com/b397adc6b277d907e4c313797b44c78171d4d793/chrome/browser/safe_browsing/services_delegate_stub.cc
[modify] https://crrev.com/b397adc6b277d907e4c313797b44c78171d4d793/chrome/browser/safe_browsing/services_delegate_stub.h
[modify] https://crrev.com/b397adc6b277d907e4c313797b44c78171d4d793/chrome/browser/safe_browsing/test_safe_browsing_service.cc
[modify] https://crrev.com/b397adc6b277d907e4c313797b44c78171d4d793/chrome/browser/safe_browsing/test_safe_browsing_service.h
[modify] https://crrev.com/b397adc6b277d907e4c313797b44c78171d4d793/components/safe_browsing_db/v4_database.cc
[modify] https://crrev.com/b397adc6b277d907e4c313797b44c78171d4d793/components/safe_browsing_db/v4_database.h
[modify] https://crrev.com/b397adc6b277d907e4c313797b44c78171d4d793/components/safe_browsing_db/v4_database_unittest.cc
[modify] https://crrev.com/b397adc6b277d907e4c313797b44c78171d4d793/components/safe_browsing_db/v4_feature_list.cc
[modify] https://crrev.com/b397adc6b277d907e4c313797b44c78171d4d793/components/safe_browsing_db/v4_feature_list.h
[modify] https://crrev.com/b397adc6b277d907e4c313797b44c78171d4d793/components/safe_browsing_db/v4_get_hash_protocol_manager.h
[modify] https://crrev.com/b397adc6b277d907e4c313797b44c78171d4d793/components/safe_browsing_db/v4_local_database_manager.cc
[modify] https://crrev.com/b397adc6b277d907e4c313797b44c78171d4d793/components/safe_browsing_db/v4_store.h
Project Member

Comment 7 by bugdroid1@chromium.org, Feb 14 2017 

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

commit 0864e253f3e391b6c7d66c4ea70fbab27c8599e4
Author: vakh <vakh@chromium.org>
Date: Tue Feb 14 00:52:55 2017

More browser tests for PVer4.

Converts the existing PVer3 tests to PVer4 by *only* changing the way
prefixes and full hashes are stuffed into the DB and full hash cache
respectively. The rest of the lines in the tests are identical.

Part 1: http://crrev.com/2675063002
Part 2: http://crrev.com/2684663002 (this CL)
Part 3: http://crrev.com/2683813002

BUG= 682495 

Review-Url: https://codereview.chromium.org/2684663002
Cr-Commit-Position: refs/heads/master@{#450177}

[modify] https://crrev.com/0864e253f3e391b6c7d66c4ea70fbab27c8599e4/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc

Comment 9 by vakh@chromium.org, Feb 28 2017

Status: Fixed (was: Started)
►

Sign in to add a comment