Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2017883002:
Make revalidation to fail when received a redirect response (Closed)
DescriptionMake revalidation to fail when received a redirect response
BUG=614989
Committed: https://crrev.com/a9531777113bf6e6ea527c1ec34bbb65a128b560
Cr-Commit-Position: refs/heads/master@{#397650}
Patch Set 1 #
Total comments: 4
Patch Set 2 : Added a unit test, reflected kinuko@'s comment. #
Messages
Total messages: 23 (9 generated)
Description was changed from ========== Make revalidation to fail when received a redirect response BUG=614989 ========== to ========== Make revalidation to fail when received a redirect response BUG=614989 ==========
The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2017883002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2017883002/1
hiroshige@chromium.org changed reviewers: + japhet@chromium.org, kinuko@chromium.org, yhirano@chromium.org
PTAL. See the crbug issue for details/background. I want to merge this to beta/stable.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
This looks reasonable to me too while I'm less sure... would like to try landing this and see what happens. https://codereview.chromium.org/2017883002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/fetch/Resource.cpp (right): https://codereview.chromium.org/2017883002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/fetch/Resource.cpp:557: } nit: no { } for consistency
Whatever behavior we end up with here, could we include a unit test? https://codereview.chromium.org/2017883002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/fetch/Resource.cpp (right): https://codereview.chromium.org/2017883002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/fetch/Resource.cpp:556: revalidationFailed(); Would m_redirectChain.clear() suffice?
https://codereview.chromium.org/2017883002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/fetch/Resource.cpp (right): https://codereview.chromium.org/2017883002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/fetch/Resource.cpp:556: revalidationFailed(); On 2016/05/27 17:10:43, Nate Chapin wrote: > Would m_redirectChain.clear() suffice? No. Just clearing |m_redirectChain| doesn't prevent |m_redirectChain.clear()| in responceReceived() after Resource::willFollowRedirect() is called during revalidation.
Added a unit test. https://codereview.chromium.org/2017883002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/fetch/Resource.cpp (right): https://codereview.chromium.org/2017883002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/fetch/Resource.cpp:557: } On 2016/05/27 14:25:01, kinuko wrote: > nit: no { } for consistency Done.
Ok, LGTM.
lgtm
lgtm/2
The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2017883002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2017883002/20001
The CQ bit was unchecked by hiroshige@chromium.org
The CQ bit was checked by hiroshige@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2017883002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2017883002/20001
Message was sent while issue was closed.
Description was changed from ========== Make revalidation to fail when received a redirect response BUG=614989 ========== to ========== Make revalidation to fail when received a redirect response BUG=614989 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Make revalidation to fail when received a redirect response BUG=614989 ========== to ========== Make revalidation to fail when received a redirect response BUG=614989 Committed: https://crrev.com/a9531777113bf6e6ea527c1ec34bbb65a128b560 Cr-Commit-Position: refs/heads/master@{#397650} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/a9531777113bf6e6ea527c1ec34bbb65a128b560 Cr-Commit-Position: refs/heads/master@{#397650} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
