New issue
Advanced search Search tips

Issue 863569 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 17
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Removing ReloadType::DISABLE_PREVIEWS

Project Member Reported by ryansturm@chromium.org, Jul 13

Issue description

Due to blacklist logic around previews, this reload type is not needed, and adds further unneeded complication to the code base. We should remove it entirely.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 17

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

commit c4da1996b401c3c057c2fcee7c603d4772f4e8d7
Author: Ryan Sturm <ryansturm@chromium.org>
Date: Tue Jul 17 16:59:01 2018

Removing ReloadType::DISABLE_PREVIEWS

This type was introduced for server LoFi before Previews had
blacklisting support. It did 3 things: bypassed cache, loaded original
request, and disabled previews.

Previews are disabled by the 5 minute rule after an opt out, so that
functionality is covered (if users have disabled their blacklist, they
will not be served the preview because of a new 5 second rule, but this
scenario is not supported by default).

Bypass cache is not important, as LoFi images, litepage, etc are using
vary to control resources based on added headers, so not setting the
various previews headers handles this (in the case of client LoFi,
range requests are used instead of vary).

Since we are using a reload type that gets the original requested URL,
we handle that case as well.

This also adds dependency injected Clock for testing the ignored
blacklist functionality.

Bug:  863569 
Change-Id: Ia1f05f6242522f883903742301025fb6d3c71044
Reviewed-on: https://chromium-review.googlesource.com/1137022
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Ryan Sturm <ryansturm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575685}
[modify] https://crrev.com/c4da1996b401c3c057c2fcee7c603d4772f4e8d7/chrome/browser/previews/previews_infobar_delegate.cc
[modify] https://crrev.com/c4da1996b401c3c057c2fcee7c603d4772f4e8d7/chrome/browser/previews/previews_infobar_delegate_unittest.cc
[modify] https://crrev.com/c4da1996b401c3c057c2fcee7c603d4772f4e8d7/chrome/browser/previews/previews_service_unittest.cc
[modify] https://crrev.com/c4da1996b401c3c057c2fcee7c603d4772f4e8d7/chrome/browser/profiles/profile_impl_io_data.cc
[modify] https://crrev.com/c4da1996b401c3c057c2fcee7c603d4772f4e8d7/chrome/browser/ui/webui/interventions_internals/interventions_internals_page_handler_unittest.cc
[modify] https://crrev.com/c4da1996b401c3c057c2fcee7c603d4772f4e8d7/components/previews/content/previews_decider_impl.cc
[modify] https://crrev.com/c4da1996b401c3c057c2fcee7c603d4772f4e8d7/components/previews/content/previews_decider_impl.h
[modify] https://crrev.com/c4da1996b401c3c057c2fcee7c603d4772f4e8d7/components/previews/content/previews_decider_impl_unittest.cc
[modify] https://crrev.com/c4da1996b401c3c057c2fcee7c603d4772f4e8d7/components/previews/content/previews_ui_service_unittest.cc
[modify] https://crrev.com/c4da1996b401c3c057c2fcee7c603d4772f4e8d7/content/browser/frame_host/navigation_controller_impl.cc
[modify] https://crrev.com/c4da1996b401c3c057c2fcee7c603d4772f4e8d7/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/c4da1996b401c3c057c2fcee7c603d4772f4e8d7/content/browser/frame_host/navigation_controller_impl_unittest.cc
[modify] https://crrev.com/c4da1996b401c3c057c2fcee7c603d4772f4e8d7/content/browser/loader/loader_browsertest.cc
[modify] https://crrev.com/c4da1996b401c3c057c2fcee7c603d4772f4e8d7/content/public/browser/reload_type.h

Status: Fixed (was: Started)

Sign in to add a comment