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

Issue 655794 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Checkbox disappears when safe browsing interstitial is reloaded

Project Member Reported by nparker@chromium.org, Oct 13 2016

Issue description

Version: At least 53.0.2785.143 (stable) and 55.0.2883.11 (dev)
OS: Linux

What steps will reproduce the problem?
* Visit http://testsafebrowsing.appspot.com/, click the first phishing link
* Note the extended-reporting checkbox shows.
* Hit reload, and it disappears

What is the expected output?
Checkbox should remain

 
The bug doesn't manifest if it is a _subresource_ that triggers the warning. 
e.g.: http://parkerly.com/sb-tests/bad-subresources.html

Comment 2 by mea...@chromium.org, Oct 13 2016

Cc: mea...@chromium.org
Owner: mattm@chromium.org
Status: Assigned (was: Untriaged)
Bisect result:
You are probably looking for a change made after 366187 (known good), but no later than 366203 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/f2c63b1954b5683b65ce3760d64a3c4db4c8c352..ee0fb4ddd63628f501863401af0976b3047ba1a5

mattm: https://codereview.chromium.org/1509073002 looks the most likely CL in that range. Can you please take a look?
mattm -- Can you take a look? If not let me know and I'll reassign.

Comment 4 by mattm@chromium.org, Nov 17 2016

Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 30 2016

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

commit 3294450834f7ae8978994a8d2f7f9dd1df3178f2
Author: mattm <mattm@chromium.org>
Date: Wed Nov 30 22:23:39 2016

If replacing a safebrowsing interstitial, don't call DontProceed on the old one.

Calling DontProceed directly has some side effects like destroying the pending NavigationEntry.
There is already common code in InterstitialPageImpl::Show to handle closing an existing interstitial when showing a new one, and it properly handles the pending NavigationEntry.

BUG= 655794 

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

[modify] https://crrev.com/3294450834f7ae8978994a8d2f7f9dd1df3178f2/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
[modify] https://crrev.com/3294450834f7ae8978994a8d2f7f9dd1df3178f2/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc

Comment 6 by mattm@chromium.org, Nov 30 2016

Status: Fixed (was: Started)

Sign in to add a comment