Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2335913002:
Add daily quota for precache (Closed)
DescriptionAdd daily quota for precache
BUG=638420
Committed: https://crrev.com/f4f0b2a973b3940ffe34af8cd3252d07d266fd01
Cr-Commit-Position: refs/heads/master@{#420745}
Patch Set 1 #Patch Set 2 : minor fix #
Total comments: 4
Patch Set 3 : Added quota proto #
Total comments: 30
Patch Set 4 : Addressed sclittle comments #
Total comments: 8
Patch Set 5 : Addressed sclittle@ comments #
Total comments: 2
Patch Set 6 : Addressed sclittle@ comments #
Total comments: 8
Patch Set 7 : Addressed nits #Patch Set 8 : rebased #Messages
Total messages: 40 (22 generated)
rajendrant@chromium.org changed reviewers: + sclittle@chromium.org
There's a bunch of references here to a "quota.proto" file that doesn't seem to exist; it's neither in the codebase already nor added in this CL. Could you fix this, either by removing the references or adding the file? https://codereview.chromium.org/2335913002/diff/2/components/precache/core/BU... File components/precache/core/BUILD.gn (right): https://codereview.chromium.org/2335913002/diff/2/components/precache/core/BU... components/precache/core/BUILD.gn:57: "proto/quota.proto", Is this a new file? I don't see it in the code review. https://codereview.chromium.org/2335913002/diff/2/components/precache/core/pr... File components/precache/core/precache_fetcher.h (right): https://codereview.chromium.org/2335913002/diff/2/components/precache/core/pr... components/precache/core/precache_fetcher.h:22: #include "components/precache/core/proto/quota.pb.h" Where is this file? I can't find it.
Added the proto file. https://codereview.chromium.org/2335913002/diff/2/components/precache/core/BU... File components/precache/core/BUILD.gn (right): https://codereview.chromium.org/2335913002/diff/2/components/precache/core/BU... components/precache/core/BUILD.gn:57: "proto/quota.proto", On 2016/09/14 16:23:28, sclittle wrote: > Is this a new file? I don't see it in the code review. Done. https://codereview.chromium.org/2335913002/diff/2/components/precache/core/pr... File components/precache/core/precache_fetcher.h (right): https://codereview.chromium.org/2335913002/diff/2/components/precache/core/pr... components/precache/core/precache_fetcher.h:22: #include "components/precache/core/proto/quota.pb.h" On 2016/09/14 16:23:28, sclittle wrote: > Where is this file? I can't find it. Done.
https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... File components/precache/core/precache_fetcher.cc (right): https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_fetcher.cc:648: quota_ = std::move(quota); nit: There's no point in std::move-ing this, since it's a const ref, so this is basically just a copy anyways. https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_fetcher.cc:653: (base::Time::Now().LocalMidnight() + base::TimeDelta::FromDays(1)) nit: Instead of calling base::Time::Now() twice, could you just call it once, store that in a variable, and re-use it? base::Time::Now() isn't free to call, plus it could get weird if e.g. the chrome process was suspended or something, so it's technically possible for this later call to Now() to happen much later than the earlier call. https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_fetcher.cc:741: quota_.set_remaining(static_cast<size_t>(remaining)); Cast this to a uint64_t, not a size_t. size_t might only be 32 bits, e.g. on a 32 bit system. https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... File components/precache/core/precache_fetcher_unittest.cc (right): https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_fetcher_unittest.cc:24: #include "base/rand_util.h" It's generally a bad idea to use any randomness in tests, since usually you can just use a specific constant. Could you remove this dependency? https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_fetcher_unittest.cc:1305: config.set_daily_quota_total(10000000); Could you reduce this by a couple orders of magnitude? Like, maybe 10000? https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_fetcher_unittest.cc:1307: PrecacheManifest good_manifest; nit: Is there some significance that this is a "good" manifest? Or could you rename this to just "manifest"? https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_fetcher_unittest.cc:1309: std::string resource_data = base::RandBytesAsString(5000000); Don't use randomness here, could you use something like std::string(5000, 'a')? https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_fetcher_unittest.cc:1323: factory_.SetFakeResponse(GURL(kGoodResourceURL), resource_data, net::HTTP_OK, Why are you requesting the same resource multiple times, e.g. kGoodResourceURL? Should these be different URLs each time? I'm worried that if someone makes a change to e.g. eliminate duplicate fetches of the same resource while precaching, then this test will just break. https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... File components/precache/core/precache_session_table.h (right): https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_session_table.h:8: #include <list> nit: Could you remove this include? https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_session_table.h:9: #include <map> nit: Could you remove this include? https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_session_table.h:14: #include "url/gurl.h" nit: Could you remove this include? https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_session_table.h:17: class TimeTicks; nit: Could you remove this? TimeTicks is not referenced in this file. https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_session_table.h:27: class PrecacheQuota; Remove this forward include, and add an #include for the full PrecacheQuota declaration, since you're returning a full PrecacheQuota object. https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_session_table.h:30: enum SessionDataType { nit: Could you change this to be an enum class? Right now, this is polluting the precache namespace, e.g. this causes problems if any other code in the precache namespace wants to use the symbol QUOTA or UNFINISHED_WORK or anything like that. https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... File components/precache/core/proto/quota.proto (right): https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/proto/quota.proto:12: // Quota limit and expiry time. nit: Could you also mention here that this is stored in a database, and not transmitted anywhere?
ptal https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... File components/precache/core/precache_fetcher.cc (right): https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_fetcher.cc:648: quota_ = std::move(quota); On 2016/09/14 17:59:15, sclittle wrote: > nit: There's no point in std::move-ing this, since it's a const ref, so this is > basically just a copy anyways. Done. https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_fetcher.cc:653: (base::Time::Now().LocalMidnight() + base::TimeDelta::FromDays(1)) On 2016/09/14 17:59:15, sclittle wrote: > nit: Instead of calling base::Time::Now() twice, could you just call it once, > store that in a variable, and re-use it? base::Time::Now() isn't free to call, > plus it could get weird if e.g. the chrome process was suspended or something, > so it's technically possible for this later call to Now() to happen much later > than the earlier call. Done. https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_fetcher.cc:741: quota_.set_remaining(static_cast<size_t>(remaining)); On 2016/09/14 17:59:15, sclittle wrote: > Cast this to a uint64_t, not a size_t. size_t might only be 32 bits, e.g. on a > 32 bit system. Done. https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... File components/precache/core/precache_fetcher_unittest.cc (right): https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_fetcher_unittest.cc:24: #include "base/rand_util.h" On 2016/09/14 17:59:17, sclittle wrote: > It's generally a bad idea to use any randomness in tests, since usually you can > just use a specific constant. Could you remove this dependency? Done. https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_fetcher_unittest.cc:1305: config.set_daily_quota_total(10000000); On 2016/09/14 17:59:15, sclittle wrote: > Could you reduce this by a couple orders of magnitude? Like, maybe 10000? Done. https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_fetcher_unittest.cc:1307: PrecacheManifest good_manifest; On 2016/09/14 17:59:16, sclittle wrote: > nit: Is there some significance that this is a "good" manifest? Or could you > rename this to just "manifest"? Done. https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_fetcher_unittest.cc:1309: std::string resource_data = base::RandBytesAsString(5000000); On 2016/09/14 17:59:16, sclittle wrote: > Don't use randomness here, could you use something like std::string(5000, 'a')? Done. https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_fetcher_unittest.cc:1323: factory_.SetFakeResponse(GURL(kGoodResourceURL), resource_data, net::HTTP_OK, On 2016/09/14 17:59:16, sclittle wrote: > Why are you requesting the same resource multiple times, e.g. kGoodResourceURL? > > Should these be different URLs each time? I'm worried that if someone makes a > change to e.g. eliminate duplicate fetches of the same resource while > precaching, then this test will just break. Done. https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... File components/precache/core/precache_session_table.h (right): https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_session_table.h:8: #include <list> On 2016/09/14 17:59:17, sclittle wrote: > nit: Could you remove this include? Done. https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_session_table.h:9: #include <map> On 2016/09/14 17:59:18, sclittle wrote: > nit: Could you remove this include? Done. https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_session_table.h:14: #include "url/gurl.h" On 2016/09/14 17:59:18, sclittle wrote: > nit: Could you remove this include? Done. https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_session_table.h:17: class TimeTicks; On 2016/09/14 17:59:18, sclittle wrote: > nit: Could you remove this? TimeTicks is not referenced in this file. Done. https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_session_table.h:27: class PrecacheQuota; On 2016/09/14 17:59:18, sclittle wrote: > Remove this forward include, and add an #include for the full PrecacheQuota > declaration, since you're returning a full PrecacheQuota object. Done. https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/precache_session_table.h:30: enum SessionDataType { On 2016/09/14 17:59:17, sclittle wrote: > nit: Could you change this to be an enum class? Right now, this is polluting the > precache namespace, e.g. this causes problems if any other code in the precache > namespace wants to use the symbol QUOTA or UNFINISHED_WORK or anything like > that. Done. https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... File components/precache/core/proto/quota.proto (right): https://codereview.chromium.org/2335913002/diff/30001/components/precache/cor... components/precache/core/proto/quota.proto:12: // Quota limit and expiry time. On 2016/09/14 17:59:18, sclittle wrote: > nit: Could you also mention here that this is stored in a database, and not > transmitted anywhere? Done.
https://codereview.chromium.org/2335913002/diff/50001/components/precache/cor... File components/precache/core/precache_fetcher.cc (right): https://codereview.chromium.org/2335913002/diff/50001/components/precache/cor... components/precache/core/precache_fetcher.cc:520: DCHECK(base::Time::FromInternalValue(quota_.expiry_time()) > This DCHECK will not hold if the user changes their clock to be back in time. https://codereview.chromium.org/2335913002/diff/50001/components/precache/cor... components/precache/core/precache_fetcher.cc:650: if (base::Time::FromInternalValue(quota_.expiry_time()) <= time_now) { What happens if the user changes their clock? If a user sets their clock back in time, then the quota here wouldn't update. Perhaps you could store a timestamp in Quota that signifies when the quota was last reset, instead of an expiry time? Then, you could check to see if the last time the quota was reset is on a different day than the current day according to Time::Now(), and then only reset the quota if it was a different day. https://codereview.chromium.org/2335913002/diff/50001/components/precache/cor... File components/precache/core/precache_fetcher_unittest.cc (right): https://codereview.chromium.org/2335913002/diff/50001/components/precache/cor... components/precache/core/precache_fetcher_unittest.cc:1353: // Continuing with the precache when quota limit is reached ,will not fetch nit: move space after comma. https://codereview.chromium.org/2335913002/diff/50001/components/precache/cor... File components/precache/core/precache_session_table.h (right): https://codereview.chromium.org/2335913002/diff/50001/components/precache/cor... components/precache/core/precache_session_table.h:21: class PrecacheQuota; nit: remove this forward declaration, since you're already including the full class declaration.
Patchset #5 (id:70001) has been deleted
https://codereview.chromium.org/2335913002/diff/50001/components/precache/cor... File components/precache/core/precache_fetcher.cc (right): https://codereview.chromium.org/2335913002/diff/50001/components/precache/cor... components/precache/core/precache_fetcher.cc:520: DCHECK(base::Time::FromInternalValue(quota_.expiry_time()) > On 2016/09/21 20:07:24, sclittle wrote: > This DCHECK will not hold if the user changes their clock to be back in time. Done. https://codereview.chromium.org/2335913002/diff/50001/components/precache/cor... components/precache/core/precache_fetcher.cc:650: if (base::Time::FromInternalValue(quota_.expiry_time()) <= time_now) { On 2016/09/21 20:07:24, sclittle wrote: > What happens if the user changes their clock? If a user sets their clock back in > time, then the quota here wouldn't update. > > Perhaps you could store a timestamp in Quota that signifies when the quota was > last reset, instead of an expiry time? Then, you could check to see if the last > time the quota was reset is on a different day than the current day according to > Time::Now(), and then only reset the quota if it was a different day. Done. I have added start_time and expiry_hours. https://codereview.chromium.org/2335913002/diff/50001/components/precache/cor... File components/precache/core/precache_fetcher_unittest.cc (right): https://codereview.chromium.org/2335913002/diff/50001/components/precache/cor... components/precache/core/precache_fetcher_unittest.cc:1353: // Continuing with the precache when quota limit is reached ,will not fetch On 2016/09/21 20:07:24, sclittle wrote: > nit: move space after comma. Done. https://codereview.chromium.org/2335913002/diff/50001/components/precache/cor... File components/precache/core/precache_session_table.h (right): https://codereview.chromium.org/2335913002/diff/50001/components/precache/cor... components/precache/core/precache_session_table.h:21: class PrecacheQuota; On 2016/09/21 20:07:24, sclittle wrote: > nit: remove this forward declaration, since you're already including the full > class declaration. Done.
https://codereview.chromium.org/2335913002/diff/90001/components/precache/cor... File components/precache/core/precache_fetcher.cc (right): https://codereview.chromium.org/2335913002/diff/90001/components/precache/cor... components/precache/core/precache_fetcher.cc:661: quota_.set_expiry_hours(24); You might not need to store expiry_hours in the proto - it's always 24 hours, right? I think the question here is, if in the future you wanted to change the quota to e.g. expire every 12 hours, then would you want the old quota in the database to expire in 12 hours or 24 hours? If you'd need it to expire in 24 hours, then you should keep expiry_hours in the proto like you do now. If it'd be OK for the old quota to expire in 12 hours, then I'd recommend removing expiry_hours for simplicity.
https://codereview.chromium.org/2335913002/diff/90001/components/precache/cor... File components/precache/core/precache_fetcher.cc (right): https://codereview.chromium.org/2335913002/diff/90001/components/precache/cor... components/precache/core/precache_fetcher.cc:661: quota_.set_expiry_hours(24); On 2016/09/22 20:02:27, sclittle wrote: > You might not need to store expiry_hours in the proto - it's always 24 hours, > right? > > I think the question here is, if in the future you wanted to change the quota to > e.g. expire every 12 hours, then would you want the old quota in the database to > expire in 12 hours or 24 hours? > > If you'd need it to expire in 24 hours, then you should keep expiry_hours in the > proto like you do now. If it'd be OK for the old quota to expire in 12 hours, > then I'd recommend removing expiry_hours for simplicity. Done. I had the expiry time in proto, since quota means a specific start time and an expiry time. If expiry time is changed from 24 to 12 hours, the old quota can expire in either 24 or 12 hours. There is no strict requirement.
lgtm % nits https://codereview.chromium.org/2335913002/diff/110001/components/precache/co... File components/precache/core/precache_fetcher.cc (right): https://codereview.chromium.org/2335913002/diff/110001/components/precache/co... components/precache/core/precache_fetcher.cc:220: // Quota expires by the end of the day specified in start time. nit: As implemented below, I don't think this comment is exactly right - the quota expires exactly 1 day after start_time, doesn't it? Not at the end of the day? Could you either change the code or the comment? https://codereview.chromium.org/2335913002/diff/110001/components/precache/co... components/precache/core/precache_fetcher.cc:736: int64_t network_response_bytes) { nit: Could you add DCHECKs here that |response_bytes| and |network_response_bytes| are non-negative? https://codereview.chromium.org/2335913002/diff/110001/components/precache/co... components/precache/core/precache_fetcher.cc:744: static_cast<int64_t>(quota_.remaining()) - network_response_bytes; nit: Instead of doing all this casting from unsigned to signed here, which will cause bugs if quota_.remaining() is between max signed int64 and max uint64, I think it'd probably be cleaner to restructure this code here to not allow |remaining| to go negative since network_response_bytes is guaranteed to be non-negative, e.g.: uint64_t used_bytes = static_cast<uint64_t>(network_response_bytes); quota_.set_remaining(used_bytes > quota_.remaining() ? 0U : quota_.remaining() - used_bytes); Alternatively, you could just change PrecacheQuota.remaining into a signed int64. I don't think there's much point in keeping it an unsigned int64 if it doesn't support numbers larger than max signed int64. https://codereview.chromium.org/2335913002/diff/110001/components/precache/co... File components/precache/core/proto/quota.proto (right): https://codereview.chromium.org/2335913002/diff/110001/components/precache/co... components/precache/core/proto/quota.proto:19: optional uint64 remaining = 3; nit: Change the field number here from 3 to 2.
The CQ bit was checked by rajendrant@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sclittle@chromium.org Link to the patchset: https://codereview.chromium.org/2335913002/#ps150001 (title: "Change default quota to 40MB")
https://codereview.chromium.org/2335913002/diff/110001/components/precache/co... File components/precache/core/precache_fetcher.cc (right): https://codereview.chromium.org/2335913002/diff/110001/components/precache/co... components/precache/core/precache_fetcher.cc:220: // Quota expires by the end of the day specified in start time. On 2016/09/22 21:36:55, sclittle wrote: > nit: As implemented below, I don't think this comment is exactly right - the > quota expires exactly 1 day after start_time, doesn't it? Not at the end of the > day? > > Could you either change the code or the comment? Done. https://codereview.chromium.org/2335913002/diff/110001/components/precache/co... components/precache/core/precache_fetcher.cc:736: int64_t network_response_bytes) { On 2016/09/22 21:36:55, sclittle wrote: > nit: Could you add DCHECKs here that |response_bytes| and > |network_response_bytes| are non-negative? Done. https://codereview.chromium.org/2335913002/diff/110001/components/precache/co... components/precache/core/precache_fetcher.cc:744: static_cast<int64_t>(quota_.remaining()) - network_response_bytes; On 2016/09/22 21:36:55, sclittle wrote: > nit: Instead of doing all this casting from unsigned to signed here, which will > cause bugs if quota_.remaining() is between max signed int64 and max uint64, I > think it'd probably be cleaner to restructure this code here to not allow > |remaining| to go negative since network_response_bytes is guaranteed to be > non-negative, e.g.: > > uint64_t used_bytes = static_cast<uint64_t>(network_response_bytes); > quota_.set_remaining(used_bytes > quota_.remaining() ? 0U : quota_.remaining() - > used_bytes); > > Alternatively, you could just change PrecacheQuota.remaining into a signed > int64. I don't think there's much point in keeping it an unsigned int64 if it > doesn't support numbers larger than max signed int64. Done. https://codereview.chromium.org/2335913002/diff/110001/components/precache/co... File components/precache/core/proto/quota.proto (right): https://codereview.chromium.org/2335913002/diff/110001/components/precache/co... components/precache/core/proto/quota.proto:19: optional uint64 remaining = 3; On 2016/09/22 21:36:55, sclittle wrote: > nit: Change the field number here from 3 to 2. Done.
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
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-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by rajendrant@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sclittle@chromium.org Link to the patchset: https://codereview.chromium.org/2335913002/#ps170001 (title: "Rebased")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
Patchset #9 (id:170001) has been deleted
The CQ bit was checked by rajendrant@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
Patchset #9 (id:190001) has been deleted
Patchset #8 (id:150001) has been deleted
The CQ bit was checked by rajendrant@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by rajendrant@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sclittle@chromium.org Link to the patchset: https://codereview.chromium.org/2335913002/#ps210001 (title: "rebased")
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.
Committed patchset #8 (id:210001)
Message was sent while issue was closed.
Description was changed from ========== Add daily quota for precache BUG=638420 ========== to ========== Add daily quota for precache BUG=638420 Committed: https://crrev.com/f4f0b2a973b3940ffe34af8cd3252d07d266fd01 Cr-Commit-Position: refs/heads/master@{#420745} ==========
Message was sent while issue was closed.
Patchset 8 (id:??) landed as https://crrev.com/f4f0b2a973b3940ffe34af8cd3252d07d266fd01 Cr-Commit-Position: refs/heads/master@{#420745} |
