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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 months ago by alexilin
Modified:
2 months, 3 weeks 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
Unified diffs Side-by-side diffs Delta from patch set Stats (+215 lines, -99 lines) Patch
M chrome/browser/predictors/resource_prefetch_predictor.cc View 3 chunks +21 lines, -5 lines 0 comments Download
M chrome/browser/predictors/resource_prefetch_predictor.proto View 1 chunk +9 lines, -9 lines 0 comments Download
M chrome/browser/predictors/resource_prefetch_predictor_tables.h View 4 chunks +6 lines, -5 lines 0 comments Download
M chrome/browser/predictors/resource_prefetch_predictor_tables.cc View 1 6 chunks +49 lines, -21 lines 3 comments Download
M chrome/browser/predictors/resource_prefetch_predictor_tables_unittest.cc View 1 4 chunks +43 lines, -14 lines 0 comments Download
M chrome/browser/predictors/resource_prefetch_predictor_test_util.h View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/predictors/resource_prefetch_predictor_test_util.cc View 1 chunk +3 lines, -1 line 0 comments Download
M chrome/browser/predictors/resource_prefetch_predictor_unittest.cc View 1 12 chunks +68 lines, -43 lines 0 comments Download
M components/precache/core/proto/precache.proto View 1 chunk +14 lines, -0 lines 2 comments Download
Commit queue not available (can’t edit this change).

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) { ...
3 months 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 ...
2 months, 3 weeks 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 ...
2 months, 3 weeks 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 ...
2 months, 3 weeks 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 ...
2 months, 3 weeks 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 ...
2 months, 3 weeks 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: > ...
2 months, 3 weeks ago (2017-04-25 17:37:05 UTC) #11
alexilin
Friendly ping.
2 months, 3 weeks 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: > ...
2 months, 3 weeks 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: > ...
2 months, 3 weeks 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
2 months, 3 weeks 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)
2 months, 3 weeks 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
2 months, 3 weeks ago (2017-04-27 17:12:51 UTC) #20
commit-bot: I haz the power
2 months, 3 weeks 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...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 25c286973