Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2181093004:
FrameFetchContext: add clear comments how FrameLoadTypeReloadMainResource works (Closed)
|
Created:
4 years, 4 months ago by Takashi Toyoshima Modified:
4 years, 4 months ago CC:
blink-reviews, chromium-reviews, gavinp+loader_chromium.org, Nate Chapin, loading-reviews_chromium.org, tyoshino+watch_chromium.org, Yoav Weiss Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFrameFetchContext: add clear comments how FrameLoadTypeReloadMainResource works
This patch just adds some comments that are asked in another codereview.
Since these comments are not related to the orignal patch directly,
let me handle this separately.
TEST=n/a
BUG=332602
Committed: https://crrev.com/476d90d93453f586a13e7bf83e596663a708a7b9
Cr-Commit-Position: refs/heads/master@{#409454}
Patch Set 1 #
Total comments: 2
Patch Set 2 : . #Messages
Total messages: 23 (12 generated)
Description was changed from ========== FrameLoader: add clear comments how FrameLoadTypeReloadMainResource works This patch just adds some comments that are asked in another codereview. Since this comments are not related to the orignal patch directly, let me handle this separately. TEST=n/a BUG=n/a NOTRY=true ========== to ========== FrameLoader: add clear comments how FrameLoadTypeReloadMainResource works This patch just adds some comments that are asked in another codereview. Since this comments are not related to the orignal patch directly, let me handle this separately. TEST=n/a BUG=n/a NOTRY=true ==========
toyoshim@chromium.org changed reviewers: + hiroshige@chromium.org
Hiroshige-san, I split this from https://codereview.chromium.org/2167623002/ What do you think?
Description was changed from ========== FrameLoader: add clear comments how FrameLoadTypeReloadMainResource works This patch just adds some comments that are asked in another codereview. Since this comments are not related to the orignal patch directly, let me handle this separately. TEST=n/a BUG=n/a NOTRY=true ========== to ========== FrameFetchContext: add clear comments how FrameLoadTypeReloadMainResource works This patch just adds some comments that are asked in another codereview. Since this comments are not related to the orignal patch directly, let me handle this separately. TEST=n/a BUG=n/a NOTRY=true ==========
Description was changed from ========== FrameFetchContext: add clear comments how FrameLoadTypeReloadMainResource works This patch just adds some comments that are asked in another codereview. Since this comments are not related to the orignal patch directly, let me handle this separately. TEST=n/a BUG=n/a NOTRY=true ========== to ========== FrameFetchContext: add clear comments how FrameLoadTypeReloadMainResource works This patch just adds some comments that are asked in another codereview. Since this comments are not related to the orignal patch directly, let me handle this separately. TEST=n/a BUG=332602 NOTRY=true ==========
https://codereview.chromium.org/2181093004/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/loader/FrameFetchContext.cpp (right): https://codereview.chromium.org/2181093004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/loader/FrameFetchContext.cpp:265: // FrameLoadTypeStandard and FrameLoadTypeReloadMainResource. > FrameLoadTypeReloadMainResource Is it referring to |frameLoadType| assigned at Line 256, not that assigned at Line 245? (For FrameLoadTypeReloadMainResource assigned at Line 245, ValidatingCacheData is returned at Line 251, not at Line 267.)
Description was changed from ========== FrameFetchContext: add clear comments how FrameLoadTypeReloadMainResource works This patch just adds some comments that are asked in another codereview. Since this comments are not related to the orignal patch directly, let me handle this separately. TEST=n/a BUG=332602 NOTRY=true ========== to ========== FrameFetchContext: add clear comments how FrameLoadTypeReloadMainResource works This patch just adds some comments that are asked in another codereview. Since these comments are not related to the orignal patch directly, let me handle this separately. TEST=n/a BUG=332602 NOTRY=true ==========
Description was changed from ========== FrameFetchContext: add clear comments how FrameLoadTypeReloadMainResource works This patch just adds some comments that are asked in another codereview. Since these comments are not related to the orignal patch directly, let me handle this separately. TEST=n/a BUG=332602 NOTRY=true ========== to ========== FrameFetchContext: add clear comments how FrameLoadTypeReloadMainResource works This patch just adds some comments that are asked in another codereview. Since these comments are not related to the orignal patch directly, let me handle this separately. TEST=n/a BUG=332602 ==========
https://codereview.chromium.org/2181093004/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/loader/FrameFetchContext.cpp (right): https://codereview.chromium.org/2181093004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/loader/FrameFetchContext.cpp:265: // FrameLoadTypeStandard and FrameLoadTypeReloadMainResource. Ah, right. This is confusing. Can I stop reusing the same variable for parent frame's load type? That makes this comment understandable too.
lgtm.
The CQ bit was checked by toyoshim@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
toyoshim@chromium.org changed reviewers: + tkent@chromium.org
tkent@ for an owner's approval.
lgtm
The CQ bit was checked by toyoshim@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== FrameFetchContext: add clear comments how FrameLoadTypeReloadMainResource works This patch just adds some comments that are asked in another codereview. Since these comments are not related to the orignal patch directly, let me handle this separately. TEST=n/a BUG=332602 ========== to ========== FrameFetchContext: add clear comments how FrameLoadTypeReloadMainResource works This patch just adds some comments that are asked in another codereview. Since these comments are not related to the orignal patch directly, let me handle this separately. TEST=n/a BUG=332602 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== FrameFetchContext: add clear comments how FrameLoadTypeReloadMainResource works This patch just adds some comments that are asked in another codereview. Since these comments are not related to the orignal patch directly, let me handle this separately. TEST=n/a BUG=332602 ========== to ========== FrameFetchContext: add clear comments how FrameLoadTypeReloadMainResource works This patch just adds some comments that are asked in another codereview. Since these comments are not related to the orignal patch directly, let me handle this separately. TEST=n/a BUG=332602 Committed: https://crrev.com/476d90d93453f586a13e7bf83e596663a708a7b9 Cr-Commit-Position: refs/heads/master@{#409454} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/476d90d93453f586a13e7bf83e596663a708a7b9 Cr-Commit-Position: refs/heads/master@{#409454} |
