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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 14264012: Create errors (especially cancellation errors) internally to WebCore, rather (Closed)

Created:
7 years, 8 months ago by Nate Chapin
Modified:
7 years, 7 months ago
CC:
blink-reviews, jamesr, abarth_chromum.org, gavinp+loader_chromium.org, jeez, jochen+watch_chromium.org
Visibility:
Public.

Description

Create errors (especially cancellation errors) internally to WebCore, rather than plumbing all the way out to FrameLoaderClientImpl or RenderViewImpl. This will be accompanied by a later chromium patch to delete a bunch of dead code. BUG=none TEST=none R=darin@chromium.org Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=149342

Patch Set 1 #

Total comments: 2

Patch Set 2 : #

Total comments: 1

Patch Set 3 : #

Total comments: 4

Patch Set 4 : #

Patch Set 5 : #

Total comments: 5

Patch Set 6 : #

Patch Set 7 : #

Total comments: 1

Patch Set 8 : #

Total comments: 1

Patch Set 9 : #

Patch Set 10 : #

Patch Set 11 : #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+52 lines, -253 lines) Patch
M Source/Platform/Platform.gyp/Platform.gyp View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -0 lines 0 comments Download
M Source/Platform/chromium/public/Platform.h View 1 2 3 4 5 6 7 8 2 chunks +3 lines, -0 lines 0 comments Download
M Source/Platform/chromium/src/Platform.cpp View 1 2 3 4 5 6 7 8 9 10 2 chunks +10 lines, -0 lines 0 comments Download
M Source/WebKit/chromium/public/WebFrameClient.h View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -17 lines 0 comments Download
M Source/WebKit/chromium/src/FrameLoaderClientImpl.h View 1 2 3 4 5 6 7 8 2 chunks +0 lines, -8 lines 0 comments Download
M Source/WebKit/chromium/src/FrameLoaderClientImpl.cpp View 1 2 3 4 5 6 7 8 5 chunks +2 lines, -61 lines 0 comments Download
M Source/WebKit/chromium/tests/AssociatedURLLoaderTest.cpp View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -9 lines 0 comments Download
M Source/WebKit/chromium/tests/WebFrameTest.cpp View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -12 lines 0 comments Download
M Source/core/html/PluginDocument.cpp View 1 2 3 4 5 6 7 8 1 chunk +1 line, -1 line 0 comments Download
M Source/core/loader/DocumentLoader.h View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -1 line 0 comments Download
M Source/core/loader/DocumentLoader.cpp View 1 2 3 4 5 6 7 8 10 chunks +11 lines, -21 lines 0 comments Download
M Source/core/loader/EmptyClients.h View 1 2 3 4 5 6 7 8 2 chunks +0 lines, -12 lines 0 comments Download
M Source/core/loader/FrameLoader.h View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -2 lines 0 comments Download
M Source/core/loader/FrameLoader.cpp View 1 2 3 4 5 6 7 8 3 chunks +4 lines, -9 lines 1 comment Download
M Source/core/loader/FrameLoaderClient.h View 1 2 3 4 5 6 7 8 2 chunks +0 lines, -12 lines 0 comments Download
M Source/core/loader/ResourceLoader.h View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -3 lines 0 comments Download
M Source/core/loader/ResourceLoader.cpp View 1 2 3 4 5 6 7 8 2 chunks +1 line, -11 lines 0 comments Download
M Source/core/page/ContextMenuController.cpp View 1 2 3 4 5 6 7 8 2 chunks +6 lines, -11 lines 0 comments Download
M Source/core/platform/network/ResourceError.h View 1 2 3 4 5 6 7 8 9 1 chunk +2 lines, -0 lines 0 comments Download
M Source/core/platform/network/ResourceError.cpp View 1 2 3 4 5 6 7 8 9 10 1 chunk +11 lines, -0 lines 1 comment Download
M Tools/DumpRenderTree/chromium/TestRunner/public/WebTestProxy.h View 1 2 3 4 5 6 7 8 3 chunks +0 lines, -18 lines 0 comments Download
M Tools/DumpRenderTree/chromium/TestRunner/src/WebTestProxy.cpp View 1 2 3 4 5 6 7 8 2 chunks +0 lines, -28 lines 0 comments Download
M Tools/DumpRenderTree/chromium/WebViewHost.h View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -3 lines 0 comments Download
M Tools/DumpRenderTree/chromium/WebViewHost.cpp View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -14 lines 0 comments Download

Messages

Total messages: 17 (0 generated)
Nate Chapin
https://codereview.chromium.org/14264012/diff/1/Source/WebCore/platform/network/ResourceError.cpp File Source/WebCore/platform/network/ResourceError.cpp (right): https://codereview.chromium.org/14264012/diff/1/Source/WebCore/platform/network/ResourceError.cpp#newcode35 Source/WebCore/platform/network/ResourceError.cpp:35: const int cancelledErrorCode = -3; This is the only ...
7 years, 8 months ago (2013-04-15 17:44:32 UTC) #1
Nate Chapin
https://codereview.chromium.org/14264012/diff/5003/LayoutTests/http/tests/misc/redirect-to-external-url.html File LayoutTests/http/tests/misc/redirect-to-external-url.html (left): https://codereview.chromium.org/14264012/diff/5003/LayoutTests/http/tests/misc/redirect-to-external-url.html#oldcode1 LayoutTests/http/tests/misc/redirect-to-external-url.html:1: <html> redirect-to-external-url.html tests a WebKit concept we don't use: ...
7 years, 8 months ago (2013-04-15 19:18:24 UTC) #2
darin (slow to review)
On 2013/04/15 17:44:32, Nate Chapin wrote: > https://codereview.chromium.org/14264012/diff/1/Source/WebCore/platform/network/ResourceError.cpp > File Source/WebCore/platform/network/ResourceError.cpp (right): > > https://codereview.chromium.org/14264012/diff/1/Source/WebCore/platform/network/ResourceError.cpp#newcode35 ...
7 years, 8 months ago (2013-04-16 18:43:26 UTC) #3
Nate Chapin
On 2013/04/16 18:43:26, darin wrote: > On 2013/04/15 17:44:32, Nate Chapin wrote: > > > ...
7 years, 8 months ago (2013-04-16 20:14:03 UTC) #4
darin (slow to review)
https://codereview.chromium.org/14264012/diff/15001/Source/WebCore/platform/network/ResourceError.h File Source/WebCore/platform/network/ResourceError.h (right): https://codereview.chromium.org/14264012/diff/15001/Source/WebCore/platform/network/ResourceError.h#newcode38 Source/WebCore/platform/network/ResourceError.h:38: extern const int policyChangeError; nit: this is a bit ...
7 years, 8 months ago (2013-04-16 20:25:39 UTC) #5
Nate Chapin
https://codereview.chromium.org/14264012/diff/15001/Source/WebCore/platform/network/ResourceError.h File Source/WebCore/platform/network/ResourceError.h (right): https://codereview.chromium.org/14264012/diff/15001/Source/WebCore/platform/network/ResourceError.h#newcode38 Source/WebCore/platform/network/ResourceError.h:38: extern const int policyChangeError; On 2013/04/16 20:25:39, darin wrote: ...
7 years, 8 months ago (2013-04-16 20:40:43 UTC) #6
darin (slow to review)
Apologies, my feedback is now a bit contrary to some of my earlier feedback :-/ ...
7 years, 8 months ago (2013-04-17 18:58:11 UTC) #7
Nate Chapin
https://codereview.chromium.org/14264012/diff/16033/Source/WebCore/loader/FrameLoader.cpp File Source/WebCore/loader/FrameLoader.cpp (right): https://codereview.chromium.org/14264012/diff/16033/Source/WebCore/loader/FrameLoader.cpp#newcode2509 Source/WebCore/loader/FrameLoader.cpp:2509: ResourceError c = CancelledResourceError(KURL()); On 2013/04/17 18:58:11, darin wrote: ...
7 years, 8 months ago (2013-04-17 20:45:05 UTC) #8
darin (slow to review)
https://codereview.chromium.org/14264012/diff/35004/Source/core/platform/network/ResourceError.cpp File Source/core/platform/network/ResourceError.cpp (right): https://codereview.chromium.org/14264012/diff/35004/Source/core/platform/network/ResourceError.cpp#newcode38 Source/core/platform/network/ResourceError.cpp:38: ResourceError error("net", WebKit::Platform::current()->cancelledErrorCode(), failingURL, String()); Are there any issues ...
7 years, 8 months ago (2013-04-18 05:47:39 UTC) #9
darin (slow to review)
See https://code.google.com/p/chromium/codesearch#chromium/src/webkit/glue/weburlloader_impl.cc&q=weburlloader_impl.cc&sq=package:chromium&type=cs&l=745
7 years, 8 months ago (2013-04-18 05:48:43 UTC) #10
Nate Chapin
https://codereview.chromium.org/14264012/diff/44001/Source/Platform/chromium/public/Platform.h File Source/Platform/chromium/public/Platform.h (right): https://codereview.chromium.org/14264012/diff/44001/Source/Platform/chromium/public/Platform.h#newcode276 Source/Platform/chromium/public/Platform.h:276: virtual const char* cancelledErrorDomain() const { return "net"; } ...
7 years, 8 months ago (2013-04-22 16:13:41 UTC) #11
darin (slow to review)
Yeah, how about just adding a WebURLError Platform::cancelledError(const WebString& failingURL) method?
7 years, 8 months ago (2013-04-22 16:20:08 UTC) #12
Nate Chapin
On 2013/04/22 16:20:08, darin wrote: > Yeah, how about just adding a WebURLError Platform::cancelledError(const > ...
7 years, 8 months ago (2013-04-22 17:24:34 UTC) #13
darin (slow to review)
You shouldn't need to include WebURLError.h to declare a method returning WebURLError. (That WebURL.h / ...
7 years, 8 months ago (2013-04-22 17:41:57 UTC) #14
Nate Chapin
On 2013/04/22 17:41:57, darin wrote: > You shouldn't need to include WebURLError.h to declare a ...
7 years, 8 months ago (2013-04-22 17:45:08 UTC) #15
darin (slow to review)
LGTM https://codereview.chromium.org/14264012/diff/73001/Source/core/loader/FrameLoader.cpp File Source/core/loader/FrameLoader.cpp (right): https://codereview.chromium.org/14264012/diff/73001/Source/core/loader/FrameLoader.cpp#newcode2448 Source/core/loader/FrameLoader.cpp:2448: if (error.errorCode() != c.errorCode() || error.domain() != c.domain()) ...
7 years, 8 months ago (2013-04-25 18:15:54 UTC) #16
Nate Chapin
7 years, 7 months ago (2013-04-29 14:46:24 UTC) #17
Message was sent while issue was closed. 
Committed patchset #11 manually as r149342 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698