Change external link handling policy for CCTs

Change external link handling policy for CCTs

- Changes the url handling so that initial urls and the associated
  redirect chain is also checked for default handlers inside CCT.
- This behavior is tied to a new extra that is added.
- This behavior is enabled by default as a feature, so that it can be
  disabled if necessary

- Followup: The extra should be moved to CustomTabsIntent in the support
  library and published as a new API. Tests that check for new corner
  cases will be added to CustomTabsExternalNavigationTest

BUG=647474
Committed: https://crrev.com/97a926b6fcc71e943f92b6bd7ff9ea2374ec49db
Cr-Commit-Position: refs/heads/master@{#423610}

[Yusuf, 2016-10-05 18:51:49]

yusufo@chromium.org changed reviewers:
+ ianwen@chromium.org, tedchoc@chromium.org

[Ian Wen, 2016-10-05 20:16:41]

https://chromiumcodereview.appspot.com/2387293005/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabIntentDataProvider.java
(right):

https://chromiumcodereview.appspot.com/2387293005/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabIntentDataProvider.java:64:
/** Signals the custom tab to favor sending any urls to external handler apps if
possible. */
I think at least in the comment we should emphasize that this extra only affects
the handling for the very first URL.

[Yusuf, 2016-10-05 23:32:08]

https://chromiumcodereview.appspot.com/2387293005/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabIntentDataProvider.java
(right):

https://chromiumcodereview.appspot.com/2387293005/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabIntentDataProvider.java:64:
/** Signals the custom tab to favor sending any urls to external handler apps if
possible. */
On 2016/10/05 20:16:40, Ian Wen wrote:
> I think at least in the comment we should emphasize that this extra only
affects
> the handling for the very first URL.

Done.

[Ted C, 2016-10-05 23:51:38]

lgtm ... granted, no sure what the best thing is for making that logic easier to
read... i'll defer to you

https://codereview.chromium.org/2387293005/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/tab/TabRedirectHandler.java
(right):

https://codereview.chromium.org/2387293005/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/tab/TabRedirectHandler.java:76:
boolean sendToExternalApps = IntentUtils.safeGetBooleanExtra(
This is getting harder to read, and I wonder if something like this is easier to
grok:

mIsCustomTabIntent = ChromeLauncherActivity.isCustomTabIntent(intent);
boolean checkIsToChrome = true;
// Comment about custom tabs doing things differently.
if (mIsCustomTabIntent) {
    boolean sendToExternalApps = IntentUtils.safeGetBooleanExtra(...);
    checkIsToChrome = sendToExternalApps && ChromeFeatureList...;
}

if (checkIsToChrome) mIsInitialIntentHeadingToChrome =
isIntentToChrome(mContext, intent);


separate function:

private static boolean isIntentToChrome(Context context, Intent intent) {
    return TextUtils....
}

Thoughts?

https://codereview.chromium.org/2387293005/diff/20001/chrome/browser/android/...
File chrome/browser/android/chrome_feature_list.cc (right):

https://codereview.chromium.org/2387293005/diff/20001/chrome/browser/android/...
chrome/browser/android/chrome_feature_list.cc:108:
base::FEATURE_ENABLED_BY_DEFAULT};
this would need to be aligned with the " above (or wrap the lines)

[Yusuf, 2016-10-06 18:14:08]

The CQ bit was checked by yusufo@chromium.org

[Yusuf, 2016-10-06 18:14:08]

The patchset sent to the CQ was uploaded after l-g-t-m from tedchoc@chromium.org
Link to the patchset: https://codereview.chromium.org/2387293005/#ps40001
(title: "Ted's styling suggestions")

[commit-bot: I haz the power, 2016-10-06 18:14:40]

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, 2016-10-06 18:53:47]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-10-06 18:55:09]

Description was changed from

==========
Change external link handling policy for CCTs

- Changes the url handling so that initial urls and the associated
  redirect chain is also checked for default handlers inside CCT.
- This behavior is tied to a new extra that is added.
- This behavior is enabled by default as a feature, so that it can be
  disabled if necessary

- Followup: The extra should be moved to CustomTabsIntent in the support
  library and published as a new API. Tests that check for new corner
  cases will be added to CustomTabsExternalNavigationTest

BUG=647474
==========

to

==========
Change external link handling policy for CCTs

- Changes the url handling so that initial urls and the associated
  redirect chain is also checked for default handlers inside CCT.
- This behavior is tied to a new extra that is added.
- This behavior is enabled by default as a feature, so that it can be
  disabled if necessary

- Followup: The extra should be moved to CustomTabsIntent in the support
  library and published as a new API. Tests that check for new corner
  cases will be added to CustomTabsExternalNavigationTest

BUG=647474

Committed: https://crrev.com/97a926b6fcc71e943f92b6bd7ff9ea2374ec49db
Cr-Commit-Position: refs/heads/master@{#423610}
==========

[commit-bot: I haz the power, 2016-10-06 18:55:09]

Patchset 3 (id:??) landed as
https://crrev.com/97a926b6fcc71e943f92b6bd7ff9ea2374ec49db
Cr-Commit-Position: refs/heads/master@{#423610}