Skip to content

Commit

Permalink
Make WebURLError member variables private
Browse files Browse the repository at this point in the history
This change makes WebURLError member variables private and 
non-writable (except for whole assignment) to forbid random state
updates.

Bug: 748491
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: Ie9260a5dc078e6fb9669a3c46e770b756efb7e51
Reviewed-on: https://chromium-review.googlesource.com/750482
Reviewed-by: Mark Seaborn <mseaborn@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: David Dorwin <ddorwin@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514408}
  • Loading branch information
yutakahirano authored and Commit Bot committed Nov 7, 2017
1 parent 21ae2fa commit a394085
Show file tree
Hide file tree
Showing 24 changed files with 147 additions and 134 deletions.
8 changes: 4 additions & 4 deletions android_webview/renderer/aw_content_renderer_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -213,11 +213,11 @@ void AwContentRendererClient::GetNavigationErrorStrings(
std::string* error_html,
base::string16* error_description) {
std::string err;
if (error.domain == blink::WebURLError::Domain::kNet) {
if (error.reason == net::ERR_TEMPORARILY_THROTTLED)
if (error.domain() == blink::WebURLError::Domain::kNet) {
if (error.reason() == net::ERR_TEMPORARILY_THROTTLED)
err = kThrottledErrorDescription;
else
err = net::ErrorToString(error.reason);
err = net::ErrorToString(error.reason());
}
if (error_description)
*error_description = base::ASCIIToUTF16(err);
Expand All @@ -230,7 +230,7 @@ void AwContentRendererClient::GetNavigationErrorStrings(
std::string url_string = gurl.possibly_invalid_spec();
int reason_id = IDS_AW_WEBPAGE_CAN_NOT_BE_LOADED;

if (error.reason == net::ERR_BLOCKED_BY_ADMINISTRATOR) {
if (error.reason() == net::ERR_BLOCKED_BY_ADMINISTRATOR) {
// This creates a different error page giving considerably more
// detail, and possibly allowing the user to request access.
// Get the details this needs from the browser.
Expand Down
6 changes: 3 additions & 3 deletions chrome/renderer/chrome_content_renderer_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1168,11 +1168,11 @@ void ChromeContentRendererClient::GetNavigationErrorStrings(
const blink::WebURLError& web_error,
std::string* error_html,
base::string16* error_description) {
DCHECK_EQ(WebURLError::Domain::kNet, web_error.domain);
DCHECK_EQ(WebURLError::Domain::kNet, web_error.domain());
GetNavigationErrorStringsInternal(
render_frame, failed_request,
error_page::Error::NetError(web_error.unreachable_url, web_error.reason,
web_error.stale_copy_in_cache),
error_page::Error::NetError(web_error.url(), web_error.reason(),
web_error.has_copy_in_cache()),
error_html, error_description);
}

Expand Down
6 changes: 3 additions & 3 deletions components/nacl/renderer/file_downloader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,16 +74,16 @@ void FileDownloader::DidFinishLoading(double finish_time) {

void FileDownloader::DidFail(const blink::WebURLError& error) {
status_ = FAILED;
if (error.domain == blink::WebURLError::Domain::kNet) {
switch (error.reason) {
if (error.domain() == blink::WebURLError::Domain::kNet) {
switch (error.reason()) {
case net::ERR_ACCESS_DENIED:
case net::ERR_NETWORK_ACCESS_DENIED:
status_ = ACCESS_DENIED;
break;
}
}

if (error.is_web_security_violation)
if (error.is_web_security_violation())
status_ = ACCESS_DENIED;

// Delete url_loader to prevent didFinishLoading from being called, which
Expand Down
6 changes: 3 additions & 3 deletions components/nacl/renderer/manifest_downloader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,16 +71,16 @@ void ManifestDownloader::DidFinishLoading(double finish_time) {
void ManifestDownloader::DidFail(const blink::WebURLError& error) {
// TODO(teravest): Find a place to share this code with PepperURLLoaderHost.
pp_nacl_error_ = PP_NACL_ERROR_MANIFEST_LOAD_URL;
if (error.domain == blink::WebURLError::Domain::kNet) {
switch (error.reason) {
if (error.domain() == blink::WebURLError::Domain::kNet) {
switch (error.reason()) {
case net::ERR_ACCESS_DENIED:
case net::ERR_NETWORK_ACCESS_DENIED:
pp_nacl_error_ = PP_NACL_ERROR_MANIFEST_NOACCESS_URL;
break;
}
}

if (error.is_web_security_violation)
if (error.is_web_security_violation())
pp_nacl_error_ = PP_NACL_ERROR_MANIFEST_NOACCESS_URL;

Close();
Expand Down
4 changes: 2 additions & 2 deletions content/renderer/browser_render_view_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ class TestShellContentRendererClient : public ShellContentRendererClient {
if (error_html)
*error_html = "A suffusion of yellow.";
latest_error_valid_ = true;
latest_error_reason_ = error.reason;
latest_error_stale_copy_in_cache_ = error.stale_copy_in_cache;
latest_error_reason_ = error.reason();
latest_error_stale_copy_in_cache_ = error.has_copy_in_cache();
}

bool GetLatestError(int* error_code, bool* stale_cache_entry_present) {
Expand Down
26 changes: 17 additions & 9 deletions content/renderer/loader/web_url_loader_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -921,7 +921,11 @@ void WebURLLoaderImpl::Context::OnCompletedRequest(
this, TRACE_EVENT_FLAG_FLOW_IN);

if (error_code != net::OK) {
WebURLError error(url_, stale_copy_in_cache, error_code);
WebURLError error(WebURLError::Domain::kNet, error_code,
stale_copy_in_cache
? WebURLError::HasCopyInCache::kTrue
: WebURLError::HasCopyInCache::kFalse,
WebURLError::IsWebSecurityViolation::kFalse, url_);
client_->DidFail(error, total_transfer_size, encoded_body_size,
decoded_body_size);
} else {
Expand Down Expand Up @@ -958,8 +962,9 @@ void WebURLLoaderImpl::Context::CancelBodyStreaming() {
}
if (client_) {
// TODO(yhirano): Set |stale_copy_in_cache| appropriately if possible.
client_->DidFail(WebURLError(url_, false, net::ERR_ABORTED),
WebURLLoaderClient::kUnknownEncodedDataLength, 0, 0);
client_->DidFail(
WebURLError(WebURLError::Domain::kNet, net::ERR_ABORTED, url_),
WebURLLoaderClient::kUnknownEncodedDataLength, 0, 0);
}

// Notify the browser process that the request is canceled.
Expand Down Expand Up @@ -1374,12 +1379,15 @@ void WebURLLoaderImpl::LoadSynchronously(const WebURLRequest& request,
// status code or status text.
int error_code = sync_load_response.error_code;
if (error_code != net::OK) {
error = WebURLError(final_url, false, error_code);
if (error_code == net::ERR_ABORTED) {
// SyncResourceHandler returns ERR_ABORTED for CORS redirect errors,
// so we treat the error as a web security violation.
error.is_web_security_violation = true;
}
// SyncResourceHandler returns ERR_ABORTED for CORS redirect errors,
// so we treat the error as a web security violation.
const bool is_web_security_violation = error_code == net::ERR_ABORTED;
error = WebURLError(WebURLError::Domain::kNet, error_code,
WebURLError::HasCopyInCache::kFalse,
is_web_security_violation
? WebURLError::IsWebSecurityViolation::kTrue
: WebURLError::IsWebSecurityViolation::kFalse,
final_url);
return;
}

Expand Down
20 changes: 10 additions & 10 deletions content/renderer/loader/web_url_loader_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,8 @@ class TestWebURLLoaderClient : public blink::WebURLLoaderClient {
// The response should have started, but must not have finished, or failed.
EXPECT_TRUE(did_receive_response_);
EXPECT_FALSE(did_finish_);
EXPECT_EQ(net::OK, error_.reason);
EXPECT_EQ(blink::WebURLError::Domain::kEmpty, error_.domain);
EXPECT_EQ(net::OK, error_.reason());
EXPECT_EQ(blink::WebURLError::Domain::kEmpty, error_.domain());

received_data_.append(data, dataLength);

Expand Down Expand Up @@ -357,8 +357,8 @@ class WebURLLoaderImplTest : public testing::Test {
strlen(kTestData));
EXPECT_TRUE(client()->did_finish());
// There should be no error.
EXPECT_EQ(net::OK, client()->error().reason);
EXPECT_EQ(blink::WebURLError::Domain::kEmpty, client()->error().domain);
EXPECT_EQ(net::OK, client()->error().reason());
EXPECT_EQ(blink::WebURLError::Domain::kEmpty, client()->error().domain());
}

void DoFailRequest() {
Expand All @@ -367,8 +367,8 @@ class WebURLLoaderImplTest : public testing::Test {
strlen(kTestData), strlen(kTestData),
strlen(kTestData));
EXPECT_FALSE(client()->did_finish());
EXPECT_EQ(net::ERR_FAILED, client()->error().reason);
EXPECT_EQ(blink::WebURLError::Domain::kNet, client()->error().domain);
EXPECT_EQ(net::ERR_FAILED, client()->error().reason());
EXPECT_EQ(blink::WebURLError::Domain::kNet, client()->error().domain());
}

void DoReceiveResponseFtp() {
Expand Down Expand Up @@ -478,8 +478,8 @@ TEST_F(WebURLLoaderImplTest, DataURL) {
base::RunLoop().RunUntilIdle();
EXPECT_EQ("blah!", client()->received_data());
EXPECT_TRUE(client()->did_finish());
EXPECT_EQ(net::OK, client()->error().reason);
EXPECT_EQ(blink::WebURLError::Domain::kEmpty, client()->error().domain);
EXPECT_EQ(net::OK, client()->error().reason());
EXPECT_EQ(blink::WebURLError::Domain::kEmpty, client()->error().domain());
}

TEST_F(WebURLLoaderImplTest, DataURLDeleteOnReceiveResponse) {
Expand Down Expand Up @@ -544,8 +544,8 @@ TEST_F(WebURLLoaderImplTest, DataURLDefersLoading) {
EXPECT_TRUE(client()->did_finish());

EXPECT_EQ("blah!", client()->received_data());
EXPECT_EQ(net::OK, client()->error().reason);
EXPECT_EQ(blink::WebURLError::Domain::kEmpty, client()->error().domain);
EXPECT_EQ(net::OK, client()->error().reason());
EXPECT_EQ(blink::WebURLError::Domain::kEmpty, client()->error().domain());
}

TEST_F(WebURLLoaderImplTest, DefersLoadingBeforeStart) {
Expand Down
6 changes: 3 additions & 3 deletions content/renderer/pepper/pepper_url_loader_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -177,18 +177,18 @@ void PepperURLLoaderHost::DidFinishLoading(double finish_time) {
void PepperURLLoaderHost::DidFail(const WebURLError& error) {
// Note that |loader| will be NULL for document loads.
int32_t pp_error = PP_ERROR_FAILED;
if (error.domain == WebURLError::Domain::kNet) {
if (error.domain() == WebURLError::Domain::kNet) {
// TODO(bbudge): Extend pp_errors.h to cover interesting network errors
// from the net error domain.
switch (error.reason) {
switch (error.reason()) {
case net::ERR_ACCESS_DENIED:
case net::ERR_NETWORK_ACCESS_DENIED:
pp_error = PP_ERROR_NOACCESS;
break;
}
}

if (error.is_web_security_violation)
if (error.is_web_security_violation())
pp_error = PP_ERROR_NOACCESS;
SendUpdateToPlugin(
std::make_unique<PpapiPluginMsg_URLLoader_FinishedLoading>(pp_error));
Expand Down
37 changes: 19 additions & 18 deletions content/renderer/render_frame_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2620,7 +2620,7 @@ void RenderFrameImpl::DidFailProvisionalLoadInternal(
// Notify the browser that we failed a provisional load with an error.
SendFailedProvisionalLoad(failed_request, error, frame_);

if (!ShouldDisplayErrorPageForFailedLoad(error.reason, error.unreachable_url))
if (!ShouldDisplayErrorPageForFailedLoad(error.reason(), error.url()))
return;

// Make sure we never show errors in view source mode.
Expand Down Expand Up @@ -2661,13 +2661,13 @@ void RenderFrameImpl::LoadNavigationErrorPage(
: blink::WebFrameLoadType::kStandard;
const blink::WebHistoryItem& history_item =
entry ? entry->root() : blink::WebHistoryItem();
DCHECK_EQ(WebURLError::Domain::kNet, error.domain);
DCHECK_EQ(WebURLError::Domain::kNet, error.domain());

// Requests blocked by the X-Frame-Options HTTP response header don't display
// error pages but a blank page instead.
// TODO(alexmos, mkwst, arthursonzogni): This block can be removed once error
// pages are refactored. See crbug.com/588314 and crbug.com/622385.
if (error.reason == net::ERR_BLOCKED_BY_RESPONSE) {
if (error.reason() == net::ERR_BLOCKED_BY_RESPONSE) {
// Do not preserve the history item for blocked navigations, since we will
// not attempt to reload it later. Also, it is important that the document
// sequence number is not preserved, so that other navigations will not be
Expand All @@ -2687,8 +2687,8 @@ void RenderFrameImpl::LoadNavigationErrorPage(
this, failed_request, error, &error_html, nullptr);
}
LoadNavigationErrorPageInternal(error_html, GURL(kUnreachableWebDataURL),
error.unreachable_url, replace,
frame_load_type, history_item);
error.url(), replace, frame_load_type,
history_item);
}

void RenderFrameImpl::LoadNavigationErrorPageForHttpStatusError(
Expand Down Expand Up @@ -2839,10 +2839,8 @@ blink::WebPlugin* RenderFrameImpl::CreatePlugin(
}

void RenderFrameImpl::LoadErrorPage(int reason) {
blink::WebURLError error;
error.unreachable_url = frame_->GetDocument().Url();
error.domain = blink::WebURLError::Domain::kNet;
error.reason = reason;
blink::WebURLError error(blink::WebURLError::Domain::kNet, reason,
frame_->GetDocument().Url());

std::string error_html;
GetContentClient()->renderer()->GetNavigationErrorStrings(
Expand All @@ -2851,8 +2849,8 @@ void RenderFrameImpl::LoadErrorPage(int reason) {

frame_->LoadData(error_html, WebString::FromUTF8("text/html"),
WebString::FromUTF8("UTF-8"), GURL(kUnreachableWebDataURL),
error.unreachable_url, true,
blink::WebFrameLoadType::kStandard, blink::WebHistoryItem(),
error.url(), true, blink::WebFrameLoadType::kStandard,
blink::WebHistoryItem(),
blink::kWebHistoryDifferentDocumentLoad, true);
}

Expand Down Expand Up @@ -4212,8 +4210,8 @@ void RenderFrameImpl::DidFailLoad(const blink::WebURLError& error,
error,
nullptr,
&error_description);
Send(new FrameHostMsg_DidFailLoadWithError(routing_id_, failed_request.Url(),
error.reason, error_description));
Send(new FrameHostMsg_DidFailLoadWithError(
routing_id_, failed_request.Url(), error.reason(), error_description));
}

void RenderFrameImpl::DidFinishLoad() {
Expand Down Expand Up @@ -5495,8 +5493,11 @@ void RenderFrameImpl::OnFailedNavigation(
common_params, StartNavigationParams(), request_params));

// Send the provisional load failure.
blink::WebURLError error(common_params.url, has_stale_copy_in_cache,
error_code);
WebURLError error(
WebURLError::Domain::kNet, error_code,
has_stale_copy_in_cache ? WebURLError::HasCopyInCache::kTrue
: WebURLError::HasCopyInCache::kFalse,
WebURLError::IsWebSecurityViolation::kFalse, common_params.url);
WebURLRequest failed_request =
CreateURLRequestForNavigation(common_params, request_params,
std::unique_ptr<StreamOverrideParameters>(),
Expand Down Expand Up @@ -6812,14 +6813,14 @@ void RenderFrameImpl::SendFailedProvisionalLoad(
const blink::WebURLError& error,
blink::WebLocalFrame* frame) {
bool show_repost_interstitial =
(error.reason == net::ERR_CACHE_MISS &&
(error.reason() == net::ERR_CACHE_MISS &&
base::EqualsASCII(request.HttpMethod().Utf16(), "POST"));

FrameHostMsg_DidFailProvisionalLoadWithError_Params params;
params.error_code = error.reason;
params.error_code = error.reason();
GetContentClient()->renderer()->GetNavigationErrorStrings(
this, request, error, nullptr, &params.error_description);
params.url = error.unreachable_url;
params.url = error.url(),
params.showing_repost_interstitial = show_repost_interstitial;
Send(new FrameHostMsg_DidFailProvisionalLoadWithError(routing_id_, params));
}
Expand Down
24 changes: 8 additions & 16 deletions content/renderer/render_view_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1397,10 +1397,8 @@ TEST_F(RenderViewImplTest, OnSetTextDirection) {
// Crashy, http://crbug.com/53247.
TEST_F(RenderViewImplTest, DISABLED_DidFailProvisionalLoadWithErrorForError) {
GetMainFrame()->EnableViewSourceMode(true);
WebURLError error;
error.domain = WebURLError::Domain::kNet;
error.reason = net::ERR_FILE_NOT_FOUND;
error.unreachable_url = GURL("http://foo");
WebURLError error(WebURLError::Domain::kNet, net::ERR_FILE_NOT_FOUND,
GURL("http://foo"));
WebLocalFrame* web_frame = GetMainFrame();

// Start a load that will reach provisional state synchronously,
Expand All @@ -1420,10 +1418,8 @@ TEST_F(RenderViewImplTest, DISABLED_DidFailProvisionalLoadWithErrorForError) {

TEST_F(RenderViewImplTest, DidFailProvisionalLoadWithErrorForCancellation) {
GetMainFrame()->EnableViewSourceMode(true);
WebURLError error;
error.domain = WebURLError::Domain::kNet;
error.reason = net::ERR_ABORTED;
error.unreachable_url = GURL("http://foo");
WebURLError error(WebURLError::Domain::kNet, net::ERR_ABORTED,
GURL("http://foo"));
WebLocalFrame* web_frame = GetMainFrame();

// Start a load that will reach provisional state synchronously,
Expand Down Expand Up @@ -1903,10 +1899,8 @@ class RendererErrorPageTest : public RenderViewImplTest {
#endif

TEST_F(RendererErrorPageTest, MAYBE_Suppresses) {
WebURLError error;
error.domain = WebURLError::Domain::kNet;
error.reason = net::ERR_FILE_NOT_FOUND;
error.unreachable_url = GURL("http://example.com/suppress");
WebURLError error(WebURLError::Domain::kNet, net::ERR_FILE_NOT_FOUND,
GURL("http://example.com/suppress"));

// Start a load that will reach provisional state synchronously,
// but won't complete synchronously.
Expand All @@ -1933,10 +1927,8 @@ TEST_F(RendererErrorPageTest, MAYBE_Suppresses) {
#endif

TEST_F(RendererErrorPageTest, MAYBE_DoesNotSuppress) {
WebURLError error;
error.domain = WebURLError::Domain::kNet;
error.reason = net::ERR_FILE_NOT_FOUND;
error.unreachable_url = GURL("http://example.com/dont-suppress");
WebURLError error(WebURLError::Domain::kNet, net::ERR_FILE_NOT_FOUND,
GURL("http://example.com/dont-suppress"));

// Start a load that will reach provisional state synchronously,
// but won't complete synchronously.
Expand Down
6 changes: 3 additions & 3 deletions content/shell/renderer/shell_content_renderer_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,9 @@ void ShellContentRendererClient::GetNavigationErrorStrings(
*error_html =
"<head><title>Error</title></head><body>Could not load the requested "
"resource.<br/>Error code: " +
base::IntToString(error.reason) +
(error.reason < 0 ? " (" + net::ErrorToString(error.reason) + ")"
: "") +
base::IntToString(error.reason()) +
(error.reason() < 0 ? " (" + net::ErrorToString(error.reason()) + ")"
: "") +
"</body>";
}
}
Expand Down
3 changes: 2 additions & 1 deletion media/blink/resource_multibuffer_data_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,8 @@ void ResourceMultiBufferDataProvider::DidFinishLoading(double finishTime) {
}

void ResourceMultiBufferDataProvider::DidFail(const WebURLError& error) {
DVLOG(1) << "didFail: reason=" << error.reason << ", domain=" << error.domain;
DVLOG(1) << "didFail: reason=" << error.reason()
<< ", domain=" << error.domain();
DCHECK(active_loader_.get());
active_loader_.reset();

Expand Down
Loading

0 comments on commit a394085

Please sign in to comment.