[Android] Chagne items shown in context menu for tel link

[Android] Chagne items shown in context menu for tel link

Current items (Copy link address, Copy link text) shown in the context
menu for tel tag link do not make sense.

Update the items shown in the context menu when long pressing a tel
link. The items include:
1. Call: acts the same as clicking on the link
2. Send message: navigates to the default message app to send a text
message for given telephone number.
3. Add ot contacts: navigates to Contacts and add the given phone number
to the contact.
4. Copy: copys the phone number.

BUG=394299
Review-Url: https://codereview.chromium.org/2619493006
Cr-Commit-Position: refs/heads/master@{#442728}
Committed: https://chromium.googlesource.com/chromium/src/+/17aab3f6517fd0ba74d8e5f7d9ec35440401d9e1

[ltian, 2017-01-06 23:51:52]

The CQ bit was checked by ltian@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-06 23:52:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-07 00:57:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-07 00:57:47]

Dry run: This issue passed the CQ dry run.

[ltian, 2017-01-07 01:00:33]

ltian@chromium.org changed reviewers:
+ jwd@chromium.org, tedchoc@chromium.org

[ltian, 2017-01-07 01:00:34]

tedchoc@chromium.org: can you take a look of my Java changes for this CL.

[ltian, 2017-01-07 01:02:59]

jwd@chromium.org: can you take a look of changes in histograms.xml?

Thanks!

[jwd, 2017-01-09 21:04:30]

histograms LGTM

[Ted C, 2017-01-09 21:45:34]

https://codereview.chromium.org/2619493006/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ContextMenuItemDelegate.java
(right):

https://codereview.chromium.org/2619493006/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ContextMenuItemDelegate.java:102:
* @return true if an activity is available to handle intent to call a phone
number.
For simple things like this, it is fine to drop either the first Returns line or
the @return line as they are redundant.

Also a tiny nit, but for boolean returns, I prefer:

@return [Whether] an activity is available to handle [an] intent to call a phone
number.

Whether implies a boolean statement so you don't need to call out true or false
explicitly.

Same below.

https://codereview.chromium.org/2619493006/diff/1/chrome/android/java/src/org...
File chrome/android/java/src/org/chromium/chrome/browser/util/UrlUtilities.java
(right):

https://codereview.chromium.org/2619493006/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/util/UrlUtilities.java:53:
if (uri != null && uri.startsWith(TEL_SCHEME)) {
return uri != null && uri.startsWith(TEL_SCHEME);

https://codereview.chromium.org/2619493006/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/util/UrlUtilities.java:59:
public static String getTelNumber(String uri) {
javadoc all non private methods.  I would also clarify that includes everything
after the scheme.   According to the RFC that is more stuff, so I wouldn't
specifically guarantee this is only the phone number.

[ltian, 2017-01-10 01:12:53]

The CQ bit was checked by ltian@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-10 01:13:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-10 03:16:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-10 03:16:19]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[ltian, 2017-01-10 04:45:06]

https://codereview.chromium.org/2619493006/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ContextMenuItemDelegate.java
(right):

https://codereview.chromium.org/2619493006/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ContextMenuItemDelegate.java:102:
* @return true if an activity is available to handle intent to call a phone
number.
On 2017/01/09 21:45:34, Ted C wrote:
> For simple things like this, it is fine to drop either the first Returns line
or
> the @return line as they are redundant.
> 
> Also a tiny nit, but for boolean returns, I prefer:
> 
> @return [Whether] an activity is available to handle [an] intent to call a
phone
> number.
> 
> Whether implies a boolean statement so you don't need to call out true or
false
> explicitly.
> 
> Same below.

Done.

https://codereview.chromium.org/2619493006/diff/1/chrome/android/java/src/org...
File chrome/android/java/src/org/chromium/chrome/browser/util/UrlUtilities.java
(right):

https://codereview.chromium.org/2619493006/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/util/UrlUtilities.java:53:
if (uri != null && uri.startsWith(TEL_SCHEME)) {
On 2017/01/09 21:45:34, Ted C wrote:
> return uri != null && uri.startsWith(TEL_SCHEME);

Done.

https://codereview.chromium.org/2619493006/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/util/UrlUtilities.java:59:
public static String getTelNumber(String uri) {
On 2017/01/09 21:45:34, Ted C wrote:
> javadoc all non private methods.  I would also clarify that includes
everything
> after the scheme.   According to the RFC that is more stuff, so I wouldn't
> specifically guarantee this is only the phone number.

Done.

[Ted C, 2017-01-10 17:22:24]

lgtm

https://codereview.chromium.org/2619493006/diff/20001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/util/UrlUtilities.java
(right):

https://codereview.chromium.org/2619493006/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/util/UrlUtilities.java:60: *
guaranteed.
nit, for the second line, align with "The string after..."

[ltian, 2017-01-10 18:58:59]

https://codereview.chromium.org/2619493006/diff/20001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/util/UrlUtilities.java
(right):

https://codereview.chromium.org/2619493006/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/util/UrlUtilities.java:60: *
guaranteed.
On 2017/01/10 17:22:24, Ted C wrote:
> nit, for the second line, align with "The string after..."

Done.

[ltian, 2017-01-10 18:59:08]

The CQ bit was checked by ltian@chromium.org

[ltian, 2017-01-10 18:59:08]

The patchset sent to the CQ was uploaded after l-g-t-m from jwd@chromium.org,
tedchoc@chromium.org
Link to the patchset: https://codereview.chromium.org/2619493006/#ps40001
(title: "Update based on Ted's comment.")

[commit-bot: I haz the power, 2017-01-10 18:59:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-10 19:02:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-10 19:02:58]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[ltian, 2017-01-10 22:00:09]

The CQ bit was checked by ltian@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-10 22:00:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-10 23:29:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-10 23:29:54]

Dry run: This issue passed the CQ dry run.

[ltian, 2017-01-10 23:30:14]

The CQ bit was checked by ltian@chromium.org

[ltian, 2017-01-10 23:30:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
tedchoc@chromium.org, jwd@chromium.org
Link to the patchset: https://codereview.chromium.org/2619493006/#ps60001
(title: "Solve conflicts of commit.")

[commit-bot: I haz the power, 2017-01-10 23:30:52]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-10 23:37:09]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1484091014951420,
"parent_rev": "5846b9cf0505c478a930d79c3b6c96a264ee4e7d", "commit_rev":
"17aab3f6517fd0ba74d8e5f7d9ec35440401d9e1"}

[commit-bot: I haz the power, 2017-01-10 23:37:43]

Description was changed from

==========
[Android] Chagne items shown in context menu for tel link

Current items (Copy link address, Copy link text) shown in the context
menu for tel tag link do not make sense.

Update the items shown in the context menu when long pressing a tel
link. The items include:
1. Call: acts the same as clicking on the link
2. Send message: navigates to the default message app to send a text
message for given telephone number.
3. Add ot contacts: navigates to Contacts and add the given phone number
to the contact.
4. Copy: copys the phone number.

BUG=394299
==========

to

==========
[Android] Chagne items shown in context menu for tel link

Current items (Copy link address, Copy link text) shown in the context
menu for tel tag link do not make sense.

Update the items shown in the context menu when long pressing a tel
link. The items include:
1. Call: acts the same as clicking on the link
2. Send message: navigates to the default message app to send a text
message for given telephone number.
3. Add ot contacts: navigates to Contacts and add the given phone number
to the contact.
4. Copy: copys the phone number.

BUG=394299

Review-Url: https://codereview.chromium.org/2619493006
Cr-Commit-Position: refs/heads/master@{#442728}
Committed:
https://chromium.googlesource.com/chromium/src/+/17aab3f6517fd0ba74d8e5f7d9ec...
==========

[commit-bot: I haz the power, 2017-01-10 23:37:45]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/17aab3f6517fd0ba74d8e5f7d9ec...