Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(1)

Issue 2831233004: predictors: Add resource type to manifest. (Closed)

Created:
1 year, 1 month ago by alexilin
Modified:
1 year ago
Reviewers:
Benoit L
CC:
chromium-reviews, wifiprefetch-reviews_google.com
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

predictors: Add resource type to manifest. This CL adds a new resource type field to the manifest proto. ResourcePrefetchPredictor begins to use this field to prefetch all resources of one resource type before another. Also we prioritize stylesheets above scripts since this CL. Stylesheets are almost always critical, scripts are not. It's clear win in the case if we're fetching resources from manifests because we don't have information about request priority in manifests. Thus we can't separate critical scripts from non-critical ones. BUG=699115 Review-Url: https://codereview.chromium.org/2831233004 Cr-Commit-Position: refs/heads/master@{#467742} Committed: https://chromium.googlesource.com/chromium/src/+/62ffefbb0a2dcd9d0ff0a1a12288577c766af408

Patch Set 1 #

Total comments: 10

Patch Set 2 : Use the same resource type ordering. #

Total comments: 5

Messages

Total messages: 23 (9 generated)
alexilin
Hi Benoit! PTAL. https://codereview.chromium.org/2831233004/diff/1/chrome/browser/predictors/resource_prefetch_predictor_tables.cc File chrome/browser/predictors/resource_prefetch_predictor_tables.cc (left): https://codereview.chromium.org/2831233004/diff/1/chrome/browser/predictors/resource_prefetch_predictor_tables.cc#oldcode318 chrome/browser/predictors/resource_prefetch_predictor_tables.cc:318: for (auto& kv : *data_map) { ...
1 year, 1 month ago (2017-04-21 12:07:28 UTC) #2
alexilin
ping lizeb.com PING lizeb.com (184.168.221.51) 56(84) bytes of data. 64 bytes from ip-184-168-221-51.ip.secureserver.net (184.168.221.51): icmp_seq=1 ...
1 year ago (2017-04-25 08:39:53 UTC) #3
Benoit L
On 2017/04/25 08:39:53, alexilin wrote: > ping http://lizeb.com > > PING http://lizeb.com (184.168.221.51) 56(84) bytes ...
1 year ago (2017-04-25 08:43:16 UTC) #4
Benoit L
https://codereview.chromium.org/2831233004/diff/1/chrome/browser/predictors/resource_prefetch_predictor.cc File chrome/browser/predictors/resource_prefetch_predictor.cc (right): https://codereview.chromium.org/2831233004/diff/1/chrome/browser/predictors/resource_prefetch_predictor.cc#newcode1001 chrome/browser/predictors/resource_prefetch_predictor.cc:1001: return ResourcePrefetchPredictorTables::ComputePrecacheResourceScore( nit: use a local "using" to make ...
1 year ago (2017-04-25 08:59:21 UTC) #5
alexilin
A couple of questions before I send the next patch. https://codereview.chromium.org/2831233004/diff/1/chrome/browser/predictors/resource_prefetch_predictor.cc File chrome/browser/predictors/resource_prefetch_predictor.cc (right): https://codereview.chromium.org/2831233004/diff/1/chrome/browser/predictors/resource_prefetch_predictor.cc#newcode1001 ...
1 year ago (2017-04-25 13:04:26 UTC) #6
Benoit L
https://codereview.chromium.org/2831233004/diff/1/chrome/browser/predictors/resource_prefetch_predictor.cc File chrome/browser/predictors/resource_prefetch_predictor.cc (right): https://codereview.chromium.org/2831233004/diff/1/chrome/browser/predictors/resource_prefetch_predictor.cc#newcode1001 chrome/browser/predictors/resource_prefetch_predictor.cc:1001: return ResourcePrefetchPredictorTables::ComputePrecacheResourceScore( On 2017/04/25 13:04:25, alexilin wrote: > On ...
1 year ago (2017-04-25 15:12:39 UTC) #7
alexilin
https://codereview.chromium.org/2831233004/diff/1/chrome/browser/predictors/resource_prefetch_predictor.cc File chrome/browser/predictors/resource_prefetch_predictor.cc (right): https://codereview.chromium.org/2831233004/diff/1/chrome/browser/predictors/resource_prefetch_predictor.cc#newcode1001 chrome/browser/predictors/resource_prefetch_predictor.cc:1001: return ResourcePrefetchPredictorTables::ComputePrecacheResourceScore( On 2017/04/25 15:12:39, Benoit L wrote: > ...
1 year ago (2017-04-25 17:37:05 UTC) #11
alexilin
Friendly ping.
1 year ago (2017-04-26 16:29:36 UTC) #12
Benoit L
lgtm https://codereview.chromium.org/2831233004/diff/20001/chrome/browser/predictors/resource_prefetch_predictor_tables.cc File chrome/browser/predictors/resource_prefetch_predictor_tables.cc (right): https://codereview.chromium.org/2831233004/diff/20001/chrome/browser/predictors/resource_prefetch_predictor_tables.cc#newcode99 chrome/browser/predictors/resource_prefetch_predictor_tables.cc:99: case predictors::ResourceData::RESOURCE_TYPE_FONT_RESOURCE: On 2017/04/25 17:37:05, alexilin wrote: > ...
1 year ago (2017-04-27 08:54:15 UTC) #13
alexilin
https://codereview.chromium.org/2831233004/diff/20001/chrome/browser/predictors/resource_prefetch_predictor_tables.cc File chrome/browser/predictors/resource_prefetch_predictor_tables.cc (right): https://codereview.chromium.org/2831233004/diff/20001/chrome/browser/predictors/resource_prefetch_predictor_tables.cc#newcode99 chrome/browser/predictors/resource_prefetch_predictor_tables.cc:99: case predictors::ResourceData::RESOURCE_TYPE_FONT_RESOURCE: On 2017/04/27 08:54:15, Benoit L wrote: > ...
1 year ago (2017-04-27 15:32:24 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2831233004/20001
1 year ago (2017-04-27 15:33:21 UTC) #16
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_ozone_rel_ng/builds/371847)
1 year ago (2017-04-27 17:09:14 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2831233004/20001
1 year ago (2017-04-27 17:12:51 UTC) #20
commit-bot: I haz the power
1 year ago (2017-04-27 19:08:58 UTC) #23
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/62ffefbb0a2dcd9d0ff0a1a12288...

Powered by Google App Engine
This is Rietveld 408576698