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

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

Created:
1 year ago by alexilin
Modified:
12 months 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 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 ...
12 months 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 ...
12 months 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 ...
12 months 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 ...
12 months 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 ...
12 months 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: > ...
12 months ago (2017-04-25 17:37:05 UTC) #11
alexilin
Friendly ping.
12 months 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: > ...
12 months 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: > ...
12 months 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
12 months 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)
12 months 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
12 months ago (2017-04-27 17:12:51 UTC) #20
commit-bot: I haz the power
12 months 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