Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(301)

Issue 1783433003: Increase priority of XHR in ResourcePriorities experiment. (Closed)

Created:
4 years, 9 months ago by Pat Meenan
Modified:
4 years, 9 months ago
Reviewers:
Nate Chapin
CC:
chromium-reviews, Yoav Weiss, tyoshino+watch_chromium.org, loading-reviews_chromium.org, gavinp+loader_chromium.org, blink-reviews, Nate Chapin
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Increase priority of XHR in ResourcePriorities experiment. This fixes the issue where the experiment moved XHR from "not delayable" to "delayable". It was caught by the Service Workers micro-benchmark but is a general issue (don't expect much real world impact but it makes more sense to not delay XHR since it could be used for just about anything). This change matches what we already do to script, font and CSS priorities. BUG=587507 Committed: https://crrev.com/1e2eb58c114b87acb269b022a9074d7290fcdbde Cr-Commit-Position: refs/heads/master@{#380192}

Patch Set 1 #

Patch Set 2 : Added unit test #

Unified diffs Side-by-side diffs Delta from patch set Stats (+8 lines, -2 lines) Patch
M third_party/WebKit/Source/core/loader/FrameFetchContext.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/loader/FrameFetchContextTest.cpp View 1 2 chunks +6 lines, -0 lines 0 comments Download

Messages

Total messages: 10 (3 generated)
Pat Meenan
japhet@ PTAL. This fixes an issue with the resource priorities experiment specific to XHR that ...
4 years, 9 months ago (2016-03-09 17:20:35 UTC) #2
Nate Chapin
LGTM, but can you add a case to FrameFetchContextTest.ModifyPriorityForExperiments to test this?
4 years, 9 months ago (2016-03-09 17:43:47 UTC) #3
Pat Meenan
On 2016/03/09 17:43:47, Nate Chapin wrote: > LGTM, but can you add a case to ...
4 years, 9 months ago (2016-03-09 18:18:30 UTC) #4
Nate Chapin
thanks, lgtm
4 years, 9 months ago (2016-03-09 18:23:16 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1783433003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1783433003/20001
4 years, 9 months ago (2016-03-09 18:28:54 UTC) #7
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 9 months ago (2016-03-09 20:09:29 UTC) #8
commit-bot: I haz the power
4 years, 9 months ago (2016-03-09 20:10:56 UTC) #10
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/1e2eb58c114b87acb269b022a9074d7290fcdbde
Cr-Commit-Position: refs/heads/master@{#380192}

Powered by Google App Engine
This is Rietveld 408576698