[Android] Use resource throttle instead of HandleExternalProtocol.

[Android] Use resource throttle instead of HandleExternalProtocol.

Chrome on Android uses a throttle-based mechanism to intercept links
so that the user may choose to run an Android application instead of
loading the link in the browser. This mechanism works for both
internal (e.g. http:, https:) as well as external protocols (e.g.
tel:, geo:).

Instead of using the HandleExternalProtocol path way to handle
external protocols, it is better to have both internal and external
protocols overridden through the same mechanism. This has the
added benefit that an error page can be shown in Chrome on Android
if there is no external handler available for the given protocol, which
is not easily done using the HandleExternalProtocol mechanism.

BUG=156044
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=164717

[joth, 2012-10-24 18:47:55]

https://codereview.chromium.org/11265013/diff/1/content/browser/renderer_host...
File content/browser/renderer_host/resource_dispatcher_host_impl.cc (right):

https://codereview.chromium.org/11265013/diff/1/content/browser/renderer_host...
content/browser/renderer_host/resource_dispatcher_host_impl.cc:663: return
false;
Did you mean this? you've changed delegate_->HandleExternalProtocol to return
bool instead -- shouldn't we just be returning that result below, no need for
ifdef here?

https://codereview.chromium.org/11265013/diff/1/content/public/browser/resour...
File content/public/browser/resource_dispatcher_host_delegate.h (right):

https://codereview.chromium.org/11265013/diff/1/content/public/browser/resour...
content/public/browser/resource_dispatcher_host_delegate.h:99: // handled.
maybe expand this to note true only implies an attempt was made to handle the
URL e.g. by starting some app. it does not guarantee that app successfully
handled it.

[mkosiba (inactive), 2012-10-25 09:02:11]

https://codereview.chromium.org/11265013/diff/1/chrome/browser/ui/android/ext...
File chrome/browser/ui/android/external_protocol_dialog_android.cc (right):

https://codereview.chromium.org/11265013/diff/1/chrome/browser/ui/android/ext...
chrome/browser/ui/android/external_protocol_dialog_android.cc:19: // protocols,
so we don't need to do anything here.
NOTREACHED?

https://codereview.chromium.org/11265013/diff/1/content/browser/renderer_host...
File content/browser/renderer_host/resource_dispatcher_host_impl.cc (right):

https://codereview.chromium.org/11265013/diff/1/content/browser/renderer_host...
content/browser/renderer_host/resource_dispatcher_host_impl.cc:663: return
false;
On 2012/10/24 18:47:55, joth wrote:
> Did you mean this? you've changed delegate_->HandleExternalProtocol to return
> bool instead -- shouldn't we just be returning that result below, no need for
> ifdef here?

Do what Joth says ;)

[John Knottenbelt, 2012-10-25 16:17:27]

Thanks for reviewing, changes made.

https://codereview.chromium.org/11265013/diff/1/chrome/browser/ui/android/ext...
File chrome/browser/ui/android/external_protocol_dialog_android.cc (right):

https://codereview.chromium.org/11265013/diff/1/chrome/browser/ui/android/ext...
chrome/browser/ui/android/external_protocol_dialog_android.cc:19: // protocols,
so we don't need to do anything here.
On 2012/10/25 09:02:12, Martin Kosiba wrote:
> NOTREACHED?

Done.

https://codereview.chromium.org/11265013/diff/1/content/browser/renderer_host...
File content/browser/renderer_host/resource_dispatcher_host_impl.cc (right):

https://codereview.chromium.org/11265013/diff/1/content/browser/renderer_host...
content/browser/renderer_host/resource_dispatcher_host_impl.cc:663: return
false;
Yes, I agree. Sorry, this was accidentally left in from an experiment. 

On 2012/10/24 18:47:55, joth wrote:
> Did you mean this? you've changed delegate_->HandleExternalProtocol to return
> bool inste
ad -- shouldn't we just be returning that result below, no need for
> ifdef here?

https://codereview.chromium.org/11265013/diff/1/content/public/browser/resour...
File content/public/browser/resource_dispatcher_host_delegate.h (right):

https://codereview.chromium.org/11265013/diff/1/content/public/browser/resour...
content/public/browser/resource_dispatcher_host_delegate.h:99: // handled.
On 2012/10/24 18:47:55, joth wrote:
> maybe expand this to note true only implies an attempt was made to handle the
> URL e.g. by starting some app. it does not guarantee that app successfully
> handled it.

Done.

[joth, 2012-10-25 16:23:19]

lgtm

[John Knottenbelt, 2012-10-26 14:19:09]

Adding OWNERS, please review:

yfriedman: chrome/browser/ui/android
joi: content/public/browser
darin: chrome, content/browser/renderer_host

[John Knottenbelt, 2012-10-29 11:08:56]

Oops, I forgot to actually add joi, yfriedman and darin to the reviewers list.
Please review:

yfriedman: chrome/browser/ui/android
joi: content/public/browser
darin: chrome, content/browser/renderer_host

[Jói, 2012-10-29 11:12:46]

LGTM for content/public.

[darin (slow to review), 2012-10-29 14:17:22]

LGTM for content/ changes.

[Yaron, 2012-10-29 17:39:24]

lgtm

[commit-bot: I haz the power, 2012-10-29 17:44:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jknotten@chromium.org/11265013/5012

[commit-bot: I haz the power, 2012-10-29 19:43:34]

Change committed as 164717