Commit e7020ca2 authored by dfalcantara's avatar dfalcantara Committed by Commit bot

Move banner counting logic much later in the pipeline

The banner can fail to be shown for any number of reasons,
from a user navigation to a failed icon fetch.  Record that
the banner could have been shown only right before the
infobar is created.

This fixes situations where a user already had a native app
installed and the website requested a banner for it, which
permanently fills a slot in app dictionary for the site --
even if the Play Store tells the AppBannerManager that the
app is installed and no banner should be shown.

BUG=453170,452825

Review URL: https://codereview.chromium.org/905913002

Cr-Commit-Position: refs/heads/master@{#315193}
parent 218b32cc
...@@ -229,10 +229,6 @@ void AppBannerManager::OnDidCheckHasServiceWorker(bool has_service_worker) { ...@@ -229,10 +229,6 @@ void AppBannerManager::OnDidCheckHasServiceWorker(bool has_service_worker) {
if (icon_url.is_empty()) if (icon_url.is_empty())
return; return;
RecordCouldShowBanner(web_app_data_.start_url.spec());
if (!CheckIfShouldShow(web_app_data_.start_url.spec()))
return;
FetchIcon(icon_url); FetchIcon(icon_url);
} }
} }
...@@ -285,11 +281,19 @@ void AppBannerManager::OnFetchComplete(const GURL url, const SkBitmap* bitmap) { ...@@ -285,11 +281,19 @@ void AppBannerManager::OnFetchComplete(const GURL url, const SkBitmap* bitmap) {
weak_infobar_ptr_ = nullptr; weak_infobar_ptr_ = nullptr;
if (!native_app_data_.is_null()) { if (!native_app_data_.is_null()) {
RecordCouldShowBanner(native_app_package_);
if (!CheckIfShouldShow(native_app_package_))
return;
weak_infobar_ptr_ = AppBannerInfoBarDelegate::CreateForNativeApp( weak_infobar_ptr_ = AppBannerInfoBarDelegate::CreateForNativeApp(
service, service,
this, this,
native_app_data_); native_app_data_);
} else if (!web_app_data_.IsEmpty()){ } else if (!web_app_data_.IsEmpty()){
RecordCouldShowBanner(web_app_data_.start_url.spec());
if (!CheckIfShouldShow(web_app_data_.start_url.spec()))
return;
weak_infobar_ptr_ = AppBannerInfoBarDelegate::CreateForWebApp( weak_infobar_ptr_ = AppBannerInfoBarDelegate::CreateForWebApp(
service, service,
this, this,
...@@ -313,10 +317,6 @@ void AppBannerManager::OnDidRetrieveMetaTagContent( ...@@ -313,10 +317,6 @@ void AppBannerManager::OnDidRetrieveMetaTagContent(
banners::TrackDisplayEvent(DISPLAY_BANNER_REQUESTED); banners::TrackDisplayEvent(DISPLAY_BANNER_REQUESTED);
RecordCouldShowBanner(tag_content);
if (!CheckIfShouldShow(tag_content))
return;
// Send the info to the Java side to get info about the app. // Send the info to the Java side to get info about the app.
JNIEnv* env = base::android::AttachCurrentThread(); JNIEnv* env = base::android::AttachCurrentThread();
ScopedJavaLocalRef<jobject> jobj = weak_java_banner_view_manager_.get(env); ScopedJavaLocalRef<jobject> jobj = weak_java_banner_view_manager_.get(env);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment