LTTF: Implement WebFrameLoaderClient::dispatchUnableToImplementPolicy() and so on

Make LayoutTests/http/tests/misc/redirect-to-external-url.html PASS.

 * Implement WebFrameLoaderClient::dispatchUnableToImplementPolicy()
 * Implement WebFrameLoaderClient::canHandleRequest() so that an
   unkown scheme makes ResourceError.
   - Introduce WebFrameClient::canHandleRequest() to avoid chrome::
     dependency from webkit/glue.
   - Introduce WebFrameClient::cannotShowURLError() to keep
     transparency of WebURLError.
 * Introduce WebFrameClient::unableToImplementPolicyWithError()
   An implmentation in test_shell prints the message same as WebKit.

BUG=24074
TEST=This change enables 1 test.

[darin (slow to review), 2009-10-16 07:10:41]

http://codereview.chromium.org/271087/diff/9001/6018
File chrome/common/url_constants.h (right):

http://codereview.chromium.org/271087/diff/9001/6018#newcode32
Line 32: // Null terminated list of all known schems.
nit: s/schems/schemes/

http://codereview.chromium.org/271087/diff/9001/6020
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/271087/diff/9001/6020#newcode1923
Line 1923: return URLRequest::IsHandledProtocol(scheme);
i'm afraid you cannot call this from the renderer.  you need to proxy this to
the IO thread of the browser using a synchronous IPC.

that said, why do we need this code here?  previously, we were always returning
true.  what bug is fixed?

i believe we already handle the behavior the layout test is checking for in the
browser.  take a look at where IsHandledURL is used by chrome in
resource_dispatcher_host.cc.

http://codereview.chromium.org/271087/diff/9001/6020#newcode1929
Line 1929: error.unreachableURL = request.url();
shouldn't you set the other fields of WebURLError?

http://codereview.chromium.org/271087/diff/9001/6016
File webkit/glue/webframeloaderclient_impl.cc (right):

http://codereview.chromium.org/271087/diff/9001/6016#newcode29
Line 29: #include "net/url_request/url_request.h"
looks like you can delete this include

http://codereview.chromium.org/271087/diff/9001/6016#newcode1045
Line 1045: const {
nit: google style says not to float the "const" like this.  instead you should
line break after the "("

[tkent, 2009-10-16 07:50:01]

The patch has been updated.

http://codereview.chromium.org/271087/diff/9001/6020
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/271087/diff/9001/6020#newcode1923
Line 1923: return URLRequest::IsHandledProtocol(scheme);
On 2009/10/16 07:10:41, darin wrote:
> that said, why do we need this code here?  previously, we were always
returning
> true.  what bug is fixed?

I don't know.  The old code had FIXME, so I tried to fix :-)
Ok, I'll keep "return true".

http://codereview.chromium.org/271087/diff/9001/6020#newcode1929
Line 1929: error.unreachableURL = request.url();
On 2009/10/16 07:10:41, darin wrote:
> shouldn't you set the other fields of WebURLError?

I don't think so.  unreachableURL is also unnecessary.
I'll add a comment.

http://codereview.chromium.org/271087/diff/9001/6016
File webkit/glue/webframeloaderclient_impl.cc (right):

http://codereview.chromium.org/271087/diff/9001/6016#newcode29
Line 29: #include "net/url_request/url_request.h"
On 2009/10/16 07:10:41, darin wrote:
> looks like you can delete this include

Done.

http://codereview.chromium.org/271087/diff/9001/6016#newcode1045
Line 1045: const {
On 2009/10/16 07:10:41, darin wrote:
> nit: google style says not to float the "const" like this.  instead you should
> line break after the "("

Done.

[darin (slow to review), 2009-10-16 07:58:46]

http://codereview.chromium.org/271087/diff/12001/13008
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/271087/diff/12001/13008#newcode1910
Line 1910: // FIXME: this appears to be used only by the context menu code to
determine
context menu code?  note: we don't use the standard WebCore context menus.  we
just intercept the context menu signal, and then we have the browser create and
show a context menu.

[tkent, 2009-10-16 08:03:52]

http://codereview.chromium.org/271087/diff/12001/13008
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/271087/diff/12001/13008#newcode1910
Line 1910: // FIXME: this appears to be used only by the context menu code to
determine
On 2009/10/16 07:58:46, darin wrote:
> context menu code?  note: we don't use the standard WebCore context menus.  we
> just intercept the context menu signal, and then we have the browser create
and
> show a context menu.

I just moved the comment in webframeloaderclient_impl.cc.
ok, I'll remove the comment.

[tkent, 2009-10-16 08:07:15]

I have updated the patch.

[darin (slow to review), 2009-10-16 15:44:03]

Looks great.  Just some minor nits:

http://codereview.chromium.org/271087/diff/10010/10012
File webkit/tools/test_shell/test_webview_delegate.cc (right):

http://codereview.chromium.org/271087/diff/10010/10012#newcode664
Line 664: return url.scheme() != "spaceballs";
nit: it is recommended to use the SchemeIs function instead for cases like this.

http://codereview.chromium.org/271087/diff/10010/10012#newcode670
Line 670: // A WebKit's layout test expects the following values.
nit: no "'s" on WebKit here

http://codereview.chromium.org/271087/diff/10010/10012#newcode680
Line 680: std::string domain = UTF16ToASCII(error.domain);
you could also use "error.domain.utf8()" here.  either is fine.

[tkent, 2009-10-19 02:26:36]

On 2009/10/16 15:44:03, darin wrote:
> Looks great.  Just some minor nits:

I fixed them and committed.  Thank you!