Open the notification URL (not the web app start_url) when deep-linking into standalone mode.

Open the notification URL (not the web app start_url) when deep-linking into standalone mode.

When deep-linking into a standalone web app from a notification, the
app's start_url is always opened and the URL that the notification wants
to open is ignored. This CL fixes the bug by injecting the notification
URL in place of the web app URL when deep-linking. We also force a
reload of the web app if it is already running to ensure that the desired
url is loaded.

Replacing the URL in this manner is correct as the notification URL is
guaranteed to live within the web app's scope for notification
deep-linking to trigger.

BUG=627791
Committed: https://crrev.com/e10048ec434da0f56f6b2c5579bdbee2fa06654d
Cr-Commit-Position: refs/heads/master@{#405638}

[dominickn, 2016-07-13 13:35:23]

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

[commit-bot: I haz the power, 2016-07-13 13:35:48]

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

[dominickn, 2016-07-13 13:36:18]

Description was changed from

==========
Open the notification URL (not the web app start_url) when deep-linking into
standalone mode.

When deep-linking into a standalone web app from a notification, the
app's start_url is always opened and the URL that the notification wants
to open is ignored. This CL fixes the bug by injecting the notification
URL in place of the web app URL when deep-linking.

Replacing the URL in this manner is correct as the notification URL is
guaranteed to live within the web app's scope for notification
deep-linking to trigger.

BUG=627791
==========

to

==========
Open the notification URL (not the web app start_url) when deep-linking into
standalone mode.

When deep-linking into a standalone web app from a notification, the
app's start_url is always opened and the URL that the notification wants
to open is ignored. This CL fixes the bug by injecting the notification
URL in place of the web app URL when deep-linking.

Replacing the URL in this manner is correct as the notification URL is
guaranteed to live within the web app's scope for notification
deep-linking to trigger.

BUG=627791
==========

[dominickn, 2016-07-13 13:36:18]

dominickn@chromium.org changed reviewers:
+ dfalcantara@chromium.org

[dominickn, 2016-07-13 13:38:32]

PTAL, thanks!

[commit-bot: I haz the power, 2016-07-13 14:27:54]

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

[commit-bot: I haz the power, 2016-07-13 14:27:55]

Dry run: This issue passed the CQ dry run.

[gone, 2016-07-13 16:53:52]

lgtm

[dominickn, 2016-07-14 07:00:27]

Description was changed from

==========
Open the notification URL (not the web app start_url) when deep-linking into
standalone mode.

When deep-linking into a standalone web app from a notification, the
app's start_url is always opened and the URL that the notification wants
to open is ignored. This CL fixes the bug by injecting the notification
URL in place of the web app URL when deep-linking.

Replacing the URL in this manner is correct as the notification URL is
guaranteed to live within the web app's scope for notification
deep-linking to trigger.

BUG=627791
==========

to

==========
Open the notification URL (not the web app start_url) when deep-linking into
standalone mode.

When deep-linking into a standalone web app from a notification, the
app's start_url is always opened and the URL that the notification wants
to open is ignored. This CL fixes the bug by injecting the notification
URL in place of the web app URL when deep-linking. We also force a
reload of the web app if it is already running to ensure that the desired
url is loaded.

Replacing the URL in this manner is correct as the notification URL is
guaranteed to live within the web app's scope for notification
deep-linking to trigger.

BUG=627791
==========

[dominickn, 2016-07-14 07:00:39]

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

[commit-bot: I haz the power, 2016-07-14 07:00:46]

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

[dominickn, 2016-07-14 07:04:54]

Upon further testing, ps1 works when the web app isn't running. But when a web
app is running, and a notification comes in, the updated URL doesn't get
forwarded on because WebappActivity.getIntent() always returns the intent that
started the activity, not the one that brought it back active.

Adding PendingIntent.FLAG_UPDATE_CURRENT, Intent.FLAG_ACTIVITY_CLEAR_TOP, or
Intent.FLAG_ACTIVITY_CLEAR_TASK to the launchIntent fired from
WebappLauncherActivity solves this problem, but with a caveat: it relaunches the
WebappActivity, even if it's in the foreground when the notification is clicked.
This is a little un-native since you get the splashscreen flash before the
notification URL loads.

The documentation seems to suggest that setting one of these flags is meant to
update the intent and call WebappActivity.onNewIntent(), but that doesn't
happen.

WDYT about ps2? The refresh is a little painful, but this ensures that
notification urls are respected. Any ideas are welcome.

[commit-bot: I haz the power, 2016-07-14 08:00:24]

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

[commit-bot: I haz the power, 2016-07-14 08:00:25]

Dry run: This issue passed the CQ dry run.

[gone, 2016-07-14 17:16:28]

Yikes... I can see why this would be necessary, but I really would have expected
onNewIntent() to be called.  I guess it should still do so on JB & KK, because
those are still run as single instances instead of allowing multiple instances
like on L+.

Do you think it's worth trying to suppress the splash screen when the restart
happens?

lgtm either way

[dominickn, 2016-07-14 23:48:31]

On 2016/07/14 17:16:28, dfalcantara wrote:
> Yikes... I can see why this would be necessary, but I really would have
expected
> onNewIntent() to be called.  I guess it should still do so on JB & KK, because
> those are still run as single instances instead of allowing multiple instances
> like on L+.
> 
> Do you think it's worth trying to suppress the splash screen when the restart
> happens?
> 
> lgtm either way

Thanks! I don't know if it's possible to detect a restart accurately to suppress
the splashscreen - if you have any ideas, let me know and I can follow up.

[dominickn, 2016-07-14 23:48:39]

The CQ bit was checked by dominickn@chromium.org

[commit-bot: I haz the power, 2016-07-14 23:49:03]

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-07-14 23:54:20]

Description was changed from

==========
Open the notification URL (not the web app start_url) when deep-linking into
standalone mode.

When deep-linking into a standalone web app from a notification, the
app's start_url is always opened and the URL that the notification wants
to open is ignored. This CL fixes the bug by injecting the notification
URL in place of the web app URL when deep-linking. We also force a
reload of the web app if it is already running to ensure that the desired
url is loaded.

Replacing the URL in this manner is correct as the notification URL is
guaranteed to live within the web app's scope for notification
deep-linking to trigger.

BUG=627791
==========

to

==========
Open the notification URL (not the web app start_url) when deep-linking into
standalone mode.

When deep-linking into a standalone web app from a notification, the
app's start_url is always opened and the URL that the notification wants
to open is ignored. This CL fixes the bug by injecting the notification
URL in place of the web app URL when deep-linking. We also force a
reload of the web app if it is already running to ensure that the desired
url is loaded.

Replacing the URL in this manner is correct as the notification URL is
guaranteed to live within the web app's scope for notification
deep-linking to trigger.

BUG=627791
==========

[commit-bot: I haz the power, 2016-07-14 23:54:21]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-14 23:54:59]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-14 23:57:54]

Description was changed from

==========
Open the notification URL (not the web app start_url) when deep-linking into
standalone mode.

When deep-linking into a standalone web app from a notification, the
app's start_url is always opened and the URL that the notification wants
to open is ignored. This CL fixes the bug by injecting the notification
URL in place of the web app URL when deep-linking. We also force a
reload of the web app if it is already running to ensure that the desired
url is loaded.

Replacing the URL in this manner is correct as the notification URL is
guaranteed to live within the web app's scope for notification
deep-linking to trigger.

BUG=627791
==========

to

==========
Open the notification URL (not the web app start_url) when deep-linking into
standalone mode.

When deep-linking into a standalone web app from a notification, the
app's start_url is always opened and the URL that the notification wants
to open is ignored. This CL fixes the bug by injecting the notification
URL in place of the web app URL when deep-linking. We also force a
reload of the web app if it is already running to ensure that the desired
url is loaded.

Replacing the URL in this manner is correct as the notification URL is
guaranteed to live within the web app's scope for notification
deep-linking to trigger.

BUG=627791

Committed: https://crrev.com/e10048ec434da0f56f6b2c5579bdbee2fa06654d
Cr-Commit-Position: refs/heads/master@{#405638}
==========

[commit-bot: I haz the power, 2016-07-14 23:57:55]

Patchset 2 (id:??) landed as
https://crrev.com/e10048ec434da0f56f6b2c5579bdbee2fa06654d
Cr-Commit-Position: refs/heads/master@{#405638}