New issue
Advanced search Search tips

Issue 919188 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 4
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Data Reduction Proxy metrics observer records data when an HTTPS server preview is served from cache

Project Member Reported by ryansturm@google.com, Jan 4

Issue description

Because the code uses a combination cached and chrome proxy header check when NetworkService is turned on to determine cached DRP responses, this will include cached HTTPS server preview responses. This should also check that the scheme is HTTP to rule out cached HTTPS server preview checks.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 4

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

commit e821a000c61f3069aa7092227c7aefa645a2602b
Author: Robert Ogden <robertogden@chromium.org>
Date: Fri Jan 04 22:23:35 2019

Only mark a response as DRP-cached if it was HTTP

HTTPS should not be included because of the overlap with Lite Page
Redirect previews.

Bug:  919188 
Change-Id: Ia3df106c0a2be2b5404f1bb8379894b6d13d0382
Reviewed-on: https://chromium-review.googlesource.com/c/1396608
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Robert Ogden <robertogden@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620084}
[modify] https://crrev.com/e821a000c61f3069aa7092227c7aefa645a2602b/chrome/browser/data_reduction_proxy/data_reduction_proxy_chrome_settings.cc
[modify] https://crrev.com/e821a000c61f3069aa7092227c7aefa645a2602b/chrome/browser/data_reduction_proxy/data_reduction_proxy_chrome_settings_unittest.cc

Status: Fixed (was: Assigned)
Labels: -M-74 M-73
Labels: Merge-Request-72
We'd like to merge the above CL to 72. It is metrics for a feature we're launching in 72. The change is pretty small and has been stable in canary and dev
Project Member

Comment 5 by sheriffbot@chromium.org, Jan 11

Labels: -Merge-Request-72 Merge-Review-72 Hotlist-Merge-Review
This bug requires manual review: M72 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: OS-Android
Labels: -Merge-Review-72 Merge-Approved-72
Approving merge to M72 branch 3626 based on comment #4 and per chat with  robertogden@, this feature is behind finch.
Labels: -Merge-Approved-72 Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/5c3cb84f6fc0577b19366cf64933d1bc9503f407

Commit: 5c3cb84f6fc0577b19366cf64933d1bc9503f407
Author: robertogden@chromium.org
Commiter: robertogden@chromium.org
Date: 2019-01-11 19:20:52 +0000 UTC

Only mark a response as DRP-cached if it was HTTP

HTTPS should not be included because of the overlap with Lite Page
Redirect previews.

Bug:  919188 
Change-Id: Ia3df106c0a2be2b5404f1bb8379894b6d13d0382
Reviewed-on: https://chromium-review.googlesource.com/c/1396608
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Robert Ogden <robertogden@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#620084}
Reviewed-on: https://chromium-review.googlesource.com/c/1407386
Reviewed-by: Robert Ogden <robertogden@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#650}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
Project Member

Comment 9 by bugdroid1@chromium.org, Jan 11

Labels: merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5c3cb84f6fc0577b19366cf64933d1bc9503f407

commit 5c3cb84f6fc0577b19366cf64933d1bc9503f407
Author: Robert Ogden <robertogden@chromium.org>
Date: Fri Jan 11 19:20:52 2019

Only mark a response as DRP-cached if it was HTTP

HTTPS should not be included because of the overlap with Lite Page
Redirect previews.

Bug:  919188 
Change-Id: Ia3df106c0a2be2b5404f1bb8379894b6d13d0382
Reviewed-on: https://chromium-review.googlesource.com/c/1396608
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Robert Ogden <robertogden@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#620084}
Reviewed-on: https://chromium-review.googlesource.com/c/1407386
Reviewed-by: Robert Ogden <robertogden@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#650}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/5c3cb84f6fc0577b19366cf64933d1bc9503f407/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.cc
[modify] https://crrev.com/5c3cb84f6fc0577b19366cf64933d1bc9503f407/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings_unittest.cc

Sign in to add a comment