[Android] Upstream external_protocol_dialog_android.cc

[Android] Upstream external_protocol_dialog_android.cc

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

[newt (away), 2012-10-16 22:18:16]

PTAL.  Thanks!

[Ted C, 2012-10-16 22:58:15]

lgtm w/ comments addressed

http://codereview.chromium.org/11193014/diff/1/chrome/browser/android/tab_and...
File chrome/browser/android/tab_android.h (right):

http://codereview.chromium.org/11193014/diff/1/chrome/browser/android/tab_and...
chrome/browser/android/tab_android.h:70: // run the external protocol dialog.
is run the right word here?  is display/show better?

http://codereview.chromium.org/11193014/diff/1/chrome/browser/ui/android/exte...
File chrome/browser/ui/android/external_protocol_dialog_android.cc (right):

http://codereview.chromium.org/11193014/diff/1/chrome/browser/ui/android/exte...
chrome/browser/ui/android/external_protocol_dialog_android.cc:20: if (tab)
return tab->RunExternalProtocolDialog(url);
c++ style is to have this on the following line

[newt (away), 2012-10-16 23:19:42]

http://codereview.chromium.org/11193014/diff/1/chrome/browser/android/tab_and...
File chrome/browser/android/tab_android.h (right):

http://codereview.chromium.org/11193014/diff/1/chrome/browser/android/tab_and...
chrome/browser/android/tab_android.h:70: // run the external protocol dialog.
On 2012/10/16 22:58:15, Ted C wrote:
> is run the right word here?  is display/show better?

This is the function name used elsewhere, e.g. external_protocol_handler.h and
other platform-specific implementations.  Do you still think it should be
changed?

http://codereview.chromium.org/11193014/diff/1/chrome/browser/ui/android/exte...
File chrome/browser/ui/android/external_protocol_dialog_android.cc (right):

http://codereview.chromium.org/11193014/diff/1/chrome/browser/ui/android/exte...
chrome/browser/ui/android/external_protocol_dialog_android.cc:20: if (tab)
return tab->RunExternalProtocolDialog(url);
On 2012/10/16 22:58:15, Ted C wrote:
> c++ style is to have this on the following line

Done.

[Ted C, 2012-10-16 23:25:10]

http://codereview.chromium.org/11193014/diff/1/chrome/browser/android/tab_and...
File chrome/browser/android/tab_android.h (right):

http://codereview.chromium.org/11193014/diff/1/chrome/browser/android/tab_and...
chrome/browser/android/tab_android.h:70: // run the external protocol dialog.
On 2012/10/16 23:19:42, newt wrote:
> On 2012/10/16 22:58:15, Ted C wrote:
> > is run the right word here?  is display/show better?
> 
> This is the function name used elsewhere, e.g. external_protocol_handler.h and
> other platform-specific implementations.  Do you still think it should be
> changed?

Sigh...I guess it's better to be consistent.

http://codereview.chromium.org/11193014/diff/1/chrome/browser/android/tab_bas...
File chrome/browser/android/tab_base_android_impl.h (right):

http://codereview.chromium.org/11193014/diff/1/chrome/browser/android/tab_bas...
chrome/browser/android/tab_base_android_impl.h:64: // run the external protocol
dialog.
also, I would leave this comment out of the impl.h as that is a common construct
for OVERRIDE methods

[newt (away), 2012-10-16 23:29:50]

http://codereview.chromium.org/11193014/diff/1/chrome/browser/android/tab_bas...
File chrome/browser/android/tab_base_android_impl.h (right):

http://codereview.chromium.org/11193014/diff/1/chrome/browser/android/tab_bas...
chrome/browser/android/tab_base_android_impl.h:64: // run the external protocol
dialog.
On 2012/10/16 23:25:10, Ted C wrote:
> also, I would leave this comment out of the impl.h as that is a common
construct
> for OVERRIDE methods

Done.

[commit-bot: I haz the power, 2012-10-16 23:30:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/newt@chromium.org/11193014/7002

[commit-bot: I haz the power, 2012-10-17 11:19:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/newt@chromium.org/11193014/7002

[commit-bot: I haz the power, 2012-10-17 13:30:47]

Change committed as 162389