Fixes redirects to external apps when navigating from PWA.

Fixes the redirect to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name (using
component name instead) and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.

With this patch the redirect works, however it takes a long time for CCT to
consult ExternalNavigationHandler. This results in suboptimal UX. Same redirect
in ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank links in Clank.

BUG=740402
Review-Url: https://codereview.chromium.org/2969143002
Cr-Original-Original-Commit-Position: refs/heads/master@{#485831}
Committed: https://chromium.googlesource.com/chromium/src/+/727468b41b2fe48bf73983ab0c3aa2632b47d89e
Review-Url: https://codereview.chromium.org/2969143002
Cr-Original-Commit-Position: refs/heads/master@{#486275}
Committed: https://chromium.googlesource.com/chromium/src/+/4dbcecfb630ac1a2dba1910f0e429b82bf3a0b5f
Review-Url: https://codereview.chromium.org/2969143002
Cr-Commit-Position: refs/heads/master@{#488228}
Committed: https://chromium.googlesource.com/chromium/src/+/471df3926c09fda0af15d9157eb29bd326b0f9fc

[piotrs, 2017-07-05 01:54:32]

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

[commit-bot: I haz the power, 2017-07-05 01:54:40]

Dry run: CQ is trying da patch.

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

[piotrs, 2017-07-05 03:32:28]

Description was changed from

==========
Fixes redirects to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER extra on the intent.

A suboptimal part is that CCT->External redirect takes noticeable amount of
time, while for ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank link.

BUG=709889
==========

to

==========
Fixes the redirect to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name (using
component name instead) and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.

A suboptimal part is that CCT->External redirect takes noticeable amount of
time, while for ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank links in Clank.

BUG=709889
==========

[piotrs, 2017-07-05 03:39:36]

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

[commit-bot: I haz the power, 2017-07-05 03:39:39]

Dry run: CQ is trying da patch.

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

[piotrs, 2017-07-05 03:40:37]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2017-07-05 04:35:15]

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

[commit-bot: I haz the power, 2017-07-05 04:35:16]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[piotrs, 2017-07-05 05:23:42]

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

[commit-bot: I haz the power, 2017-07-05 05:23:58]

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-07-05 07:17:17]

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

[commit-bot: I haz the power, 2017-07-05 07:17:18]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[piotrs, 2017-07-05 23:15:01]

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

[commit-bot: I haz the power, 2017-07-05 23:15:19]

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-07-06 01:11:52]

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

[commit-bot: I haz the power, 2017-07-06 01:11:53]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[piotrs, 2017-07-06 01:47:58]

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

[commit-bot: I haz the power, 2017-07-06 01:48:26]

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-07-06 04:00:14]

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

[commit-bot: I haz the power, 2017-07-06 04:00:15]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[piotrs, 2017-07-07 04:56:07]

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

[commit-bot: I haz the power, 2017-07-07 04:56:20]

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-07-07 05:39:52]

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

[commit-bot: I haz the power, 2017-07-07 05:39:52]

Dry run: This issue passed the CQ dry run.

[piotrs, 2017-07-07 05:41:39]

Patchset #1 (id:20001) has been deleted

[piotrs, 2017-07-07 05:41:46]

Patchset #3 (id:80001) has been deleted

[piotrs, 2017-07-07 05:41:53]

Patchset #2 (id:60001) has been deleted

[piotrs, 2017-07-07 05:42:01]

Patchset #1 (id:40001) has been deleted

[piotrs, 2017-07-07 05:45:19]

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

[piotrs, 2017-07-07 05:45:29]

Patchset #1 (id:100001) has been deleted

[commit-bot: I haz the power, 2017-07-07 05:45:35]

Dry run: CQ is trying da patch.

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

[piotrs, 2017-07-07 05:48:00]

piotrs@chromium.org changed reviewers:
+ dominickn@chromium.org, yusufo@chromium.org

[piotrs, 2017-07-07 05:48:01]

Hi folks, a small fix that allows the redirect to native apps for PWAs. Should
be fairly non-controversial.

[dominickn, 2017-07-07 05:56:04]

This looks fine to me % nits, but I'll defer to Yusuf since this is mostly CCT
related.

https://codereview.chromium.org/2969143002/diff/120001/chrome/android/javates...
File
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TopActivityListener.java
(right):

https://codereview.chromium.org/2969143002/diff/120001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TopActivityListener.java:77:
> TimeUnit.SECONDS.toMillis(1);
Nit: possibly put the 1 as a static class level constant for clarity.

https://codereview.chromium.org/2969143002/diff/120001/chrome/android/javates...
File
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappNavigationTest.java
(right):

https://codereview.chromium.org/2969143002/diff/120001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappNavigationTest.java:5:
package org.chromium.chrome.browser.webapps;
Nit: put this newline back in?

[commit-bot: I haz the power, 2017-07-07 06:35:07]

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

[commit-bot: I haz the power, 2017-07-07 06:35:08]

Dry run: This issue passed the CQ dry run.

[pkotwicz, 2017-07-07 14:05:05]

Piotr, can you please file a separate bug for this CL? A pet peeve of mine is
when all of the CLs for a given feature are written against a single uber bug
In your CL description (or in the new crbug) can you describe what the current
behavior is without your CL?

Thanks for working on this CL. It looks super useful. (I can't do a review
because I am not a navigation expert)

[Yusuf, 2017-07-07 20:50:03]

lgtm

https://codereview.chromium.org/2969143002/diff/120001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappTabDelegate.java
(right):

https://codereview.chromium.org/2969143002/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappTabDelegate.java:37:
intent.putExtra(CustomTabIntentDataProvider.EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER,
true);
Good investigation finding out about this!

I am still a bit confused about your CL description. So the flow was working
even without this but was longer with more transitions? If that is the case we
are hitting a code path that I am not aware of. Since basically AFAIK if you
need this extra, without it things should simply wouldn't work. (Maybe it is
because we are still in Chrome and our internal intent handling eventually takes
care of this but takes more hops to do so?)

[piotrs, 2017-07-09 23:31:23]

Description was changed from

==========
Fixes the redirect to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name (using
component name instead) and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.

A suboptimal part is that CCT->External redirect takes noticeable amount of
time, while for ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank links in Clank.

BUG=709889
==========

to

==========
Fixes the redirect to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name (using
component name instead) and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.

With this patch the redirect works, however it takes a long time for CCT to
consult ExternalNavigationHandler. This results in suboptimal UX. Same redirect
in ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank links in Clank.

BUG=709889
==========

[piotrs, 2017-07-09 23:36:37]

Description was changed from

==========
Fixes the redirect to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name (using
component name instead) and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.

With this patch the redirect works, however it takes a long time for CCT to
consult ExternalNavigationHandler. This results in suboptimal UX. Same redirect
in ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank links in Clank.

BUG=709889
==========

to

==========
Fixes the redirect to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name (using
component name instead) and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.

With this patch the redirect works, however it takes a long time for CCT to
consult ExternalNavigationHandler. This results in suboptimal UX. Same redirect
in ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank links in Clank.

BUG=740402
==========

[piotrs, 2017-07-09 23:42:45]

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

[commit-bot: I haz the power, 2017-07-09 23:42:54]

Dry run: CQ is trying da patch.

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

[piotrs, 2017-07-09 23:43:38]

Thanks all.

@pkotwicz: Create a separate bug for this, thanks for feedback and sorry for
being lazy ;)

https://codereview.chromium.org/2969143002/diff/120001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappTabDelegate.java
(right):

https://codereview.chromium.org/2969143002/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappTabDelegate.java:37:
intent.putExtra(CustomTabIntentDataProvider.EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER,
true);
My patch description wasn't clear enough, I updated it.

TL;DR
Previously: Redirecting to external apps didn't work.
Now: Redirecting to external apps works, but CCT shows up for a noticeable
amount of time.

I will be looking into how to make ExternalNavigationHandler run sooner in CCT,
same as it does in ChromeTabbedActivity. Any help here would be appreciated BTW.

https://codereview.chromium.org/2969143002/diff/120001/chrome/android/javates...
File
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TopActivityListener.java
(right):

https://codereview.chromium.org/2969143002/diff/120001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TopActivityListener.java:77:
> TimeUnit.SECONDS.toMillis(1);
On 2017/07/07 05:56:04, dominickn wrote:
> Nit: possibly put the 1 as a static class level constant for clarity.

Done.

[commit-bot: I haz the power, 2017-07-10 00:29:04]

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

[commit-bot: I haz the power, 2017-07-10 00:29:05]

Dry run: This issue passed the CQ dry run.

[piotrs, 2017-07-10 00:43:14]

The CQ bit was checked by piotrs@chromium.org

[piotrs, 2017-07-10 00:43:15]

The patchset sent to the CQ was uploaded after l-g-t-m from yusufo@chromium.org
Link to the patchset: https://codereview.chromium.org/2969143002/#ps160001
(title: "Merge")

[commit-bot: I haz the power, 2017-07-10 00:43:30]

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-07-10 00:54:53]

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

[commit-bot: I haz the power, 2017-07-10 00:54:54]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[piotrs, 2017-07-10 01:17:36]

piotrs@chromium.org changed reviewers:
+ tedchoc@chromium.org

[piotrs, 2017-07-10 01:17:37]

Hi all,

Dominick, can you consider LGTM for webapps/ directory.
Ted, can you please take a look at the change in TabDelegate.java?

Thanks to you both!

[dominickn, 2017-07-10 01:19:44]

webapps lgtm

[Ted C, 2017-07-10 18:10:15]

https://codereview.chromium.org/2969143002/diff/160001/chrome/android/javates...
File
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TopActivityListener.java
(right):

https://codereview.chromium.org/2969143002/diff/160001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TopActivityListener.java:66:
return mMostRecentActivity != null
why not just use ApplicationStatus.getLastTrackedFocusedActivity() ?

https://codereview.chromium.org/2969143002/diff/160001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TopActivityListener.java:77:
return mMostRecentActivity == null
Just use ApplicationStatus.getStateForApplication == HAS_STOPPED_ACTIVITIES ||
HAS_DESTROYED_ACTIVITIES

https://codereview.chromium.org/2969143002/diff/160001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TopActivityListener.java:78:
&& (System.currentTimeMillis() - mTimeWhenSetToNullMillis)
this delay is very much specific to your test.  Instead, don't assertEquals in
your test about your expectation but use CriteriaHelper to check there for you.

[piotrs, 2017-07-10 23:40:05]

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

[commit-bot: I haz the power, 2017-07-10 23:40:34]

Dry run: CQ is trying da patch.

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

[piotrs, 2017-07-10 23:40:54]

Thank you Ted!

https://codereview.chromium.org/2969143002/diff/160001/chrome/android/javates...
File
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TopActivityListener.java
(right):

https://codereview.chromium.org/2969143002/diff/160001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TopActivityListener.java:66:
return mMostRecentActivity != null
An Oversight. I deleted this whole class.

https://codereview.chromium.org/2969143002/diff/160001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TopActivityListener.java:77:
return mMostRecentActivity == null
An Oversight. I deleted this whole class.

https://codereview.chromium.org/2969143002/diff/160001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TopActivityListener.java:78:
&& (System.currentTimeMillis() - mTimeWhenSetToNullMillis)
I don't quite follow. My intention was to ensure that we are really outside of
Chrome instead of catching a glimpse of being in between activities (previous
got stopped while the new one isn't started yet). Therefore the time check,
which I don't see as specific to my test.

But I'm probably a little paranoid. Removed that now.

[Ted C, 2017-07-11 00:03:05]

lgtm

https://codereview.chromium.org/2969143002/diff/180001/chrome/android/javates...
File
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappNavigationTest.java
(right):

https://codereview.chromium.org/2969143002/diff/180001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappNavigationTest.java:290:
return ApplicationStatus.getStateForApplication() == HAS_PAUSED_ACTIVITIES
Ahh...I was going to say you shouldn't have paused, but I see you also accept
the intent picker.  In that case, Chrome would be visible and paused is the
correct state.  Thanks for good naming!

[Ted C, 2017-07-11 00:05:39]

https://codereview.chromium.org/2969143002/diff/160001/chrome/android/javates...
File
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TopActivityListener.java
(right):

https://codereview.chromium.org/2969143002/diff/160001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TopActivityListener.java:78:
&& (System.currentTimeMillis() - mTimeWhenSetToNullMillis)
On 2017/07/10 23:40:54, piotrs wrote:
> I don't quite follow. My intention was to ensure that we are really outside of
> Chrome instead of catching a glimpse of being in between activities (previous
> got stopped while the new one isn't started yet). Therefore the time check,
> which I don't see as specific to my test.
> 
> But I'm probably a little paranoid. Removed that now.

In general, any explicit time is likely to give you grief in Android land.  What
runs in 10s of milliseconds on a high end device could take 100s on the lowest
end.  Whenever I see these sorts of things, I push hard towards CriteriaHelper
(which has scaled timeouts based on bot configuration) and is a bit more
flexible.  Removing is good though :-P.

You are right that there could be a glimpse between activities, but that should
be purely with what is rendered to the screen and not what Chrome's awareness of
it's state is.

Just clarifying "hopefully".

[piotrs, 2017-07-11 00:33:08]

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

[commit-bot: I haz the power, 2017-07-11 00:33:25]

Dry run: CQ is trying da patch.

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

[piotrs, 2017-07-11 00:34:57]

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

[commit-bot: I haz the power, 2017-07-11 00:35:03]

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-07-11 03:02:27]

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

[commit-bot: I haz the power, 2017-07-11 03:02:27]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[piotrs, 2017-07-11 04:49:21]

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

[commit-bot: I haz the power, 2017-07-11 04:49: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-07-11 06:31:28]

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

[commit-bot: I haz the power, 2017-07-11 06:31:29]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[piotrs, 2017-07-11 07:09:30]

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

[commit-bot: I haz the power, 2017-07-11 07:09:44]

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-07-11 07:53:46]

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

[commit-bot: I haz the power, 2017-07-11 07:53:47]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[piotrs, 2017-07-11 23:01:26]

Merge

[piotrs, 2017-07-11 23:01:28]

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

[commit-bot: I haz the power, 2017-07-11 23:02:05]

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-07-12 02:49:44]

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

[commit-bot: I haz the power, 2017-07-12 02:49:46]

Dry run: This issue passed the CQ dry run.

[piotrs, 2017-07-12 03:19:28]

The CQ bit was checked by piotrs@chromium.org

[piotrs, 2017-07-12 03:19:28]

The patchset sent to the CQ was uploaded after l-g-t-m from
dominickn@chromium.org, yusufo@chromium.org, tedchoc@chromium.org
Link to the patchset: https://codereview.chromium.org/2969143002/#ps280001
(title: "Merge")

[commit-bot: I haz the power, 2017-07-12 03:19:37]

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-07-12 04:06:13]

CQ is committing da patch.

Bot data: {"patchset_id": 280001, "attempt_start_ts": 1499829568591600,
"parent_rev": "90dc55dea70e9b4ad2f53d1da06719461d27efc4", "commit_rev":
"15e89a4b85d44c4c1723e6e716702de5e8cce767"}

[commit-bot: I haz the power, 2017-07-12 04:06:44]

CQ is committing da patch.

Bot data: {"patchset_id": 280001, "attempt_start_ts": 1499829568591600,
"parent_rev": "28132e66abebde370e168fff9ff2fa157c979d55", "commit_rev":
"727468b41b2fe48bf73983ab0c3aa2632b47d89e"}

[commit-bot: I haz the power, 2017-07-12 04:07:02]

Description was changed from

==========
Fixes the redirect to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name (using
component name instead) and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.

With this patch the redirect works, however it takes a long time for CCT to
consult ExternalNavigationHandler. This results in suboptimal UX. Same redirect
in ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank links in Clank.

BUG=740402
==========

to

==========
Fixes the redirect to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name (using
component name instead) and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.

With this patch the redirect works, however it takes a long time for CCT to
consult ExternalNavigationHandler. This results in suboptimal UX. Same redirect
in ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank links in Clank.

BUG=740402

Review-Url: https://codereview.chromium.org/2969143002
Cr-Commit-Position: refs/heads/master@{#485831}
Committed:
https://chromium.googlesource.com/chromium/src/+/727468b41b2fe48bf73983ab0c3a...
==========

[commit-bot: I haz the power, 2017-07-12 04:07:05]

Committed patchset #9 (id:280001) as
https://chromium.googlesource.com/chromium/src/+/727468b41b2fe48bf73983ab0c3a...

[aberent, 2017-07-12 10:03:10]

A revert of this CL (patchset #9 id:280001) has been created in
https://codereview.chromium.org/2975883003/ by aberent@chromium.org.

The reason for reverting is: Test failures from new test
WebappNavigationTest#testNewTabLinkToExternalApp on hromium.android/Marshmallow
64 bit Tester.

BUG=741568.

[piotrs, 2017-07-12 23:27:30]

Description was changed from

==========
Fixes the redirect to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name (using
component name instead) and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.

With this patch the redirect works, however it takes a long time for CCT to
consult ExternalNavigationHandler. This results in suboptimal UX. Same redirect
in ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank links in Clank.

BUG=740402

Review-Url: https://codereview.chromium.org/2969143002
Cr-Commit-Position: refs/heads/master@{#485831}
Committed:
https://chromium.googlesource.com/chromium/src/+/727468b41b2fe48bf73983ab0c3a...
==========

to

==========
Fixes the redirect to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name (using
component name instead) and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.

With this patch the redirect works, however it takes a long time for CCT to
consult ExternalNavigationHandler. This results in suboptimal UX. Same redirect
in ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank links in Clank.

BUG=740402

Review-Url: https://codereview.chromium.org/2969143002
Cr-Commit-Position: refs/heads/master@{#485831}
Committed:
https://chromium.googlesource.com/chromium/src/+/727468b41b2fe48bf73983ab0c3a...
==========

[piotrs, 2017-07-13 00:37:49]

Patchset #11 (id:320001) has been deleted

[piotrs, 2017-07-13 00:40:40]

I cannot reproduce the waterfall test failure on a local Marshmallow N5X. In a
new patchset I increased the timeout and added logging to have more insight into
what happened on waterfall in case it fails again. If increased timeout fixes
the issue I will clean up the logging in a follow up patch.

[piotrs, 2017-07-13 00:41:04]

The CQ bit was checked by piotrs@chromium.org

[piotrs, 2017-07-13 00:41:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
dominickn@chromium.org, tedchoc@chromium.org, yusufo@chromium.org
Link to the patchset: https://codereview.chromium.org/2969143002/#ps300001
(title: "Increasing the timeut and adding logging to figure out the waterfall
errors")

[commit-bot: I haz the power, 2017-07-13 00:41:25]

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-07-13 03:18:00]

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

[commit-bot: I haz the power, 2017-07-13 03:18:02]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[piotrs, 2017-07-13 04:18:06]

The CQ bit was checked by piotrs@chromium.org

[commit-bot: I haz the power, 2017-07-13 04:18:19]

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-07-13 05:16:34]

CQ is committing da patch.

Bot data: {"patchset_id": 300001, "attempt_start_ts": 1499919486744530,
"parent_rev": "184a75bcf8dda55ceef7f6df67154ce822c7cc5d", "commit_rev":
"4dbcecfb630ac1a2dba1910f0e429b82bf3a0b5f"}

[commit-bot: I haz the power, 2017-07-13 05:16:49]

Description was changed from

==========
Fixes the redirect to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name (using
component name instead) and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.

With this patch the redirect works, however it takes a long time for CCT to
consult ExternalNavigationHandler. This results in suboptimal UX. Same redirect
in ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank links in Clank.

BUG=740402

Review-Url: https://codereview.chromium.org/2969143002
Cr-Commit-Position: refs/heads/master@{#485831}
Committed:
https://chromium.googlesource.com/chromium/src/+/727468b41b2fe48bf73983ab0c3a...
==========

to

==========
Fixes the redirect to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name (using
component name instead) and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.

With this patch the redirect works, however it takes a long time for CCT to
consult ExternalNavigationHandler. This results in suboptimal UX. Same redirect
in ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank links in Clank.

BUG=740402

Review-Url: https://codereview.chromium.org/2969143002
Cr-Original-Commit-Position: refs/heads/master@{#485831}
Committed:
https://chromium.googlesource.com/chromium/src/+/727468b41b2fe48bf73983ab0c3a...
Review-Url: https://codereview.chromium.org/2969143002
Cr-Commit-Position: refs/heads/master@{#486275}
Committed:
https://chromium.googlesource.com/chromium/src/+/4dbcecfb630ac1a2dba1910f0e42...
==========

[commit-bot: I haz the power, 2017-07-13 05:16:50]

Committed patchset #10 (id:300001) as
https://chromium.googlesource.com/chromium/src/+/4dbcecfb630ac1a2dba1910f0e42...

[aberent, 2017-07-13 10:17:42]

A revert of this CL (patchset #10 id:300001) has been created in
https://codereview.chromium.org/2977023002/ by aberent@chromium.org.

The reason for reverting is: Still failing on Marshmallow 64 bit Tester. See,
for example
https://uberchromegw.corp.google.com/i/chromium.android/builders/Marshmallow%...

BUG=741568.

[piotrs, 2017-07-14 05:39:57]

Description was changed from

==========
Fixes the redirect to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name (using
component name instead) and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.

With this patch the redirect works, however it takes a long time for CCT to
consult ExternalNavigationHandler. This results in suboptimal UX. Same redirect
in ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank links in Clank.

BUG=740402

Review-Url: https://codereview.chromium.org/2969143002
Cr-Original-Commit-Position: refs/heads/master@{#485831}
Committed:
https://chromium.googlesource.com/chromium/src/+/727468b41b2fe48bf73983ab0c3a...
Review-Url: https://codereview.chromium.org/2969143002
Cr-Commit-Position: refs/heads/master@{#486275}
Committed:
https://chromium.googlesource.com/chromium/src/+/4dbcecfb630ac1a2dba1910f0e42...
==========

to

==========
Fixes the redirect to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name (using
component name instead) and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.

With this patch the redirect works, however it takes a long time for CCT to
consult ExternalNavigationHandler. This results in suboptimal UX. Same redirect
in ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank links in Clank.

BUG=740402

Review-Url: https://codereview.chromium.org/2969143002
Cr-Original-Commit-Position: refs/heads/master@{#485831}
Committed:
https://chromium.googlesource.com/chromium/src/+/727468b41b2fe48bf73983ab0c3a...
Review-Url: https://codereview.chromium.org/2969143002
Cr-Commit-Position: refs/heads/master@{#486275}
Committed:
https://chromium.googlesource.com/chromium/src/+/4dbcecfb630ac1a2dba1910f0e42...
==========

[piotrs, 2017-07-18 00:53:31]

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

[commit-bot: I haz the power, 2017-07-18 00:53:44]

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-07-18 02:28:05]

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

[commit-bot: I haz the power, 2017-07-18 02:28:06]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[piotrs, 2017-07-18 03:03:26]

Description was changed from

==========
Fixes the redirect to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name (using
component name instead) and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.

With this patch the redirect works, however it takes a long time for CCT to
consult ExternalNavigationHandler. This results in suboptimal UX. Same redirect
in ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank links in Clank.

BUG=740402

Review-Url: https://codereview.chromium.org/2969143002
Cr-Original-Commit-Position: refs/heads/master@{#485831}
Committed:
https://chromium.googlesource.com/chromium/src/+/727468b41b2fe48bf73983ab0c3a...
Review-Url: https://codereview.chromium.org/2969143002
Cr-Commit-Position: refs/heads/master@{#486275}
Committed:
https://chromium.googlesource.com/chromium/src/+/4dbcecfb630ac1a2dba1910f0e42...
==========

to

==========
Fixes the redirect to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name (using
component name instead) and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.
Additionally a small change to ExternalNavigationHandler is
required for this change take effect with PlzNavigate turned
on. It only forces NO_REDIRECT on incoming chrome intents if
intent is explicitly directed at Chrome, otherwise a redirect
is allowed.

With this patch the redirect works, however it takes a long time for CCT to
consult ExternalNavigationHandler. This results in suboptimal UX. Same redirect
in ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank links in Clank.

BUG=740402

Review-Url: https://codereview.chromium.org/2969143002
Cr-Original-Commit-Position: refs/heads/master@{#485831}
Committed:
https://chromium.googlesource.com/chromium/src/+/727468b41b2fe48bf73983ab0c3a...
Review-Url: https://codereview.chromium.org/2969143002
Cr-Commit-Position: refs/heads/master@{#486275}
Committed:
https://chromium.googlesource.com/chromium/src/+/4dbcecfb630ac1a2dba1910f0e42...
==========

[piotrs, 2017-07-18 05:27:53]

Description was changed from

==========
Fixes the redirect to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name (using
component name instead) and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.
Additionally a small change to ExternalNavigationHandler is
required for this change take effect with PlzNavigate turned
on. It only forces NO_REDIRECT on incoming chrome intents if
intent is explicitly directed at Chrome, otherwise a redirect
is allowed.

With this patch the redirect works, however it takes a long time for CCT to
consult ExternalNavigationHandler. This results in suboptimal UX. Same redirect
in ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank links in Clank.

BUG=740402

Review-Url: https://codereview.chromium.org/2969143002
Cr-Original-Commit-Position: refs/heads/master@{#485831}
Committed:
https://chromium.googlesource.com/chromium/src/+/727468b41b2fe48bf73983ab0c3a...
Review-Url: https://codereview.chromium.org/2969143002
Cr-Commit-Position: refs/heads/master@{#486275}
Committed:
https://chromium.googlesource.com/chromium/src/+/4dbcecfb630ac1a2dba1910f0e42...
==========

to

==========
Fixes the redirect to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name (using
component name instead) and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.

With this patch the redirect works, however it takes a long time for CCT to
consult ExternalNavigationHandler. This results in suboptimal UX. Same redirect
in ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank links in Clank.

BUG=740402

Review-Url: https://codereview.chromium.org/2969143002
Cr-Original-Commit-Position: refs/heads/master@{#485831}
Committed:
https://chromium.googlesource.com/chromium/src/+/727468b41b2fe48bf73983ab0c3a...
Review-Url: https://codereview.chromium.org/2969143002
Cr-Commit-Position: refs/heads/master@{#486275}
Committed:
https://chromium.googlesource.com/chromium/src/+/4dbcecfb630ac1a2dba1910f0e42...
==========

[piotrs, 2017-07-18 05:28:03]

Patchset #11 (id:340001) has been deleted

[piotrs, 2017-07-18 05:28:41]

FYI pulled out a fixed needed to get this in into crrev.com/c/575111/.

[piotrs, 2017-07-19 00:34:20]

Sends intents to specialized apps in WebappTabDelegate

[piotrs, 2017-07-19 00:34:21]

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

[commit-bot: I haz the power, 2017-07-19 00:34:34]

Dry run: CQ is trying da patch.

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

[piotrs, 2017-07-19 00:39:20]

For the record, newest patch adding lookup for specialized URL handler to
WebappTabDelegate is in response to our discussion of this issue in
crrev.com/c/575111/.

Yusuf, can you PTAL whether this in line with that you would expect?

[commit-bot: I haz the power, 2017-07-19 02:07:24]

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

[commit-bot: I haz the power, 2017-07-19 02:07:25]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Yusuf, 2017-07-19 03:33:37]

https://codereview.chromium.org/2969143002/diff/360001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappTabDelegate.java
(right):

https://codereview.chromium.org/2969143002/diff/360001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappTabDelegate.java:71:
mActivity.startActivity(intent);
I think dependence on ExternalNavigationDelegate here is hurting us because it
makes us create and cache stuff unnecessarily. It would be good to not cache the
Activity here.

isSpecializedHandler is pretty generic code through PackageManager. And we do go
through the same queryActivities/resolve logic in other places. It might be even
be possible to make getSpecializedHandlersWithFilter static there. Can we try
that or just copying an independent impl here and use
ContextUtils.getApplicationContext to launch the intent and not depend on any
params?

https://codereview.chromium.org/2969143002/diff/360001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappTabDelegate.java:79:
if (intent.getPackage() != null) return true;
is this needed? Explicit intents should be covered well with the
isSpecializedHandlerAvailable below.

[piotrs, 2017-07-19 04:04:48]

No longer instantiates ExternalNavigationDelegateImpl, uses static calls instead

[piotrs, 2017-07-19 04:04:51]

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

[commit-bot: I haz the power, 2017-07-19 04:05:01]

Dry run: CQ is trying da patch.

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

[piotrs, 2017-07-19 04:05:45]

No longer instantiates ExternalNavigationDelegateImpl, uses static calls instead

[piotrs, 2017-07-19 04:05:46]

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

[commit-bot: I haz the power, 2017-07-19 04:05:52]

Dry run: CQ is trying da patch.

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

[piotrs, 2017-07-19 04:05:55]

Patchset #12 (id:380001) has been deleted

[piotrs, 2017-07-19 04:06:25]

Worked out pretty well, PTAL.

https://codereview.chromium.org/2969143002/diff/360001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappTabDelegate.java
(right):

https://codereview.chromium.org/2969143002/diff/360001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappTabDelegate.java:71:
mActivity.startActivity(intent);
I found isPackageSpecializedHandler, which is what I needed and is static.
Cleaned up, PTAL :)

https://codereview.chromium.org/2969143002/diff/360001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappTabDelegate.java:79:
if (intent.getPackage() != null) return true;
Ack, removed.

[Yusuf, 2017-07-19 05:08:41]

lgtm

[piotrs, 2017-07-19 05:36:30]

Merge

[piotrs, 2017-07-19 05:36:31]

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

[commit-bot: I haz the power, 2017-07-19 05:36:40]

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-07-19 06:52:12]

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

[commit-bot: I haz the power, 2017-07-19 06:52:15]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[piotrs, 2017-07-19 07:28:33]

The CQ bit was checked by piotrs@chromium.org

[piotrs, 2017-07-19 07:28:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
dominickn@chromium.org, tedchoc@chromium.org, yusufo@chromium.org
Link to the patchset: https://codereview.chromium.org/2969143002/#ps420001
(title: "Merge")

[commit-bot: I haz the power, 2017-07-19 07:28:43]

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-07-19 09:49:53]

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

[commit-bot: I haz the power, 2017-07-19 09:49:55]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[piotrs, 2017-07-19 23:10:53]

Fix for Marshmallow

[piotrs, 2017-07-19 23:12:11]

Merge

[piotrs, 2017-07-19 23:12:44]

The CQ bit was checked by piotrs@chromium.org

[piotrs, 2017-07-19 23:12:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
dominickn@chromium.org, tedchoc@chromium.org, yusufo@chromium.org
Link to the patchset: https://codereview.chromium.org/2969143002/#ps460001
(title: "Merge")

[commit-bot: I haz the power, 2017-07-19 23:13:04]

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-07-19 23:26:02]

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

[commit-bot: I haz the power, 2017-07-19 23:26:05]

Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)

[piotrs, 2017-07-19 23:59:09]

Merge

[piotrs, 2017-07-20 00:00:28]

Patchset #15 (id:460001) has been deleted

[piotrs, 2017-07-20 00:00:55]

The CQ bit was checked by piotrs@chromium.org

[piotrs, 2017-07-20 00:00:56]

The patchset sent to the CQ was uploaded after l-g-t-m from
dominickn@chromium.org, tedchoc@chromium.org, yusufo@chromium.org
Link to the patchset: https://codereview.chromium.org/2969143002/#ps480001
(title: "Merge")

[commit-bot: I haz the power, 2017-07-20 00:01:17]

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-07-20 02:21:32]

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

[commit-bot: I haz the power, 2017-07-20 02:21:33]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[piotrs, 2017-07-20 03:11:55]

Strict mode disk read violation workaround for
PackageManager#queryIntentActivities()

[piotrs, 2017-07-20 03:12:42]

The CQ bit was checked by piotrs@chromium.org

[piotrs, 2017-07-20 03:12:42]

The patchset sent to the CQ was uploaded after l-g-t-m from
dominickn@chromium.org, tedchoc@chromium.org, yusufo@chromium.org
Link to the patchset: https://codereview.chromium.org/2969143002/#ps500001
(title: "Strict mode disk read violation workaround for
PackageManager#queryIntentActivities()")

[commit-bot: I haz the power, 2017-07-20 03:12:55]

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-07-20 05:19:34]

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

[commit-bot: I haz the power, 2017-07-20 05:19:36]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[piotrs, 2017-07-20 05:34:30]

Start activity for WebappActivity reference

[piotrs, 2017-07-20 05:37:06]

Merge

[piotrs, 2017-07-20 05:37:19]

The CQ bit was checked by piotrs@chromium.org

[piotrs, 2017-07-20 05:37:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
dominickn@chromium.org, tedchoc@chromium.org, yusufo@chromium.org
Link to the patchset: https://codereview.chromium.org/2969143002/#ps540001
(title: "Merge")

[commit-bot: I haz the power, 2017-07-20 05:37: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, 2017-07-20 07:44:30]

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

[commit-bot: I haz the power, 2017-07-20 07:44:32]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[piotrs, 2017-07-20 10:51:36]

Merge

[piotrs, 2017-07-20 10:58:53]

Loosen strict mode, as it is stil violated

[piotrs, 2017-07-20 10:59:41]

The CQ bit was checked by piotrs@chromium.org

[piotrs, 2017-07-20 10:59:42]

The patchset sent to the CQ was uploaded after l-g-t-m from
dominickn@chromium.org, tedchoc@chromium.org, yusufo@chromium.org
Link to the patchset: https://codereview.chromium.org/2969143002/#ps580001
(title: "Loosen strict mode, as it is stil violated")

[commit-bot: I haz the power, 2017-07-20 10:59:55]

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-07-20 11:39:23]

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

[commit-bot: I haz the power, 2017-07-20 11:39:25]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[piotrs, 2017-07-20 12:23:37]

Fix

[piotrs, 2017-07-20 12:24:01]

Patchset #18 (id:540001) has been deleted

[piotrs, 2017-07-20 12:24:16]

The CQ bit was checked by piotrs@chromium.org

[piotrs, 2017-07-20 12:24:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
dominickn@chromium.org, tedchoc@chromium.org, yusufo@chromium.org
Link to the patchset: https://codereview.chromium.org/2969143002/#ps600001
(title: "Fix")

[commit-bot: I haz the power, 2017-07-20 12:24:28]

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-07-20 13:41:23]

CQ is committing da patch.

Bot data: {"patchset_id": 600001, "attempt_start_ts": 1500553456951610,
"parent_rev": "9219f3033f0f56ec5477336b11915e7e2e9f46f2", "commit_rev":
"471df3926c09fda0af15d9157eb29bd326b0f9fc"}

[commit-bot: I haz the power, 2017-07-20 13:41:44]

Description was changed from

==========
Fixes the redirect to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name (using
component name instead) and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.

With this patch the redirect works, however it takes a long time for CCT to
consult ExternalNavigationHandler. This results in suboptimal UX. Same redirect
in ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank links in Clank.

BUG=740402

Review-Url: https://codereview.chromium.org/2969143002
Cr-Original-Commit-Position: refs/heads/master@{#485831}
Committed:
https://chromium.googlesource.com/chromium/src/+/727468b41b2fe48bf73983ab0c3a...
Review-Url: https://codereview.chromium.org/2969143002
Cr-Commit-Position: refs/heads/master@{#486275}
Committed:
https://chromium.googlesource.com/chromium/src/+/4dbcecfb630ac1a2dba1910f0e42...
==========

to

==========
Fixes the redirect to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name (using
component name instead) and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.

With this patch the redirect works, however it takes a long time for CCT to
consult ExternalNavigationHandler. This results in suboptimal UX. Same redirect
in ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank links in Clank.

BUG=740402

Review-Url: https://codereview.chromium.org/2969143002
Cr-Original-Original-Commit-Position: refs/heads/master@{#485831}
Committed:
https://chromium.googlesource.com/chromium/src/+/727468b41b2fe48bf73983ab0c3a...
Review-Url: https://codereview.chromium.org/2969143002
Cr-Original-Commit-Position: refs/heads/master@{#486275}
Committed:
https://chromium.googlesource.com/chromium/src/+/4dbcecfb630ac1a2dba1910f0e42...
Review-Url: https://codereview.chromium.org/2969143002
Cr-Commit-Position: refs/heads/master@{#488228}
Committed:
https://chromium.googlesource.com/chromium/src/+/471df3926c09fda0af15d9157eb2...
==========

[commit-bot: I haz the power, 2017-07-20 13:41:47]

Committed patchset #20 (id:600001) as
https://chromium.googlesource.com/chromium/src/+/471df3926c09fda0af15d9157eb2...