New issue
Advanced search Search tips

Issue 792275 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

NoScript now missing info bar for HTTP->HTTPS redirection

Project Member Reported by dougarnett@chromium.org, Dec 6 2017

Issue description

Now have issues with PreviewsInfoBarTabHelper logic getting confused by LitePage bit in previews state.

Overall, it has been very fragile to try to get appropriate info bar behavior with small code changes. I'm starting to reconsider M-64 Dev experiment and trying to merge in fixes (instead do more proper change for M-65).
 
Labels: -M-65 M-64
Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 6 2017

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

commit fc89e8f72e41fb56e11c1ecc14a04054f54187c5
Author: Doug Arnett <dougarnett@chromium.org>
Date: Wed Dec 06 19:43:17 2017

Fixes issue with previews info bar not shown for redirect nav

There is an issue with not having the previews state bits cleared
per server previews response headers in the UI thread. This causes
an issue for an HTTP->HTTPS redirection that preforms NoScript
preview - the previews state confused the PreviewsInfoBarTabHelper.
This is a patch to fix M64 Dev channel. The proper fix (setting
previews state consistently is a separate change in progress for
M65).

Bug:  792275 
Change-Id: I28788c608ef37f208edbc72df7602294f56995f5
Reviewed-on: https://chromium-review.googlesource.com/811626
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Doug Arnett <dougarnett@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522170}
[modify] https://crrev.com/fc89e8f72e41fb56e11c1ecc14a04054f54187c5/chrome/browser/previews/previews_infobar_tab_helper.cc
[modify] https://crrev.com/fc89e8f72e41fb56e11c1ecc14a04054f54187c5/chrome/browser/previews/previews_infobar_tab_helper_unittest.cc

Labels: Merge-Request-64
Status: Fixed (was: Started)
Verified on Canary 65.0.3287.0 for http->https redirected navigations.

Good previews bar behavior now with this fix and one for 792133
Project Member

Comment 4 by sheriffbot@chromium.org, Dec 8 2017

Labels: -Merge-Request-64 Hotlist-Merge-Approved Merge-Approved-64
Your change meets the bar and is auto-approved for M64. Please go ahead and merge the CL to branch 3282 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 8 2017

Labels: -merge-approved-64 merge-merged-3282
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/042e0f66efeb33e0404290da4263f6dc9396e5d6

commit 042e0f66efeb33e0404290da4263f6dc9396e5d6
Author: Doug Arnett <dougarnett@chromium.org>
Date: Fri Dec 08 20:35:01 2017

Fixes issue with previews info bar not shown for redirect nav

There is an issue with not having the previews state bits cleared
per server previews response headers in the UI thread. This causes
an issue for an HTTP->HTTPS redirection that preforms NoScript
preview - the previews state confused the PreviewsInfoBarTabHelper.
This is a patch to fix M64 Dev channel. The proper fix (setting
previews state consistently is a separate change in progress for
M65).

Bug:  792275 
Change-Id: I28788c608ef37f208edbc72df7602294f56995f5
Reviewed-on: https://chromium-review.googlesource.com/811626
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Doug Arnett <dougarnett@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#522170}(cherry picked from commit fc89e8f72e41fb56e11c1ecc14a04054f54187c5)
Reviewed-on: https://chromium-review.googlesource.com/818174
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Cr-Commit-Position: refs/branch-heads/3282@{#103}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[modify] https://crrev.com/042e0f66efeb33e0404290da4263f6dc9396e5d6/chrome/browser/previews/previews_infobar_tab_helper.cc
[modify] https://crrev.com/042e0f66efeb33e0404290da4263f6dc9396e5d6/chrome/browser/previews/previews_infobar_tab_helper_unittest.cc

Sign in to add a comment