|
|
Created:
3 years, 10 months ago by vakh (use Gerrit instead) Modified:
3 years, 10 months ago CC:
chromium-reviews, grt+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMore 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}
Committed: https://chromium.googlesource.com/chromium/src/+/0864e253f3e391b6c7d66c4ea70fbab27c8599e4
Patch Set 1 #Patch Set 2 : More tests, including SubResource ones #Patch Set 3 : rebase + fix failures after rebase #
Total comments: 3
Patch Set 4 : rebase #Patch Set 5 : nparker@'s review #Messages
Total messages: 34 (25 generated)
The CQ bit was checked by vakh@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
vakh@chromium.org changed reviewers: + nparker@chromium.org, shess@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
More tests, including SubResource ones
The CQ bit was checked by vakh@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
rebase + fix failures after rebase
The CQ bit was checked by vakh@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm I didn't look super closely at the test contents. https://codereview.chromium.org/2684663002/diff/40001/chrome/browser/safe_bro... File chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc (right): https://codereview.chromium.org/2684663002/diff/40001/chrome/browser/safe_bro... chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc:666: std::string exp_group = prefetch ? "ExperimentYes" : "ExperimentNo"; I realize you just moved this code, but this seems fragile, and maybe already broken. I can't find what code uses this experiment. I see only test code: https://cs.chromium.org/search/?q=%5C%22Prefetch%5C%22+Experiment(Yes%7CNo)&t... If we drop the FieldTrialList code, does it still pass? https://codereview.chromium.org/2684663002/diff/40001/chrome/browser/safe_bro... chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc:888: SetPrefetchForTest instance(true); nit: maybe scoped_prefetch_setting(true) so it looks less confusing. https://codereview.chromium.org/2684663002/diff/40001/chrome/browser/safe_bro... chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc:2189: EXPECT_CALL(*driver(), ActivateForProvisionalLoad(::testing::_, ::testing::_, You've already got a "using" for this, so just s/::testing::_/_/ globally.
The CQ bit was checked by vakh@chromium.org to run a CQ dry run
rebase
The CQ bit was checked by vakh@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
nparker@'s review
The CQ bit was checked by vakh@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by vakh@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from nparker@chromium.org Link to the patchset: https://codereview.chromium.org/2684663002/#ps80001 (title: "nparker@'s review")
The CQ bit was unchecked by commit-bot@chromium.org
This CL has an open dependency (Issue 2675063002 Patch 280001). Please resolve the dependency and try again. If you are sure that there is no real dependency, please use one of the options listed in https://goo.gl/9Es4OR to land the CL.
Description was changed from ========== 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. Follow-up from http://crrev.com/2675063002 BUG=682495 ========== to ========== 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 ==========
The CQ bit was checked by vakh@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 80001, "attempt_start_ts": 1487027433388580, "parent_rev": "411d4e0b873c2ca5da19d1dc70e56e3828c08a5e", "commit_rev": "0864e253f3e391b6c7d66c4ea70fbab27c8599e4"}
Message was sent while issue was closed.
Description was changed from ========== 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 ========== to ========== 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} Committed: https://chromium.googlesource.com/chromium/src/+/0864e253f3e391b6c7d66c4ea70f... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/0864e253f3e391b6c7d66c4ea70f... |