New issue
Advanced search Search tips

Issue 816964 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Improve Previews whitelist and blacklist checks on committed URLs

Project Member Reported by dougarnett@chromium.org, Feb 27 2018

Issue description

For  crbug.com/787191 , we added a committed URL check using the 
ShouldAllowPreviewAtECT() interface with EFFECTIVE_CONNECTION_TYPE_LAST to avoid constraining by ECT. This performs more checks and logging than needed. Consider adding a new PreviewsDecider method targetted to just the URL blacklist/whitelist checks desired on the committed URL and use that instead.
 
Topic - do we want to re-log any/all checks done at commit-time to interventions-internals and also log UMA counts for eligibility reasons (ie, each request might have 2 ALLOWED counts in histogram)?
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 28 2018

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

commit 02991edfc56a622c1b32803d63b7eccfe0a9c868
Author: Doug Arnett <dougarnett@chromium.org>
Date: Wed Feb 28 19:36:30 2018

Adds new PreviewsDecider method to check URL against blacklist/whitelist

New interface method PreviewsDecider::IsURLAllowedForPreview() checks
a URL against the local blacklist and optimization hints whitelist if
applicable. This reduced check is now used on the committed URL for
NoScript (refactor from using ShouldAllowPreviewAtEct() at commit time).
This reduces the preview logging cruft that the current commit time
check adds.


Bug:  816964 
Change-Id: I2e81d06d3397cd3f3f7727f658ee8818c22e122c
Reviewed-on: https://chromium-review.googlesource.com/922459
Reviewed-by: Peter Williamson <petewil@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Doug Arnett <dougarnett@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539925}
[modify] https://crrev.com/02991edfc56a622c1b32803d63b7eccfe0a9c868/chrome/browser/offline_pages/offline_page_request_job_unittest.cc
[modify] https://crrev.com/02991edfc56a622c1b32803d63b7eccfe0a9c868/components/previews/content/previews_content_util.cc
[modify] https://crrev.com/02991edfc56a622c1b32803d63b7eccfe0a9c868/components/previews/content/previews_content_util_unittest.cc
[modify] https://crrev.com/02991edfc56a622c1b32803d63b7eccfe0a9c868/components/previews/content/previews_io_data.cc
[modify] https://crrev.com/02991edfc56a622c1b32803d63b7eccfe0a9c868/components/previews/content/previews_io_data.h
[modify] https://crrev.com/02991edfc56a622c1b32803d63b7eccfe0a9c868/components/previews/content/previews_io_data_unittest.cc
[modify] https://crrev.com/02991edfc56a622c1b32803d63b7eccfe0a9c868/components/previews/core/previews_decider.h
[modify] https://crrev.com/02991edfc56a622c1b32803d63b7eccfe0a9c868/components/previews/core/test_previews_decider.cc
[modify] https://crrev.com/02991edfc56a622c1b32803d63b7eccfe0a9c868/components/previews/core/test_previews_decider.h

Status: Fixed (was: Assigned)

Sign in to add a comment