Index: chrome/browser/banners/app_banner_manager.cc |
diff --git a/chrome/browser/banners/app_banner_manager.cc b/chrome/browser/banners/app_banner_manager.cc |
index 9524f7eff2093ecb1489a53631f279852a4d1392..976db92ce6a877047bac558b1e39f1fa499e5a9f 100644 |
--- a/chrome/browser/banners/app_banner_manager.cc |
+++ b/chrome/browser/banners/app_banner_manager.cc |
@@ -110,6 +110,7 @@ void AppBannerManager::RequestAppBanner(const GURL& validated_url, |
if (binding_.is_bound()) |
binding_.Close(); |
+ UpdateState(State::PENDING_MANIFEST); |
manager_->GetData( |
ParamsToGetManifest(), |
base::Bind(&AppBannerManager::OnDidGetManifest, GetWeakPtr())); |
@@ -206,6 +207,7 @@ bool AppBannerManager::IsWebAppInstalled( |
} |
void AppBannerManager::OnDidGetManifest(const InstallableData& data) { |
+ UpdateState(State::ACTIVE); |
if (data.error_code != NO_ERROR_DETECTED) { |
ReportStatus(web_contents(), data.error_code); |
Stop(); |
@@ -237,6 +239,11 @@ InstallableParams AppBannerManager::ParamsToPerformInstallableCheck() { |
params.check_installable = true; |
params.fetch_valid_primary_icon = true; |
+ // Don't wait for the worker if we're in debug mode. However, we want to |
+ // ignore whether the bypass flag is on for this purpose (otherwise we won't |
+ // wait for the worker in that instance). |
benwells
2017/06/14 11:39:06
Maybe it's just late but this confuses me.
Edit:
dominickn
2017/06/15 04:16:07
Done.
|
+ params.wait_for_worker = !is_debug_mode_; |
+ |
return params; |
} |
@@ -245,6 +252,7 @@ void AppBannerManager::PerformInstallableCheck() { |
return; |
// Fetch and verify the other required information. |
+ UpdateState(State::PENDING_INSTALLABLE_CHECK); |
manager_->GetData(ParamsToPerformInstallableCheck(), |
base::Bind(&AppBannerManager::OnDidPerformInstallableCheck, |
GetWeakPtr())); |
@@ -252,6 +260,7 @@ void AppBannerManager::PerformInstallableCheck() { |
void AppBannerManager::OnDidPerformInstallableCheck( |
const InstallableData& data) { |
+ UpdateState(State::ACTIVE); |
if (data.is_installable) |
TrackDisplayEvent(DISPLAY_EVENT_WEB_APP_BANNER_REQUESTED); |
@@ -327,14 +336,15 @@ void AppBannerManager::Stop() { |
!has_sufficient_engagement_) { |
TrackDisplayEvent(DISPLAY_EVENT_NOT_VISITED_ENOUGH); |
ReportStatus(web_contents(), INSUFFICIENT_ENGAGEMENT); |
+ } else if (state_ == State::PENDING_MANIFEST) { |
+ ReportStatus(web_contents(), WAITING_FOR_MANIFEST); |
+ } else if (state_ == State::PENDING_INSTALLABLE_CHECK) { |
+ ReportStatus(web_contents(), WAITING_FOR_INSTALLABLE_CHECK); |
} |
// In every non-debug run through the banner pipeline, we should have called |
- // ReportStatus() and set need_to_log_status_ to false. The only case where |
- // we don't is if we're still active and waiting for a callback from the |
- // InstallableManager (e.g. the renderer crashes or the browser is shutting |
- // down). These situations are explicitly not logged. |
- DCHECK(!need_to_log_status_ || is_active()); |
+ // ReportStatus() and set need_to_log_status_ to false. |
+ DCHECK(!need_to_log_status_); |
weak_factory_.InvalidateWeakPtrs(); |
binding_.Close(); |
@@ -342,7 +352,6 @@ void AppBannerManager::Stop() { |
event_.reset(); |
UpdateState(State::COMPLETE); |
- need_to_log_status_ = false; |
has_sufficient_engagement_ = false; |
} |