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

Issue 2842253002: Move ReportLocalLoadFailed to ExecutionContext (Closed)

Created:
3 years, 8 months ago by Nate Chapin
Modified:
3 years, 7 months ago
CC:
blink-reviews, blink-reviews-dom_chromium.org, blink-reviews-html_chromium.org, chromium-reviews, dglazkov+blink, dom-dev_chromium.org, eae+blinkwatch, eric.carlson_apple.com, feature-media-reviews_chromium.org, fs, gasubic, mlamouri+watch-blink_chromium.org, rwlbuis, sof, nessy, Srirama, Yoav Weiss
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Move ReportLocalLoadFailed to ExecutionContext It's static in FrameLoader, and takes a LocalFrame* parameter, but what it really cares about is calling ExecutionContext::AddMessageToConsole. Just put it on ExecutionContext and make it non-static. BUG= Review-Url: https://codereview.chromium.org/2842253002 Cr-Commit-Position: refs/heads/master@{#474108} Committed: https://chromium.googlesource.com/chromium/src/+/fd24fc9f7417c5e9a580911faa80a7c75938c004

Patch Set 1 #

Total comments: 1

Patch Set 2 : Inline log messages, drop ReportLocalLoadFailed #

Patch Set 3 : Merge checks into BaseFetcherContext, update a few tests #

Patch Set 4 : content_browsertests #

Patch Set 5 : Fix content_browsertests #

Patch Set 6 : +comment #

Total comments: 2

Patch Set 7 : Revert ResourceLoader changes, handle main resource redirect in DocumentLoader #

Unified diffs Side-by-side diffs Delta from patch set Stats (+25 lines, -54 lines) Patch
M third_party/WebKit/LayoutTests/http/tests/misc/bubble-drag-events-expected.txt View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/http/tests/misc/drag-over-iframe-invalid-source-crash-expected.txt View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/http/tests/misc/iframe-invalid-source-crash-expected.txt View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/http/tests/security/filesystem-iframe-from-remote.html View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/LayoutTests/http/tests/security/filesystem-iframe-from-remote-expected.txt View 1 2 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/http/tests/security/local-iFrame-from-remote.html View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/LayoutTests/http/tests/security/local-iFrame-from-remote-expected.txt View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLFrameOwnerElement.cpp View 1 2 3 4 5 6 1 chunk +0 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLMediaElement.cpp View 1 1 chunk +5 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLPlugInElement.cpp View 1 2 3 4 5 6 1 chunk +0 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/loader/BaseFetchContext.h View 1 1 chunk +0 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/loader/BaseFetchContext.cpp View 1 2 1 chunk +4 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/loader/BaseFetchContextTest.cpp View 1 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 2 chunks +4 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/loader/FrameFetchContext.h View 1 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/loader/FrameFetchContext.cpp View 1 2 3 4 5 6 1 chunk +0 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/loader/FrameLoader.h View 1 1 chunk +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/loader/FrameLoader.cpp View 1 2 3 4 5 6 2 chunks +3 lines, -11 lines 0 comments Download
M third_party/WebKit/Source/core/loader/PingLoader.cpp View 1 1 chunk +0 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/loader/WorkerFetchContext.h View 1 2 3 4 5 6 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp View 1 2 3 4 5 6 1 chunk +0 lines, -5 lines 0 comments Download

Messages

Total messages: 51 (37 generated)
Nate Chapin
I don't know who the primary owner of ExecutionContext, any thoughts on who else (if ...
3 years, 7 months ago (2017-04-28 21:38:20 UTC) #11
kinuko
lgtm On 2017/04/28 21:38:20, Nate Chapin wrote: > I don't know who the primary owner ...
3 years, 7 months ago (2017-04-29 14:11:16 UTC) #12
Nate Chapin
Greetings, dom-dev! Are any of you good reviewers for ExecutionContext? I just want a sanity ...
3 years, 7 months ago (2017-05-01 19:42:37 UTC) #13
kinuko
On 2017/05/01 19:42:37, Nate Chapin wrote: > Greetings, dom-dev! > > Are any of you ...
3 years, 7 months ago (2017-05-02 01:08:16 UTC) #15
Nate Chapin
On 2017/05/02 01:08:16, kinuko wrote: > On 2017/05/01 19:42:37, Nate Chapin wrote: > > Greetings, ...
3 years, 7 months ago (2017-05-16 23:24:14 UTC) #16
dominicc (has gone to gerrit)
I think this is an improvement because there are fewer moving pieces, but on the ...
3 years, 7 months ago (2017-05-17 05:44:07 UTC) #17
kinuko
Yeah on the second thought I feel what dominicc says makes more sense. https://codereview.chromium.org/2842253002/diff/1/third_party/WebKit/Source/core/dom/ExecutionContext.cpp File ...
3 years, 7 months ago (2017-05-17 06:10:23 UTC) #18
kinuko
On 2017/05/17 06:10:23, kinuko wrote: > Yeah on the second thought I feel what dominicc ...
3 years, 7 months ago (2017-05-17 06:22:25 UTC) #19
Nate Chapin
On 2017/05/17 06:22:25, kinuko wrote: > On 2017/05/17 06:10:23, kinuko wrote: > > Yeah on ...
3 years, 7 months ago (2017-05-22 18:44:31 UTC) #34
kinuko
Cool, I like this better. https://codereview.chromium.org/2842253002/diff/100001/third_party/WebKit/Source/platform/loader/fetch/ResourceLoader.cpp File third_party/WebKit/Source/platform/loader/fetch/ResourceLoader.cpp (right): https://codereview.chromium.org/2842253002/diff/100001/third_party/WebKit/Source/platform/loader/fetch/ResourceLoader.cpp#newcode173 third_party/WebKit/Source/platform/loader/fetch/ResourceLoader.cpp:173: SecurityOrigin::Create(redirect_response.Url()); For DocumentLoader case ...
3 years, 7 months ago (2017-05-23 13:54:38 UTC) #35
Nate Chapin
https://codereview.chromium.org/2842253002/diff/100001/third_party/WebKit/Source/platform/loader/fetch/ResourceLoader.cpp File third_party/WebKit/Source/platform/loader/fetch/ResourceLoader.cpp (right): https://codereview.chromium.org/2842253002/diff/100001/third_party/WebKit/Source/platform/loader/fetch/ResourceLoader.cpp#newcode173 third_party/WebKit/Source/platform/loader/fetch/ResourceLoader.cpp:173: SecurityOrigin::Create(redirect_response.Url()); On 2017/05/23 13:54:38, kinuko wrote: > For DocumentLoader ...
3 years, 7 months ago (2017-05-23 21:20:50 UTC) #44
kinuko
On 2017/05/23 21:20:50, Nate Chapin wrote: > https://codereview.chromium.org/2842253002/diff/100001/third_party/WebKit/Source/platform/loader/fetch/ResourceLoader.cpp > File third_party/WebKit/Source/platform/loader/fetch/ResourceLoader.cpp (right): > > https://codereview.chromium.org/2842253002/diff/100001/third_party/WebKit/Source/platform/loader/fetch/ResourceLoader.cpp#newcode173 ...
3 years, 7 months ago (2017-05-23 23:26:59 UTC) #45
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/2842253002/120001
3 years, 7 months ago (2017-05-23 23:28:47 UTC) #47
commit-bot: I haz the power
3 years, 7 months ago (2017-05-23 23:48:09 UTC) #51
Message was sent while issue was closed.
Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/fd24fc9f7417c5e9a580911faa80...

Powered by Google App Engine
This is Rietveld 408576698