Expose a WasLaunchedRecently method using the app banner content setting.

Expose a WasLaunchedRecently method using the app banner content setting.

The app banner system records a DID_ADD_TO_HOMESCREEN event every time:

a) a user adds a site to homescreen via an app banner
b) a user adds a site to homescreen via the add to homescreen overflow
   menu item
c) the user launches a site from homescreen

This CL adds AppBannerSettingsHelper::WasLaunchedRecently, which returns
true if there is any site under a supplied origin which has been added
to or launched from homescreen in the last 10 days. This mirrors the
Java-side heuristic used to determine if notification deep-linking
should take effect.

A future CL will look to combine the Java and C++ side heuristics so
that the timestamp data is stored in one location that is accessible to
both.

BUG=631305
Committed: https://crrev.com/83864870eb6ecf3d28266e6fe191fe182fb86436
Cr-Commit-Position: refs/heads/master@{#408530}

[dominickn, 2016-07-26 03:11:53]

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

[commit-bot: I haz the power, 2016-07-26 03:12:02]

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, 2016-07-26 04:13:33]

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

[commit-bot: I haz the power, 2016-07-26 04:13:34]

Dry run: This issue passed the CQ dry run.

[dominickn, 2016-07-26 04:22:49]

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

[dominickn, 2016-07-26 04:22:50]

PTAL, thanks! This is for the ImportantSites folks, and we should also think
about marrying up the Java side WebappDataStorage heuristic with this.

[benwells, 2016-07-28 07:47:01]

lgtm

https://codereview.chromium.org/2185453003/diff/1/chrome/browser/banners/app_...
File chrome/browser/banners/app_banner_settings_helper.cc (right):

https://codereview.chromium.org/2185453003/diff/1/chrome/browser/banners/app_...
chrome/browser/banners/app_banner_settings_helper.cc:577: if (delta >=
base::TimeDelta() &&
Optional nit: maybe we shouldn't test for a negative delta - that could also
return true. Might make more sense for this weird edge case.

https://codereview.chromium.org/2185453003/diff/1/chrome/browser/banners/app_...
File chrome/browser/banners/app_banner_settings_helper.h (right):

https://codereview.chromium.org/2185453003/diff/1/chrome/browser/banners/app_...
chrome/browser/banners/app_banner_settings_helper.h:137: static bool
WasLaunchedRecently(content::WebContents* web_contents,
Its kind of weird that ImportantSites would depend on code in banners/ to see if
something had been launched from the homescreen.

Something to clean up in future CLs.

[dominickn, 2016-07-28 08:41:23]

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

[commit-bot: I haz the power, 2016-07-28 08:41:34]

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, 2016-07-28 09:09:26]

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

[commit-bot: I haz the power, 2016-07-28 09:09:27]

Dry run: This issue passed the CQ dry run.

[dominickn, 2016-07-29 00:34:09]

https://codereview.chromium.org/2185453003/diff/1/chrome/browser/banners/app_...
File chrome/browser/banners/app_banner_settings_helper.cc (right):

https://codereview.chromium.org/2185453003/diff/1/chrome/browser/banners/app_...
chrome/browser/banners/app_banner_settings_helper.cc:577: if (delta >=
base::TimeDelta() &&
On 2016/07/28 07:47:01, benwells wrote:
> Optional nit: maybe we shouldn't test for a negative delta - that could also
> return true. Might make more sense for this weird edge case.

Done.

https://codereview.chromium.org/2185453003/diff/1/chrome/browser/banners/app_...
File chrome/browser/banners/app_banner_settings_helper.h (right):

https://codereview.chromium.org/2185453003/diff/1/chrome/browser/banners/app_...
chrome/browser/banners/app_banner_settings_helper.h:137: static bool
WasLaunchedRecently(content::WebContents* web_contents,
On 2016/07/28 07:47:01, benwells wrote:
> Its kind of weird that ImportantSites would depend on code in banners/ to see
if
> something had been launched from the homescreen.
> 
> Something to clean up in future CLs.

Yeah, this is definitely something I want to tidy up. It's a mess with the Java
side / native side split. It's unfortunate that the one consistent marker of
homescreen-ness across both is the freshness of this content setting event.

[dominickn, 2016-07-29 00:34:15]

The CQ bit was checked by dominickn@chromium.org

[dominickn, 2016-07-29 00:34:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
benwells@chromium.org
Link to the patchset: https://codereview.chromium.org/2185453003/#ps20001
(title: "Nits")

[commit-bot: I haz the power, 2016-07-29 00:34:38]

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-29 00:38:18]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-29 00:39:29]

Description was changed from

==========
Expose a WasLaunchedRecently method using the app banner content setting.

The app banner system records a DID_ADD_TO_HOMESCREEN event every time:

a) a user adds a site to homescreen via an app banner
b) a user adds a site to homescreen via the add to homescreen overflow
   menu item
c) the user launches a site from homescreen

This CL adds AppBannerSettingsHelper::WasLaunchedRecently, which returns
true if there is any site under a supplied origin which has been added
to or launched from homescreen in the last 10 days. This mirrors the
Java-side heuristic used to determine if notification deep-linking
should take effect.

A future CL will look to combine the Java and C++ side heuristics so
that the timestamp data is stored in one location that is accessible to
both.

BUG=631305
==========

to

==========
Expose a WasLaunchedRecently method using the app banner content setting.

The app banner system records a DID_ADD_TO_HOMESCREEN event every time:

a) a user adds a site to homescreen via an app banner
b) a user adds a site to homescreen via the add to homescreen overflow
   menu item
c) the user launches a site from homescreen

This CL adds AppBannerSettingsHelper::WasLaunchedRecently, which returns
true if there is any site under a supplied origin which has been added
to or launched from homescreen in the last 10 days. This mirrors the
Java-side heuristic used to determine if notification deep-linking
should take effect.

A future CL will look to combine the Java and C++ side heuristics so
that the timestamp data is stored in one location that is accessible to
both.

BUG=631305

Committed: https://crrev.com/83864870eb6ecf3d28266e6fe191fe182fb86436
Cr-Commit-Position: refs/heads/master@{#408530}
==========

[commit-bot: I haz the power, 2016-07-29 00:39:31]

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