Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2146023003:
Add UMA Precache.Freshness.Prefetch (Closed)
|
Created:
4 years, 5 months ago by jamartin Modified:
4 years, 4 months ago CC:
chromium-reviews, droger+watchlist_chromium.org, blundell+watchlist_chromium.org, sdefresne+watchlist_chromium.org, jam, darin-cc_chromium.org, asvitkine+watch_chromium.org, wifiprefetch-reviews_google.com Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd UMA Precache.Freshness.Prefetch and include validated resources.
- Precache.Freshness.Prefetch measures the freshness (in seconds) of
all the prefetched resources in seconds since the moment they were
downloaded.
- Expose HttpResponseInfo through the call chain of
UpdatePrecacheMetricsAndState
Precache.Freshness.Prefetch will be useful as a comparison to
Precache.TimeSinceLastPrecache.
BUG=619211
Committed: https://crrev.com/8e1bdb68cb6d13fc258a7498948a3c590929606a
Cr-Commit-Position: refs/heads/master@{#407974}
Patch Set 1 #Patch Set 2 : Fix precache_util.cc #Patch Set 3 : Typo #
Total comments: 9
Patch Set 4 : Added Precache.Saved.Freshness and changed the range to 5 min - 1 year #Patch Set 5 : Doc changes #
Total comments: 4
Patch Set 6 : Changed the histogram summary #
Total comments: 10
Patch Set 7 : gclient sync #Patch Set 8 : bengr's comments; Forward declare and no default args #Patch Set 9 : gclient sync #
Total comments: 6
Patch Set 10 : Added missing includes #Messages
Total messages: 42 (26 generated)
Description was changed from ========== Add UMA Precache.Freshness.Prefetch and include validated resources. - Precache.Freshness.Prefetch measures the freshness (in seconds) of all the prefetched resources in seconds since the moment they were downloaded. It is bucketed from 1 min to 36 h. - Expose HttpResponseInfo through the call chain of UpdatePrecacheMetricsAndState - Add to the savings UMAs the prefetched resources that were already in the cache due to user browsing but needed validation (and thus where validated by the Prefetch). Precache.Freshness.Prefetch will be useful as a comparison to Precache.TimeSinceLastPrecache. Adding validated resources will give us a more accurate estimation of the benefits of the precache. BUG=619211 ========== to ========== Add UMA Precache.Freshness.Prefetch and include validated resources. - Precache.Freshness.Prefetch measures the freshness (in seconds) of all the prefetched resources in seconds since the moment they were downloaded. It is bucketed from 1 min to 36 h. - Expose HttpResponseInfo through the call chain of UpdatePrecacheMetricsAndState - Add to the savings UMAs the prefetched resources that were already in the cache due to user browsing but needed validation (and thus where validated by the Prefetch). Precache.Freshness.Prefetch will be useful as a comparison to Precache.TimeSinceLastPrecache. Adding validated resources will give us a more accurate estimation of the benefits of the precache. BUG=619211 ==========
jamartin@chromium.org changed reviewers: + twifkak@chromium.org
jamartin@chromium.org changed reviewers: + bengr@chromium.org, rkaplow@chromium.org
@bengr: Please approve for precache_util.cc @rkaplow: Please review histograms.xml
jamartin@chromium.org changed reviewers: + gavinp@chromium.org
@gavinp: Please review the change in DEPS on net/http
lgtm
https://codereview.chromium.org/2146023003/diff/40001/components/precache/cor... File components/precache/core/precache_database.cc (right): https://codereview.chromium.org/2146023003/diff/40001/components/precache/cor... components/precache/core/precache_database.cc:115: UMA_HISTOGRAM_CUSTOM_COUNTS( Why not UMA_HISTOGRAM_CUSTOM_TIMES? https://codereview.chromium.org/2146023003/diff/40001/components/precache/cor... components/precache/core/precache_database.cc:120: base::TimeDelta::FromHours(36).InSeconds() /* max */, Obviously a lot of things are going to go in the overflow bucket (http://httparchive.org/interesting.php#caching). I assume that's intended. :) Come to think of it, would it be useful to have an UMA of freshness lifetimes for Precache.Saved resources? That way we could see if we're benefitting 1-year lifetime resources, or if our pipeline should have a maximum lifetime in addition to a minimum. https://codereview.chromium.org/2146023003/diff/40001/components/precache/cor... components/precache/core/precache_database.cc:124: if (info.was_cached && !info.network_accessed && I thought the plan was to add a Precache.Saved.UpperBound metric for grey area cases like this. By revalidating the cache entry, isn't it possible that all we did was save the user a few bytes of 304 traffic later? OTOH, isn't it possible that accessing a cache entry without needing revalidating would still update it's last-accessed timestamp, thus saving it from LRU eviction?
PTAL. BTW, please note that the freshness can be 0 in, for instance, the cases that always force a revalidation. However, I've just set the min value to 5 min, which is OK but it will put in the same bucket 0 and everything below 5 minutes. Do we want to report 0 in any special way? Either not report it, report it in a different UMA or change the minimum to 1 second? Another thing to note is that, for Prefetch.Saved.Freshness, we are reporting the freshness regardless if the entry was revalidated or not. In both cases the freshness value is from the moment the URL was fetched or validated, so they are comparable. But, for instance, it can happen that URL initially had a freshness of X but then, on validation, it changed its freshness to Y. Is this OK? https://codereview.chromium.org/2146023003/diff/40001/components/precache/cor... File components/precache/core/precache_database.cc (right): https://codereview.chromium.org/2146023003/diff/40001/components/precache/cor... components/precache/core/precache_database.cc:115: UMA_HISTOGRAM_CUSTOM_COUNTS( On 2016/07/18 04:30:35, twifkak wrote: > Why not UMA_HISTOGRAM_CUSTOM_TIMES? I could if you really wanted. However, UMA_HISTOGRAM_CUSTOM_TIMES reports in milliseconds and, given our range and desired resolution, I think it is misleading to report in milliseconds. Besides, it's going to be a bit confusing comparing the max-age of the headers (in seconds) against numbers in milliseconds. As I learned from rkaplow@, there is no unit conversion. UMA will report whatever number we provide as-is. https://codereview.chromium.org/2146023003/diff/40001/components/precache/cor... components/precache/core/precache_database.cc:120: base::TimeDelta::FromHours(36).InSeconds() /* max */, On 2016/07/18 04:30:35, twifkak wrote: > Obviously a lot of things are going to go in the overflow bucket > (http://httparchive.org/interesting.php#caching). I assume that's intended. :) > > Come to think of it, would it be useful to have an UMA of freshness lifetimes > for Precache.Saved resources? That way we could see if we're benefitting 1-year > lifetime resources, or if our pipeline should have a maximum lifetime in > addition to a minimum. Well, my thought was to mimic the other counter (...TimeSinceLastPrecache) since both are focused on the range between prefetches. The kind of question I had in mind was, how frequently do the client need to do the precaching in order to benefit from X% of the resources? However, you rightly pointed out that the 95% range is about a year. What about the following: min: 5 min max: 1 year With that, there are 49 points (49.5 ranges) in the first 24h and 78 in the first 30 days, which should be enough to answer my original question and it will be more representative of the entire range. Don't you think? BTW, thanks a lot for the link. I think you shared one to the same domain before but I forgot, and it is really relevant. I've been thinking on how to compute these stats internally, but since there seem that the HTTP headers are only in doc.content.representation, then I'd need to write a small MR, since the Dremel table does not contain that heavy field. https://codereview.chromium.org/2146023003/diff/40001/components/precache/cor... components/precache/core/precache_database.cc:124: if (info.was_cached && !info.network_accessed && On 2016/07/18 04:30:35, twifkak wrote: > I thought the plan was to add a Precache.Saved.UpperBound metric for grey area > cases like this. By revalidating the cache entry, isn't it possible that all we > did was save the user a few bytes of 304 traffic later? OTOH, isn't it possible > that accessing a cache entry without needing revalidating would still update > it's last-accessed timestamp, thus saving it from LRU eviction? You are right. I don't understand this code as well as I thought I did. I don't know why I thought that we were not considering the latency improvement for those URLs that were not added to the table but that's not true. We measure latency regardless of the URL table and we compare just by membership to the experiment group. The URL table is, currently, only for measuring the byte savings, and 304s don't amount for much, so I'm dropping this part of the CL. Regarding the upper bound, I'll coordinate with Raj before doing it. I'll need new fields in the DB which will conflict with his intended changes for the bitvector, so it is better if we take turns at it.
@rkaplow: Please note I've added another histogram since the time you LGTM'ed.
On 2016/07/18 16:29:43, jamartin wrote: > BTW, please note that the freshness can be 0 in, for instance, the cases that > always force a revalidation. However, I've just set the min value to 5 min, > which is OK but it will put in the same bucket 0 and everything below 5 minutes. > > Do we want to report 0 in any special way? Either not report it, report it in a > different UMA or change the minimum to 1 second? No, I think treating uncacheable as equivalent to cacheable < 5 mins is fine for our usage here. > Another thing to note is that, for Prefetch.Saved.Freshness, we are reporting > the freshness regardless if the entry was revalidated or not. In both cases the > freshness value is from the moment the URL was fetched or validated, so they are > comparable. But, for instance, it can happen that URL initially had a freshness > of X but then, on validation, it changed its freshness to Y. > > Is this OK? Gosh, I hope so. I expect there aren't a lot of URLs that report changing lifetimes, since (IIRC) a vast majority of lifetimes are 1 hour, 1 day, 7 days, 14 days, 30 days, 180 days, or 1 year.
lgtm https://codereview.chromium.org/2146023003/diff/40001/components/precache/cor... File components/precache/core/precache_database.cc (right): https://codereview.chromium.org/2146023003/diff/40001/components/precache/cor... components/precache/core/precache_database.cc:115: UMA_HISTOGRAM_CUSTOM_COUNTS( On 2016/07/18 16:29:43, jamartin wrote: > On 2016/07/18 04:30:35, twifkak wrote: > > Why not UMA_HISTOGRAM_CUSTOM_TIMES? > > I could if you really wanted. However, UMA_HISTOGRAM_CUSTOM_TIMES reports in > milliseconds and, given our range and desired resolution, I think it is > misleading to report in milliseconds. Besides, it's going to be a bit confusing > comparing the max-age of the headers (in seconds) against numbers in > milliseconds. > > As I learned from rkaplow@, there is no unit conversion. UMA will report > whatever number we provide as-is. Nah, that's a good enough reason for me. https://codereview.chromium.org/2146023003/diff/40001/components/precache/cor... components/precache/core/precache_database.cc:120: base::TimeDelta::FromHours(36).InSeconds() /* max */, On 2016/07/18 16:29:43, jamartin wrote: > Well, my thought was to mimic the other counter (...TimeSinceLastPrecache) since > both are focused on the range between prefetches. > > The kind of question I had in mind was, how frequently do the client need to do > the precaching in order to benefit from X% of the resources? Yeah, that sounds pretty reasonable. > However, you rightly pointed out that the 95% range is about a year. What about > the following: > min: 5 min > max: 1 year > > With that, there are 49 points (49.5 ranges) in the first 24h and 78 in the > first 30 days, which should be enough to answer my original question and it will > be more representative of the entire range. Don't you think? That sounds fine, too. :P https://codereview.chromium.org/2146023003/diff/40001/components/precache/cor... components/precache/core/precache_database.cc:124: if (info.was_cached && !info.network_accessed && On 2016/07/18 16:29:43, jamartin wrote: > Regarding the upper bound, I'll coordinate with Raj before doing it. I'll need > new fields in the DB which will conflict with his intended changes for the > bitvector, so it is better if we take turns at it. Oy, that sounds fun. https://codereview.chromium.org/2146023003/diff/80001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2146023003/diff/80001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:42711: + (and thus served from the cache) as described in RFC 2616 13.2.4. This is nit: put "as described in RFC blah" in parens after "freshness lifetimes"; it's a bit of a dangling modifier in its current position. https://codereview.chromium.org/2146023003/diff/80001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:42714: + fetching (or revalidation) time. Logged per-request but only once per URL What do you mean by "counting from the fetching time"? AFAIK RecordURLNonPrefetch is called for every user-initiated URL fetch (i.e. during interactive browsing).
The CQ bit was checked by jamartin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
@rkaplow: I've added a histogram and changed their summary, in case you want to take another look. @bengr and @gavinp: I need your ownership approval. https://codereview.chromium.org/2146023003/diff/80001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2146023003/diff/80001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:42711: + (and thus served from the cache) as described in RFC 2616 13.2.4. This is On 2016/07/18 23:50:44, twifkak wrote: > nit: put "as described in RFC blah" in parens after "freshness lifetimes"; it's > a bit of a dangling modifier in its current position. Done. https://codereview.chromium.org/2146023003/diff/80001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:42714: + fetching (or revalidation) time. Logged per-request but only once per URL On 2016/07/18 23:50:44, twifkak wrote: > What do you mean by "counting from the fetching time"? AFAIK > RecordURLNonPrefetch is called for every user-initiated URL fetch (i.e. during > interactive browsing). I meant "fetching time" as the time when the resource was actually downloaded or validated over the network. That means either the original precaching time or the current request time, if the resource has been validated. Anyway, all of this is part of the RFC spec, so I'm removing this lousy explanation :-P
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2146023003/diff/100001/chrome/browser/precach... File chrome/browser/precache/precache_util.cc (right): https://codereview.chromium.org/2146023003/diff/100001/chrome/browser/precach... chrome/browser/precache/precache_util.cc:15: #include "net/http/http_response_info.h" You should be able to forward declare HttpResponseInfo instead. https://codereview.chromium.org/2146023003/diff/100001/components/precache/co... File components/precache/content/precache_manager.cc (right): https://codereview.chromium.org/2146023003/diff/100001/components/precache/co... components/precache/content/precache_manager.cc:29: #include "net/http/http_response_info.h" Remove this include. You will forward declare in the .h https://codereview.chromium.org/2146023003/diff/100001/components/precache/co... File components/precache/content/precache_manager.h (right): https://codereview.chromium.org/2146023003/diff/100001/components/precache/co... components/precache/content/precache_manager.h:24: #include "net/http/http_response_info.h" Forward declare instead. https://codereview.chromium.org/2146023003/diff/100001/components/precache/co... File components/precache/content/precache_manager_unittest.cc (right): https://codereview.chromium.org/2146023003/diff/100001/components/precache/co... components/precache/content/precache_manager_unittest.cc:526: GURL(), base::TimeDelta(), #include "base/time/time.h" https://codereview.chromium.org/2146023003/diff/100001/components/precache/co... File components/precache/core/precache_database.h (right): https://codereview.chromium.org/2146023003/diff/100001/components/precache/co... components/precache/core/precache_database.h:24: #include "net/http/http_response_info.h" Forward declare instead. https://codereview.chromium.org/2146023003/diff/100001/components/precache/co... File components/precache/core/precache_database_unittest.cc (right): https://codereview.chromium.org/2146023003/diff/100001/components/precache/co... components/precache/core/precache_database_unittest.cc:48: HttpResponseInfo CreateHttpResponseInfo(bool was_cached = false, Don't use default values.
The CQ bit was checked by jamartin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Add UMA Precache.Freshness.Prefetch and include validated resources. - Precache.Freshness.Prefetch measures the freshness (in seconds) of all the prefetched resources in seconds since the moment they were downloaded. It is bucketed from 1 min to 36 h. - Expose HttpResponseInfo through the call chain of UpdatePrecacheMetricsAndState - Add to the savings UMAs the prefetched resources that were already in the cache due to user browsing but needed validation (and thus where validated by the Prefetch). Precache.Freshness.Prefetch will be useful as a comparison to Precache.TimeSinceLastPrecache. Adding validated resources will give us a more accurate estimation of the benefits of the precache. BUG=619211 ========== to ========== Add UMA Precache.Freshness.Prefetch and include validated resources. - Precache.Freshness.Prefetch measures the freshness (in seconds) of all the prefetched resources in seconds since the moment they were downloaded. - Expose HttpResponseInfo through the call chain of UpdatePrecacheMetricsAndState Precache.Freshness.Prefetch will be useful as a comparison to Precache.TimeSinceLastPrecache. BUG=619211 ==========
jamartin@chromium.org changed reviewers: - gavinp@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-device-gn on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) ios-simulator-gn on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)
The CQ bit was checked by jamartin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
PTAL. https://codereview.chromium.org/2146023003/diff/100001/chrome/browser/precach... File chrome/browser/precache/precache_util.cc (right): https://codereview.chromium.org/2146023003/diff/100001/chrome/browser/precach... chrome/browser/precache/precache_util.cc:15: #include "net/http/http_response_info.h" On 2016/07/21 00:03:27, bengr wrote: > You should be able to forward declare HttpResponseInfo instead. Done. https://codereview.chromium.org/2146023003/diff/100001/components/precache/co... File components/precache/content/precache_manager.cc (right): https://codereview.chromium.org/2146023003/diff/100001/components/precache/co... components/precache/content/precache_manager.cc:29: #include "net/http/http_response_info.h" On 2016/07/21 00:03:27, bengr wrote: > Remove this include. You will forward declare in the .h Done. https://codereview.chromium.org/2146023003/diff/100001/components/precache/co... File components/precache/content/precache_manager.h (right): https://codereview.chromium.org/2146023003/diff/100001/components/precache/co... components/precache/content/precache_manager.h:24: #include "net/http/http_response_info.h" On 2016/07/21 00:03:27, bengr wrote: > Forward declare instead. Done. Nice! This should get rid of the change in DEPS and thus one reviewer less :-) https://codereview.chromium.org/2146023003/diff/100001/components/precache/co... File components/precache/core/precache_database_unittest.cc (right): https://codereview.chromium.org/2146023003/diff/100001/components/precache/co... components/precache/core/precache_database_unittest.cc:48: HttpResponseInfo CreateHttpResponseInfo(bool was_cached = false, On 2016/07/21 00:03:27, bengr wrote: > Don't use default values. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm, but please address these last comments on includes and forward declarations. https://codereview.chromium.org/2146023003/diff/160001/chrome/browser/precach... File chrome/browser/precache/precache_util.h (right): https://codereview.chromium.org/2146023003/diff/160001/chrome/browser/precach... chrome/browser/precache/precache_util.h:10: class HttpResponseInfo; Forward declare this in the .cc, since it's not needed here. https://codereview.chromium.org/2146023003/diff/160001/components/precache/co... File components/precache/core/precache_database.cc (right): https://codereview.chromium.org/2146023003/diff/160001/components/precache/co... components/precache/core/precache_database.cc:138: if (info.headers) { #include "net/http/http_response_info.h" https://codereview.chromium.org/2146023003/diff/160001/components/precache/co... File components/precache/core/precache_database_unittest.cc (right): https://codereview.chromium.org/2146023003/diff/160001/components/precache/co... components/precache/core/precache_database_unittest.cc:49: HttpResponseInfo result; #include "net/http/http_response_info.h"
The CQ bit was checked by jamartin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Thanks for the review. I'll request submission once git cl try finishes. https://codereview.chromium.org/2146023003/diff/160001/chrome/browser/precach... File chrome/browser/precache/precache_util.h (right): https://codereview.chromium.org/2146023003/diff/160001/chrome/browser/precach... chrome/browser/precache/precache_util.h:10: class HttpResponseInfo; On 2016/07/26 17:29:10, bengr wrote: > Forward declare this in the .cc, since it's not needed here. Done. https://codereview.chromium.org/2146023003/diff/160001/components/precache/co... File components/precache/core/precache_database.cc (right): https://codereview.chromium.org/2146023003/diff/160001/components/precache/co... components/precache/core/precache_database.cc:138: if (info.headers) { On 2016/07/26 17:29:10, bengr wrote: > #include "net/http/http_response_info.h" Done. https://codereview.chromium.org/2146023003/diff/160001/components/precache/co... File components/precache/core/precache_database_unittest.cc (right): https://codereview.chromium.org/2146023003/diff/160001/components/precache/co... components/precache/core/precache_database_unittest.cc:49: HttpResponseInfo result; On 2016/07/26 17:29:10, bengr wrote: > #include "net/http/http_response_info.h" Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by jamartin@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rkaplow@chromium.org, twifkak@chromium.org, bengr@chromium.org Link to the patchset: https://codereview.chromium.org/2146023003/#ps180001 (title: "Added missing includes")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Add UMA Precache.Freshness.Prefetch and include validated resources. - Precache.Freshness.Prefetch measures the freshness (in seconds) of all the prefetched resources in seconds since the moment they were downloaded. - Expose HttpResponseInfo through the call chain of UpdatePrecacheMetricsAndState Precache.Freshness.Prefetch will be useful as a comparison to Precache.TimeSinceLastPrecache. BUG=619211 ========== to ========== Add UMA Precache.Freshness.Prefetch and include validated resources. - Precache.Freshness.Prefetch measures the freshness (in seconds) of all the prefetched resources in seconds since the moment they were downloaded. - Expose HttpResponseInfo through the call chain of UpdatePrecacheMetricsAndState Precache.Freshness.Prefetch will be useful as a comparison to Precache.TimeSinceLastPrecache. BUG=619211 ==========
Message was sent while issue was closed.
Committed patchset #10 (id:180001)
Message was sent while issue was closed.
Description was changed from ========== Add UMA Precache.Freshness.Prefetch and include validated resources. - Precache.Freshness.Prefetch measures the freshness (in seconds) of all the prefetched resources in seconds since the moment they were downloaded. - Expose HttpResponseInfo through the call chain of UpdatePrecacheMetricsAndState Precache.Freshness.Prefetch will be useful as a comparison to Precache.TimeSinceLastPrecache. BUG=619211 ========== to ========== Add UMA Precache.Freshness.Prefetch and include validated resources. - Precache.Freshness.Prefetch measures the freshness (in seconds) of all the prefetched resources in seconds since the moment they were downloaded. - Expose HttpResponseInfo through the call chain of UpdatePrecacheMetricsAndState Precache.Freshness.Prefetch will be useful as a comparison to Precache.TimeSinceLastPrecache. BUG=619211 Committed: https://crrev.com/8e1bdb68cb6d13fc258a7498948a3c590929606a Cr-Commit-Position: refs/heads/master@{#407974} ==========
Message was sent while issue was closed.
Patchset 10 (id:??) landed as https://crrev.com/8e1bdb68cb6d13fc258a7498948a3c590929606a Cr-Commit-Position: refs/heads/master@{#407974} |
