content/browser/network_error_request_job.cc - Issue 1421743002: Implement chrome://network-errors for direct access to network error interstitials

Side by Side Diff: content/browser/network_error_request_job.cc

Issue 1421743002: Implement chrome://network-errors for direct access to network error interstitials (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Created 5 years, 1 month ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« content/browser/network_error_request_job.h ('K') | « content/browser/network_error_request_job.h ('k') | content/browser/webui/url_data_manager_backend.cc » ('j') | content/browser/webui/url_data_manager_backend.cc » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 // Copyright (c) 2015 The Chromium Authors. All rights reserved.
	mmenke 2015/10/22 15:41:31 (c) not needed in new files. (c) not needed in new files. edwardjung 2015/11/06 12:05:50 Done. Show quoted text On 2015/10/22 15:41:31, mmenke wrote: > (c) not needed in new files. Done.
	2 // Use of this source code is governed by a BSD-style license that can be

	3 // found in the LICENSE file.

	4

	5 #include "content/browser/network_error_request_job.h"

	6

	7 #include "base/strings/string_number_conversions.h"

	8 #include "base/values.h"

	9 #include "net/base/escape.h"

	10 #include "net/base/net_errors.h"

	11 #include "net/url_request/url_request.h"

	12 #include "net/url_request/url_request_error_job.h"
	mmenke 2015/10/22 15:41:31 Not needed, since you're using another approach (W Not needed, since you're using another approach (Which is fine) edwardjung 2015/11/06 12:05:50 Done. Show quoted text On 2015/10/22 15:41:31, mmenke wrote: > Not needed, since you're using another approach (Which is fine) Done.
	13 #include "url/gurl.h"

	14

	15 namespace content {

	16

	17 NetworkErrorRequestJob::NetworkErrorRequestJob(
	mmenke 2015/10/22 15:41:31 I think we want browser tests for this, both that I think we want browser tests for this, both that it displays an error page, and for the index. Error pages are generated in chrome/, but I think we can actually make a content/ test for this - a WebContentsObserver does see error codes for navigations.
	18 net::URLRequest* request,

	19 net::NetworkDelegate* network_delegate)

	20 : net::URLRequestSimpleJob(request, network_delegate) {
	mmenke 2015/10/22 15:41:31 BUG: Need to initialize error_code_. BUG: Need to initialize error_code_. edwardjung 2015/11/06 12:05:50 Done. Show quoted text On 2015/10/22 15:41:31, mmenke wrote: > BUG: Need to initialize error_code_. Done.
	21 const std::string& spec = request->url().possibly_invalid_spec();

	22 const url::Parsed& parsed = request->url().parsed_for_possibly_invalid_spec();

	23 // + 1 to skip the slash at the beginning of the path.

	24 int offset = parsed.CountCharactersBefore(url::Parsed::PATH, false) + 1;
	mmenke 2015/10/22 15:41:31 Just use this: request->url().path().substr(1, st Just use this: request->url().path().substr(1, std::string::npos); edwardjung 2015/11/06 12:05:51 Thanks. Show quoted text On 2015/10/22 15:41:31, mmenke wrote: > Just use this: request->url().path().substr(1, std::string::npos); Thanks.
	25

	26 if (offset < static_cast<int>(spec.size())) {
	mmenke 2015/10/22 15:41:31 Using above suggestion, this just becomes: if (!b Using above suggestion, this just becomes: if (!base::StringToInt(whatever, &error_code_)) error_code_ = 0; edwardjung 2015/11/06 12:05:50 Done. Show quoted text On 2015/10/22 15:41:31, mmenke wrote: > Using above suggestion, this just becomes: > > if (!base::StringToInt(whatever, &error_code_)) > error_code_ = 0; Done.
	27 std::string path;

	28 path.assign(spec.substr(offset));

	29 base::StringToInt(path, &error_code_);

	30 }

	31 }

	32

	33 void NetworkErrorRequestJob::ListAllErrorCodes(std::string* data) {
	mmenke 2015/10/22 15:41:30 This can just go in an anonymous namespace. This can just go in an anonymous namespace. edwardjung 2015/11/06 12:05:50 Done. Show quoted text On 2015/10/22 15:41:30, mmenke wrote: > This can just go in an anonymous namespace. Done.
	34 std::string unescaped_query;

	35 std::string unescaped_title("Network Error codes");

	36

	37 data->append("<!DOCTYPE html>\n<html>\n<head>\n");

	38 data->append(

	39 "<meta http-equiv=\"Content-Security-Policy\" "

	40 "content=\"object-src 'none'; script-src 'none'\">");

	41 data->append("<title>");

	42 data->append(net::EscapeForHTML(unescaped_title));
	mmenke 2015/10/22 15:41:31 Why is this needed? Why is this needed? edwardjung 2015/11/06 12:05:50 Removed. Show quoted text On 2015/10/22 15:41:31, mmenke wrote: > Why is this needed? Removed.
	43 data->append("</title>\n");

	44 data->append("</head><body>");

	45 data->append("<h1>Network errors</h1>");

	46 data->append("<ul>");

	47 #define NET_ERROR(label, value) \

	48 data->append("<li><a href=" #value ">" #label "</a></li>");
	mmenke 2015/10/22 15:41:30 I really don't like to add all these strings. The I really don't like to add all these strings. There are a couple ways to fix this. net::GetNetConstants() returns a dictionary which contains a list of network errors in its "netError" field. We could either use that, or move the array with the into net_errors.h, and provide some public way to access it (Problem with making it accessible it easily accessible is we don't know its length). Or we could add a method IsValidNetError, and make it use the same backing code as ErrorToShortString, and then just call it on all numbers from -2 to -2000... We also want to exclude net::OK (Which is 0, so will just display all error codes) and maybe net::ERR_IO_PENDING from the list (Which will just cause the navigation to hang).
	49 #include "net/base/net_error_list.h"

	50 #undef NET_ERROR

	51 data->append("</ul>");
	mmenke 2015/10/22 15:41:30 </body></html>? </body></html>? edwardjung 2015/11/06 12:05:50 Technically not needed in HTML5. Show quoted text On 2015/10/22 15:41:30, mmenke wrote: > </body></html>? Technically not needed in HTML5.
	52 }

	53

	54 int NetworkErrorRequestJob::GetData(

	55 std::string* mime_type,

	56 std::string* charset,

	57 std::string* data,

	58 const net::CompletionCallback& callback) const {

	59 mime_type->assign("text/html");

	60 charset->assign("UTF8");

	61 data->clear();
	mmenke 2015/10/22 15:41:30 Not needed. Not needed. edwardjung 2015/11/06 12:05:50 Done. Show quoted text On 2015/10/22 15:41:30, mmenke wrote: > Not needed. Done.
	62

	63 if (error_code_ != 0) {

	64 return error_code_;
	mmenke 2015/10/22 15:41:31 BUG: This will cause invalid memory accesses if e BUG: This will cause invalid memory accesses if error_code_ is > 0, and a hung request if it's ERR_IO_PENDING (Though that one is not really the end of the world). I also don't think we want to allow passing non-standard network error codes to the renderer. That will, at least, hit a NOTREACHED in debug builds.
	65 } else {

	66 ListAllErrorCodes(data);

	67 return net::OK;
	mmenke 2015/10/22 15:41:30 I'd actually include this case first (As the "erro I'd actually include this case first (As the "error" / fallback case), and get rid of the else. edwardjung 2015/11/06 12:05:50 Done. Show quoted text On 2015/10/22 15:41:30, mmenke wrote: > I'd actually include this case first (As the "error" / fallback case), and get > rid of the else. Done.
	68 }

	69 }

	70

	71 } // namespace content

OLD	NEW