New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 761468 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug
Proj-Servicification

Blocking:
issue 715673



Sign in to add a comment

There are a few test failures when S13nSafeBrowsingParallelUrlCheck is turned on.

Project Member Reported by yzshen@chromium.org, Sep 1 2017

Issue description

Project Member

Comment 1 by bugdroid1@chromium.org, Sep 7 2017

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

commit 452f145d4e78f2338c8695f1c291bf5803294571
Author: Yuzhu Shen <yzshen@chromium.org>
Date: Thu Sep 07 17:41:16 2017

Fix browser test crashes when S13nSafeBrowsingParallelUrlCheck is turned on.

This CL handles the case where safe_browsing::BrowserURLLoaderThrottle is
notified even after it calls CancelWithError() on its delegate, which could
happen when CancelWithError() happens outside of the notification
handlers.

It fixes:
- V4SafeBrowsingServiceTest.Prefetch
- SafeBrowsingServiceTest.Prefetch

BUG= 761468 

Change-Id: If390bc5a6488a1205b93b249e6219494ea16ae19
Reviewed-on: https://chromium-review.googlesource.com/653203
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500329}
[modify] https://crrev.com/452f145d4e78f2338c8695f1c291bf5803294571/components/safe_browsing/browser/base_parallel_resource_throttle.cc
[modify] https://crrev.com/452f145d4e78f2338c8695f1c291bf5803294571/components/safe_browsing/browser/browser_url_loader_throttle.cc

Labels: SafeBrowsing-Triaged
Project Member

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

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

commit 2e68db72412c61f03cd33da42d89bf9abca1326b
Author: Yuzhu Shen <yzshen@chromium.org>
Date: Wed Oct 11 23:09:47 2017

SafeBrowsingBlockingPageTest: fix race condition of waiting for interstitial page to be ready.

The previous code may get the interstitial page while it hasn't loaded the actual contents (i.e., it is still about:blank).

Without this fix, the tests SecurityStateGoBackOnSubresourceInterstitial* become (more?) flaky when S13nSafeBrowsingParallelUrlCheck is enabled, since that feature changes timing of events.

BUG= 761468 

Change-Id: I0785bf5084b280f6185bc39cf1a4ac6331950f46
Reviewed-on: https://chromium-review.googlesource.com/713754
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Reviewed-by: Jialiu Lin <jialiul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508159}
[modify] https://crrev.com/2e68db72412c61f03cd33da42d89bf9abca1326b/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc

Project Member

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

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

commit c92c9adbab2ce1dadb6b594ae86e7fa91e156863
Author: Yuzhu Shen <yzshen@chromium.org>
Date: Thu Oct 12 19:56:17 2017

Fix prefetch browser tests for SafeBrowsing.

This CL fixes the following for the S13nSafeBrowsingParallelUrlCheck
feature:
- NoStatePrefetchBrowserTest/NoStatePrefetchBrowserTest.PrerenderSafeBrowsingSubresource/*
- NoStatePrefetchBrowserTest/NoStatePrefetchBrowserTest.PrerenderSafeBrowsingTopLevel/*

With the feature enabled, requests are started in parallel with
SafeBrowsing checks, therefore it is unreliable to check that unsafe requests
are not started (SafeBrowsing still guarantees to block the response).

BUG= 761468 

Change-Id: Ifa2f73879744fa9cf4c2400e11c64f1018007726
Reviewed-on: https://chromium-review.googlesource.com/711017
Reviewed-by: Egor Pasko <pasko@chromium.org>
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508402}
[modify] https://crrev.com/c92c9adbab2ce1dadb6b594ae86e7fa91e156863/chrome/browser/prerender/prerender_nostate_prefetch_browsertest.cc

Comment 5 by yzshen@chromium.org, Oct 19 2017

Status: Fixed (was: Started)

Comment 6 by laforge@google.com, Nov 7 2017

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.

Sign in to add a comment