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

Issue 615434 link

Starred by 2 users
Status: Fixed
Owner:
jkarlin@chromium.org
Closed: Jul 2016
Cc:
█ twif...@chromium.org
mmenke@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Expose more detailed cache information in URLRequest

Project Member Reported by jkarlin@chromium.org, May 27 2016 
Net clients are requesting more detail about the cache, such as the cache pattern. Where should it be exposed? URLRequest already has a was_cached() function, so perhaps we should at it there.

Comment 1 by jkarlin@chromium.org, May 27 2016

Labels: -Pri-3 Pri-2
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 26 2016 

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

commit 19ff6f1bd9d1ea24e3104d0cbec6352640545d8c
Author: jamartin <jamartin@chromium.org>
Date: Tue Jul 26 23:37:41 2016

Exposing CacheEntryStatus (former TransactionPattern) via UrlRequest

This will allow to know how the UrlRequest affected the cache.

Currently, it was possible to retrieve:
                           cache used = was_cached && !network_accessed
                      cache validated = was_cached && network_accessed
  not used/updated/cantconditionalize = !was_cached

This change allows to tell apart the not used/updated/cantconditionalize
sub-categories.

This change has the added benefit that HttpCacheTransaction::RecordHistograms
could be refactorized out of that class and moved to upper layers.

BUG= 615434 

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

[modify] https://crrev.com/19ff6f1bd9d1ea24e3104d0cbec6352640545d8c/net/http/http_cache_transaction.cc
[modify] https://crrev.com/19ff6f1bd9d1ea24e3104d0cbec6352640545d8c/net/http/http_cache_transaction.h
[modify] https://crrev.com/19ff6f1bd9d1ea24e3104d0cbec6352640545d8c/net/http/http_cache_unittest.cc
[modify] https://crrev.com/19ff6f1bd9d1ea24e3104d0cbec6352640545d8c/net/http/http_response_info.cc
[modify] https://crrev.com/19ff6f1bd9d1ea24e3104d0cbec6352640545d8c/net/http/http_response_info.h

Comment 4 by twif...@chromium.org, Jul 27 2016

Status: Fixed (was: Assigned)
Since precache was the net client in question, I think it's safe to mark this as fixed. :)

Comment 5 by twif...@chromium.org, Aug 1 2016

Labels: Merge-Request-53
Not stability or security-related, but a minimal change that would be useful for our experiment (a metric requested in our launch bug 309216). Thanks for your consideration!

Comment 6 by dimu@chromium.org, Aug 1 2016

Labels: -Merge-Request-53 Merge-Approved-53 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M53 (branch: 2785)
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 1 2016

Labels: -merge-approved-53 merge-merged-2785
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8939e8c9152b2afd54bfbd8400af6cc61706d6c1

commit 8939e8c9152b2afd54bfbd8400af6cc61706d6c1
Author: Devin Mullins <twifkak@google.com>
Date: Mon Aug 01 19:32:18 2016

Exposing CacheEntryStatus (former TransactionPattern) via UrlRequest

This will allow to know how the UrlRequest affected the cache.

Currently, it was possible to retrieve:
                           cache used = was_cached && !network_accessed
                      cache validated = was_cached && network_accessed
  not used/updated/cantconditionalize = !was_cached

This change allows to tell apart the not used/updated/cantconditionalize
sub-categories.

This change has the added benefit that HttpCacheTransaction::RecordHistograms
could be refactorized out of that class and moved to upper layers.

BUG= 615434 

Review-Url: https://codereview.chromium.org/2113603003
Cr-Commit-Position: refs/heads/master@{#407971}
(cherry picked from commit 19ff6f1bd9d1ea24e3104d0cbec6352640545d8c)

Review URL: https://codereview.chromium.org/2205453002 .

Cr-Commit-Position: refs/branch-heads/2785@{#441}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/8939e8c9152b2afd54bfbd8400af6cc61706d6c1/net/http/http_cache_transaction.cc
[modify] https://crrev.com/8939e8c9152b2afd54bfbd8400af6cc61706d6c1/net/http/http_cache_transaction.h
[modify] https://crrev.com/8939e8c9152b2afd54bfbd8400af6cc61706d6c1/net/http/http_cache_unittest.cc
[modify] https://crrev.com/8939e8c9152b2afd54bfbd8400af6cc61706d6c1/net/http/http_response_info.cc
[modify] https://crrev.com/8939e8c9152b2afd54bfbd8400af6cc61706d6c1/net/http/http_response_info.h

Comment 8 by twif...@chromium.org, Aug 1 2016 

I've discovered that https://chromium.googlesource.com/chromium/src/+/8e1bdb68cb6d13fc258a7498948a3c590929606a is a prerequisite for the the second commit (comment #3). Is approval still valid?

Comment 9 by twif...@chromium.org, Aug 1 2016

Labels: -Hotlist-Merge-Approved -merge-merged-2785 Merge-Request-53

Comment 10 by dimu@google.com, Aug 1 2016

Labels: -Merge-Request-53 Hotlist-Merge-Approved Merge-Approved-53
►

Sign in to add a comment