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

Issue 1757633005: Don't duplicate ResourceRequests and ResourceLoaderOptions on ResourceLoader (Closed)

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

Description

Don't duplicate ResourceRequests and ResourceLoaderOptions on ResourceLoader Resource already has copies. BUG= Committed: https://crrev.com/1bef485187d2a5eff4fd471cbb0ec97e61759811 Cr-Commit-Position: refs/heads/master@{#380466}

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Patch Set 4 : #

Patch Set 5 : #

Patch Set 6 : #

Patch Set 7 : #

Patch Set 8 : #

Total comments: 15

Patch Set 9 : #

Patch Set 10 : #

Total comments: 3

Patch Set 11 : Rebase #

Patch Set 12 : Make synchronousPolicy check higher priority in determineRevalidationPolicy #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+103 lines, -187 lines) Patch
M third_party/WebKit/LayoutTests/http/tests/loading/307-after-303-after-post-expected.txt View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/http/tests/loading/redirect-methods-expected.txt View 1 2 3 4 chunks +4 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/fetch/RawResource.h View 1 2 3 2 chunks +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/fetch/RawResource.cpp View 1 2 3 1 chunk +0 lines, -8 lines 0 comments Download
M third_party/WebKit/Source/core/fetch/Resource.h View 1 2 3 4 5 6 7 8 9 2 chunks +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/fetch/Resource.cpp View 1 2 3 4 5 6 7 8 9 10 3 chunks +14 lines, -10 lines 0 comments Download
M third_party/WebKit/Source/core/fetch/ResourceFetcher.h View 1 2 3 4 5 6 7 8 9 1 chunk +2 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp View 1 2 3 4 5 6 7 8 9 10 11 8 chunks +39 lines, -40 lines 1 comment Download
M third_party/WebKit/Source/core/fetch/ResourceLoader.h View 1 2 3 4 3 chunks +4 lines, -11 lines 0 comments Download
M third_party/WebKit/Source/core/fetch/ResourceLoader.cpp View 1 2 3 4 5 6 7 8 9 11 chunks +31 lines, -83 lines 0 comments Download
M third_party/WebKit/Source/core/loader/DocumentLoader.h View 1 2 3 4 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/loader/DocumentLoader.cpp View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +7 lines, -21 lines 0 comments Download

Messages

Total messages: 35 (15 generated)
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1757633005/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1757633005/40001
4 years, 9 months ago (2016-03-03 23:10:08 UTC) #2
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_ozone_rel_ng/builds/134649)
4 years, 9 months ago (2016-03-03 23:54:08 UTC) #4
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1757633005/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1757633005/120001
4 years, 9 months ago (2016-03-08 18:00:11 UTC) #6
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 9 months ago (2016-03-08 21:40:40 UTC) #8
Nate Chapin
One more ResourceLoader refactoring, PTAL! https://codereview.chromium.org/1757633005/diff/140001/third_party/WebKit/Source/core/fetch/Resource.cpp File third_party/WebKit/Source/core/fetch/Resource.cpp (left): https://codereview.chromium.org/1757633005/diff/140001/third_party/WebKit/Source/core/fetch/Resource.cpp#oldcode223 third_party/WebKit/Source/core/fetch/Resource.cpp:223: m_loader->changeToSynchronous(); I don't remember ...
4 years, 9 months ago (2016-03-09 00:38:20 UTC) #10
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1757633005/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1757633005/140001
4 years, 9 months ago (2016-03-09 00:43:39 UTC) #12
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1757633005/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1757633005/160001
4 years, 9 months ago (2016-03-09 00:53:07 UTC) #14
hiroshige
https://codereview.chromium.org/1757633005/diff/140001/third_party/WebKit/Source/core/fetch/Resource.cpp File third_party/WebKit/Source/core/fetch/Resource.cpp (right): https://codereview.chromium.org/1757633005/diff/140001/third_party/WebKit/Source/core/fetch/Resource.cpp#newcode210 third_party/WebKit/Source/core/fetch/Resource.cpp:210: (m_redirectChain.size() ? m_redirectChain.last().m_request : m_resourceRequest) : this can be ...
4 years, 9 months ago (2016-03-09 02:01:56 UTC) #15
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 9 months ago (2016-03-09 02:19:27 UTC) #17
Nate Chapin
https://codereview.chromium.org/1757633005/diff/140001/third_party/WebKit/Source/core/fetch/Resource.cpp File third_party/WebKit/Source/core/fetch/Resource.cpp (right): https://codereview.chromium.org/1757633005/diff/140001/third_party/WebKit/Source/core/fetch/Resource.cpp#newcode210 third_party/WebKit/Source/core/fetch/Resource.cpp:210: (m_redirectChain.size() ? m_redirectChain.last().m_request : m_resourceRequest) : On 2016/03/09 02:01:55, ...
4 years, 9 months ago (2016-03-09 22:35:55 UTC) #18
yhirano
lgtm It feels a bit strange to me that ResourceLoader doesn't have ResourceLoaderOptions but Resource ...
4 years, 9 months ago (2016-03-09 23:59:00 UTC) #19
Nate Chapin
On 2016/03/09 23:59:00, yhirano wrote: > lgtm > > It feels a bit strange to ...
4 years, 9 months ago (2016-03-10 00:03:55 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1757633005/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1757633005/200001
4 years, 9 months ago (2016-03-10 00:06:04 UTC) #22
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/186734)
4 years, 9 months ago (2016-03-10 00:20:25 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1757633005/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1757633005/200001
4 years, 9 months ago (2016-03-10 00:21:43 UTC) #26
hiroshige
https://codereview.chromium.org/1757633005/diff/180001/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (right): https://codereview.chromium.org/1757633005/diff/180001/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp#newcode771 third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:771: if (fetchRequest.options().synchronousPolicy == RequestSynchronously && existingResource->isLoading()) This doesn't Reload ...
4 years, 9 months ago (2016-03-10 00:24:39 UTC) #27
Nate Chapin
https://codereview.chromium.org/1757633005/diff/180001/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (right): https://codereview.chromium.org/1757633005/diff/180001/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp#newcode771 third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:771: if (fetchRequest.options().synchronousPolicy == RequestSynchronously && existingResource->isLoading()) On 2016/03/10 00:24:38, ...
4 years, 9 months ago (2016-03-10 18:46:01 UTC) #29
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1757633005/220001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1757633005/220001
4 years, 9 months ago (2016-03-10 18:46:54 UTC) #32
commit-bot: I haz the power
Committed patchset #12 (id:220001)
4 years, 9 months ago (2016-03-10 21:20:23 UTC) #33
commit-bot: I haz the power
4 years, 9 months ago (2016-03-10 21:21:14 UTC) #35
Message was sent while issue was closed.
Patchset 12 (id:??) landed as
https://crrev.com/1bef485187d2a5eff4fd471cbb0ec97e61759811
Cr-Commit-Position: refs/heads/master@{#380466}

Powered by Google App Engine
This is Rietveld 408576698