Separate web app installability logic from native app installability in method names

This CL makes a distinction between the web app installability code path and the
native app (Play) installability code path used by Android using the names
"InstallableWebApp" and "InstallableNativeApp" respectively.

This also rejigs the WeakPtrFactory arrangement to not require the base class
to have subclass only methods.

There are no behavioural changes made by this CL.

Bug: 964314
Change-Id: I1b1d0d3edf91889da4e3caf673208b9a8671fbb4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1626056
Auto-Submit: Alan Cutter <alancutter@chromium.org>
Commit-Queue: Michael Wasserman <msw@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#663856}

chrome/browser/banners/app_banner_manager.cc

@@ -210,10 +210,6 @@ void AppBannerManager::SendBannerDismissed() {
   SendBannerPromptRequest();
 }
 
-base::WeakPtr<AppBannerManager> AppBannerManager::GetWeakPtr() {
-  return weak_factory_.GetWeakPtr();
-}
-
 void AppBannerManager::AddObserver(Observer* observer) {
   observer_list_.AddObserver(observer);
 }
@@ -233,8 +229,8 @@ AppBannerManager::AppBannerManager(content::WebContents* web_contents)
       load_finished_(false),
       status_reporter_(std::make_unique<NullStatusReporter>()),
       install_animation_pending_(false),
-      installable_(Installable::UNKNOWN),
-      weak_factory_(this) {
+      installable_web_app_check_result_(
+          InstallableWebAppCheckResult::kUnknown) {
   DCHECK(manager_);
 
   AppBannerSettingsHelper::UpdateFromFieldTrial();
@@ -314,10 +310,10 @@ void AppBannerManager::OnDidGetManifest(const InstallableData& data) {
   manifest_url_ = data.manifest_url;
   manifest_ = *data.manifest;
 
-  PerformInstallableCheck();
+  PerformInstallableChecks();
 }
 
-InstallableParams AppBannerManager::ParamsToPerformInstallableCheck() {
+InstallableParams AppBannerManager::ParamsToPerformInstallableWebAppCheck() {
   InstallableParams params;
   params.valid_primary_icon = true;
   params.valid_manifest = true;
@@ -327,27 +323,32 @@ InstallableParams AppBannerManager::ParamsToPerformInstallableCheck() {
   return params;
 }
 
-void AppBannerManager::PerformInstallableCheck() {
+void AppBannerManager::PerformInstallableChecks() {
+  PerformInstallableWebAppCheck();
+}
+
+void AppBannerManager::PerformInstallableWebAppCheck() {
   if (!CheckIfShouldShowBanner())
     return;
 
   // Fetch and verify the other required information.
   UpdateState(State::PENDING_INSTALLABLE_CHECK);
   manager_->GetData(
-      ParamsToPerformInstallableCheck(),
-      base::BindOnce(&AppBannerManager::OnDidPerformInstallableCheck,
+      ParamsToPerformInstallableWebAppCheck(),
+      base::BindOnce(&AppBannerManager::OnDidPerformInstallableWebAppCheck,
                      GetWeakPtr()));
 }
 
-void AppBannerManager::OnDidPerformInstallableCheck(
+void AppBannerManager::OnDidPerformInstallableWebAppCheck(
     const InstallableData& data) {
   UpdateState(State::ACTIVE);
   if (data.has_worker && data.valid_manifest)
     TrackDisplayEvent(DISPLAY_EVENT_WEB_APP_BANNER_REQUESTED);
 
   auto error = data.errors.empty() ? NO_ERROR_DETECTED : data.errors[0];
-  SetInstallable(error == NO_ERROR_DETECTED ? Installable::INSTALLABLE_YES
-                                            : Installable::INSTALLABLE_NO);
+  SetInstallableWebAppCheckResult(error == NO_ERROR_DETECTED
+                                      ? InstallableWebAppCheckResult::kYes
+                                      : InstallableWebAppCheckResult::kNo);
 
   if (error != NO_ERROR_DETECTED) {
     if (error == NO_MATCHING_SERVICE_WORKER)
@@ -410,7 +411,7 @@ void AppBannerManager::ResetCurrentPageData() {
   manifest_ = blink::Manifest();
   manifest_url_ = GURL();
   validated_url_ = GURL();
-  SetInstallable(Installable::UNKNOWN);
+  SetInstallableWebAppCheckResult(InstallableWebAppCheckResult::kUnknown);
 }
 
 void AppBannerManager::Terminate() {
@@ -448,34 +449,36 @@ InstallableStatusCode AppBannerManager::TerminationCode() const {
   return NO_ERROR_DETECTED;
 }
 
-bool AppBannerManager::IsInstallable() const {
-  return installable_ == Installable::INSTALLABLE_YES;
+bool AppBannerManager::IsInstallableWebApp() const {
+  return installable_web_app_check_result_ ==
+         InstallableWebAppCheckResult::kYes;
 }
 
-void AppBannerManager::SetInstallable(Installable installable) {
-  if (installable_ == installable)
+void AppBannerManager::SetInstallableWebAppCheckResult(
+    InstallableWebAppCheckResult result) {
+  if (installable_web_app_check_result_ == result)
     return;
 
-  installable_ = installable;
+  installable_web_app_check_result_ = result;
 
-  switch (installable) {
-    case Installable::UNKNOWN:
+  switch (result) {
+    case InstallableWebAppCheckResult::kUnknown:
       break;
-    case Installable::INSTALLABLE_YES:
-      last_installable_scope_ = manifest_.scope;
-      DCHECK(!last_installable_scope_.is_empty());
+    case InstallableWebAppCheckResult::kYes:
+      last_installable_web_app_scope_ = manifest_.scope;
+      DCHECK(!last_installable_web_app_scope_.is_empty());
       install_animation_pending_ =
           AppBannerSettingsHelper::CanShowInstallTextAnimation(
-              web_contents(), last_installable_scope_);
+              web_contents(), last_installable_web_app_scope_);
       break;
-    case Installable::INSTALLABLE_NO:
-      last_installable_scope_ = GURL();
+    case InstallableWebAppCheckResult::kNo:
+      last_installable_web_app_scope_ = GURL();
       install_animation_pending_ = false;
       break;
   }
 
   for (Observer& observer : observer_list_)
-    observer.OnInstallabilityUpdated();
+    observer.OnInstallableWebAppStatusUpdated();
 }
 
 void AppBannerManager::MigrateObserverListForTesting(
@@ -491,9 +494,10 @@ void AppBannerManager::MigrateObserverListForTesting(
 void AppBannerManager::Stop(InstallableStatusCode code) {
   ReportStatus(code);
 
-  if (installable_ == Installable::UNKNOWN)
-    SetInstallable(Installable::INSTALLABLE_NO);
-  weak_factory_.InvalidateWeakPtrs();
+  if (installable_web_app_check_result_ ==
+      InstallableWebAppCheckResult::kUnknown)
+    SetInstallableWebAppCheckResult(InstallableWebAppCheckResult::kNo);
+  InvalidateWeakPtrs();
   ResetBindings();
   UpdateState(State::COMPLETE);
   status_reporter_ = std::make_unique<NullStatusReporter>(),
@@ -634,30 +638,31 @@ bool AppBannerManager::IsRunning() const {
 }
 
 // static
-base::string16 AppBannerManager::GetInstallableAppName(
+base::string16 AppBannerManager::GetInstallableWebAppName(
     content::WebContents* web_contents) {
   AppBannerManager* manager = FromWebContents(web_contents);
-  if (!manager || !manager->IsInstallable())
+  if (!manager || !manager->IsInstallableWebApp())
     return base::string16();
   return manager->GetAppName();
 }
 
-bool AppBannerManager::IsProbablyInstallable() const {
-  if (IsInstallable())
+bool AppBannerManager::IsProbablyInstallableWebApp() const {
+  if (IsInstallableWebApp())
     return true;
-  return installable_ == Installable::UNKNOWN &&
-         last_installable_scope_.is_valid() &&
+  return installable_web_app_check_result_ ==
+             InstallableWebAppCheckResult::kUnknown &&
+         last_installable_web_app_scope_.is_valid() &&
          base::StartsWith(web_contents()->GetLastCommittedURL().spec(),
-                          last_installable_scope_.spec(),
+                          last_installable_web_app_scope_.spec(),
                           base::CompareCase::SENSITIVE);
 }
 
 bool AppBannerManager::MaybeConsumeInstallAnimation() {
-  DCHECK(IsProbablyInstallable());
+  DCHECK(IsProbablyInstallableWebApp());
   if (!install_animation_pending_)
     return false;
   AppBannerSettingsHelper::RecordInstallTextAnimationShown(
-      web_contents(), last_installable_scope_);
+      web_contents(), last_installable_web_app_scope_);
   install_animation_pending_ = false;
   return true;
 }

chrome/browser/banners/app_banner_manager.h

@@ -15,7 +15,6 @@
 #include "base/scoped_observer.h"
 #include "base/strings/string16.h"
 #include "chrome/browser/engagement/site_engagement_observer.h"
-#include "chrome/browser/installable/installable_ambient_badge_infobar_delegate.h"
 #include "chrome/browser/installable/installable_logging.h"
 #include "chrome/browser/installable/installable_manager.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
@@ -35,12 +34,6 @@ class RenderFrameHost;
 class WebContents;
 }  // namespace content
 
-// This forward declaration exists solely for the DidFinishCreatingWebApp
-// callback, implemented and called on desktop platforms only.
-namespace web_app {
-enum class InstallResultCode;
-}
-
 namespace banners {
 
 // Coordinates the creation of an app banner, from detecting eligibility to
@@ -48,10 +41,6 @@ namespace banners {
 // banner using the web app manifest. One web/native app may occupy the pipeline
 // at a time; navigation resets the manager and discards any work in progress.
 //
-// This class contains the generic functionality shared between all platforms,
-// as well as no-op callbacks that the platform-specific implementations pass to
-// base::Bind. This allows a WeakPtrFactory to be housed in this class.
-//
 // The InstallableManager fetches and validates whether a site is eligible for
 // banners. The manager is first called to fetch the manifest, so we can verify
 // whether the site is already installed (and on Android, divert the flow to a
@@ -59,7 +48,6 @@ namespace banners {
 // web app banner (checking manifest validity, service worker, and icon).
 class AppBannerManager : public content::WebContentsObserver,
                          public blink::mojom::AppBannerService,
-                         public InstallableAmbientBadgeInfoBarDelegate::Client,
                          public SiteEngagementObserver {
  public:
   class Observer : public base::CheckedObserver {
@@ -69,7 +57,7 @@ class AppBannerManager : public content::WebContentsObserver,
 
     void ObserveAppBannerManager(AppBannerManager* manager);
 
-    virtual void OnInstallabilityUpdated() = 0;
+    virtual void OnInstallableWebAppStatusUpdated() = 0;
 
    private:
     ScopedObserver<AppBannerManager, Observer> scoped_observer_{this};
@@ -119,7 +107,7 @@ class AppBannerManager : public content::WebContentsObserver,
 
   // Installable describes whether a site satisifes the installablity
   // requirements.
-  enum class Installable { INSTALLABLE_YES, INSTALLABLE_NO, UNKNOWN };
+  enum class InstallableWebAppCheckResult { kUnknown, kNo, kYes };
 
   // Retrieves the platform specific instance of AppBannerManager from
   // |web_contents|.
@@ -140,13 +128,13 @@ class AppBannerManager : public content::WebContentsObserver,
 
   // Returns the app name if the current page is installable, otherwise returns
   // the empty string.
-  static base::string16 GetInstallableAppName(
+  static base::string16 GetInstallableWebAppName(
       content::WebContents* web_contents);
 
   // Returns whether installability checks have passed (e.g. having a service
   // worker fetch event) or have passed previously within the current manifest
   // scope.
-  bool IsProbablyInstallable() const;
+  bool IsProbablyInstallableWebApp() const;
 
   // Each successful installability check gets to show one animation prompt,
   // this returns and consumes the animation prompt if it is available.
@@ -167,34 +155,9 @@ class AppBannerManager : public content::WebContentsObserver,
   // Sends a message to the renderer that the user dismissed the banner.
   void SendBannerDismissed();
 
-  // Returns a WeakPtr to this object. Exposed so subclasses/infobars may
-  // may bind callbacks without needing their own WeakPtrFactory.
-  base::WeakPtr<AppBannerManager> GetWeakPtr();
-
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
 
-  // Overridden on desktop platforms. Called to initiate the web app
-  // install. Not used on Android.
-  virtual void CreateWebApp(WebappInstallSource install_source) {}
-
-  // Overridden and passed through base::Bind on desktop platforms. Called when
-  // the web app install initiated by a banner has completed. Not used on
-  // Android.
-  virtual void DidFinishCreatingWebApp(const web_app::AppId& app_id,
-                                       web_app::InstallResultCode code) {}
-
-  // Overridden and passed through base::Bind on Android. Called when the
-  // download of a native app's icon is complete, as native banners use an icon
-  // provided from the Play Store rather than the web manifest. Not used on
-  // desktop platforms.
-  virtual void OnAppIconFetched(const SkBitmap& bitmap) {}
-
-  // InstallableAmbientBadgeInfoBarDelegate::Client overrides. Further
-  // overridden on Android.
-  void AddToHomescreenFromBadge() override {}
-  void BadgeDismissed() override {}
-
  protected:
   explicit AppBannerManager(content::WebContents* web_contents);
   ~AppBannerManager() override;
@@ -219,6 +182,9 @@ class AppBannerManager : public content::WebContentsObserver,
   // alerting websites that a banner is about to be created.
   virtual std::string GetBannerType();
 
+  virtual base::WeakPtr<AppBannerManager> GetWeakPtr() = 0;
+  virtual void InvalidateWeakPtrs() = 0;
+
   // Returns true if |has_sufficient_engagement_| is true or
   // ShouldBypassEngagementChecks() returns true.
   bool HasSufficientEngagement() const;
@@ -226,7 +192,7 @@ class AppBannerManager : public content::WebContentsObserver,
   // Returns true if the kBypassAppBannerEngagementChecks flag is set.
   bool ShouldBypassEngagementChecks() const;
 
-  // Returns true if the webapp at |start_url| has already been installed, or
+  // Returns true if the web app at |start_url| has already been installed, or
   // should be considered installed. On Android, we rely on a heuristic that
   // may yield false negatives or false positives (crbug.com/786268).
   virtual bool IsWebAppConsideredInstalled(content::WebContents* web_contents,
@@ -240,18 +206,21 @@ class AppBannerManager : public content::WebContentsObserver,
 
   // Returns an InstallableParams object that requests all checks necessary for
   // a web app banner.
-  virtual InstallableParams ParamsToPerformInstallableCheck();
+  virtual InstallableParams ParamsToPerformInstallableWebAppCheck();
 
   // Run at the conclusion of OnDidGetManifest. For web app banners, this calls
   // back to the InstallableManager to continue checking criteria. For native
   // app banners, this checks whether native apps are preferred in the manifest,
   // and calls to Java to verify native app details. If a native banner isn't or
   // can't be requested, it continues with the web app banner checks.
-  virtual void PerformInstallableCheck();
+  virtual void PerformInstallableChecks();
+
+  virtual void PerformInstallableWebAppCheck();
 
   // Callback invoked by the InstallableManager once it has finished checking
   // all other installable properties.
-  virtual void OnDidPerformInstallableCheck(const InstallableData& result);
+  virtual void OnDidPerformInstallableWebAppCheck(
+      const InstallableData& result);
 
   // Records that a banner was shown. The |event_name| corresponds to the RAPPOR
   // metric being recorded.
@@ -280,7 +249,7 @@ class AppBannerManager : public content::WebContentsObserver,
   void SendBannerPromptRequest();
 
   // Shows the ambient badge if the current page advertises a native app or is
-  // a PWA. By default this shows nothing, but platform-specific code might
+  // a web app. By default this shows nothing, but platform-specific code might
   // override this to show UI (e.g. on Android).
   virtual void MaybeShowAmbientBadge();
 
@@ -368,8 +337,8 @@ class AppBannerManager : public content::WebContentsObserver,
   // Returns a status code based on the current state, to log when terminating.
   InstallableStatusCode TerminationCode() const;
 
-  bool IsInstallable() const;
-  void SetInstallable(Installable installable);
+  bool IsInstallableWebApp() const;
+  void SetInstallableWebAppCheckResult(InstallableWebAppCheckResult result);
 
   // Fetches the data required to display a banner for the current page.
   InstallableManager* manager_;
@@ -390,19 +359,14 @@ class AppBannerManager : public content::WebContentsObserver,
 
   std::unique_ptr<StatusReporter> status_reporter_;
   bool install_animation_pending_;
-  Installable installable_;
+  InstallableWebAppCheckResult installable_web_app_check_result_;
 
   // The scope of the most recent installability check if successful otherwise
   // invalid.
-  GURL last_installable_scope_;
+  GURL last_installable_web_app_scope_;
 
   base::ObserverList<Observer, true> observer_list_;
 
-  // The concrete subclasses of this class are expected to have their lifetimes
-  // scoped to the WebContents which they are observing. This allows us to use
-  // weak pointers for callbacks.
-  base::WeakPtrFactory<AppBannerManager> weak_factory_;
-
   DISALLOW_COPY_AND_ASSIGN(AppBannerManager);
 };
 

chrome/browser/banners/app_banner_manager_android.cc

@@ -114,7 +114,8 @@ bool AppBannerManagerAndroid::OnAppDetailsRetrieved(
       web_contents(), primary_icon_url_,
       ShortcutHelper::GetIdealHomescreenIconSizeInPx(),
       ShortcutHelper::GetMinimumHomescreenIconSizeInPx(),
-      base::BindOnce(&AppBannerManager::OnAppIconFetched, GetWeakPtr()));
+      base::BindOnce(&AppBannerManagerAndroid::OnNativeAppIconFetched,
+                     weak_factory_.GetWeakPtr()));
 }
 
 void AppBannerManagerAndroid::RequestAppBanner(const GURL& validated_url) {
@@ -165,44 +166,31 @@ bool AppBannerManagerAndroid::IsWebAppConsideredInstalled(
                                            start_url, manifest_url);
 }
 
-InstallableParams AppBannerManagerAndroid::ParamsToPerformInstallableCheck() {
+InstallableParams
+AppBannerManagerAndroid::ParamsToPerformInstallableWebAppCheck() {
   InstallableParams params =
-      AppBannerManager::ParamsToPerformInstallableCheck();
+      AppBannerManager::ParamsToPerformInstallableWebAppCheck();
   params.valid_badge_icon = can_install_webapk_;
 
   return params;
 }
 
-void AppBannerManagerAndroid::PerformInstallableCheck() {
-  // Check if the manifest prefers that we show a native app banner. If so, call
-  // to Java to verify the details.
-  if (manifest_.prefer_related_applications &&
-      manifest_.related_applications.size() &&
-      !java_banner_manager_.is_null()) {
-    InstallableStatusCode code = NO_ERROR_DETECTED;
-    for (const auto& application : manifest_.related_applications) {
-      std::string platform = base::UTF16ToUTF8(application.platform.string());
-      std::string id = base::UTF16ToUTF8(application.id.string());
-      code = QueryNativeApp(platform, application.url, id);
-      if (code == NO_ERROR_DETECTED)
-        return;
-    }
-
-    // We must have some error in |code| if we reached this point, so report it.
-    Stop(code);
-    return;
-  }
+void AppBannerManagerAndroid::PerformInstallableChecks() {
+  if (ShouldPerformInstallableNativeAppCheck())
+    PerformInstallableNativeAppCheck();
+  else
+    PerformInstallableWebAppCheck();
+}
 
+void AppBannerManagerAndroid::PerformInstallableWebAppCheck() {
   if (can_install_webapk_ && !AreWebManifestUrlsWebApkCompatible(manifest_)) {
     Stop(URL_NOT_SUPPORTED_FOR_WEBAPK);
     return;
   }
-
-  // No native app banner was requested. Continue checking for a web app banner.
-  AppBannerManager::PerformInstallableCheck();
+  AppBannerManager::PerformInstallableWebAppCheck();
 }
 
-void AppBannerManagerAndroid::OnDidPerformInstallableCheck(
+void AppBannerManagerAndroid::OnDidPerformInstallableWebAppCheck(
     const InstallableData& data) {
   if (data.badge_icon && !data.badge_icon->drawsNothing()) {
     DCHECK(!data.badge_icon_url.is_empty());
@@ -211,27 +199,7 @@ void AppBannerManagerAndroid::OnDidPerformInstallableCheck(
     badge_icon_ = *data.badge_icon;
   }
 
-  AppBannerManager::OnDidPerformInstallableCheck(data);
-}
-
-void AppBannerManagerAndroid::OnAppIconFetched(const SkBitmap& bitmap) {
-  if (bitmap.drawsNothing()) {
-    Stop(NO_ICON_AVAILABLE);
-    return;
-  }
-
-  primary_icon_ = bitmap;
-
-  // If we triggered the installability check on page load, then it's possible
-  // we don't have enough engagement yet. If that's the case, return here but
-  // don't call Terminate(). We wait for OnEngagementEvent to tell us that we
-  // should trigger.
-  if (!HasSufficientEngagement()) {
-    UpdateState(State::PENDING_ENGAGEMENT);
-    return;
-  }
-
-  SendBannerPromptRequest();
+  AppBannerManager::OnDidPerformInstallableWebAppCheck(data);
 }
 
 void AppBannerManagerAndroid::ResetCurrentPageData() {
@@ -247,13 +215,13 @@ void AppBannerManagerAndroid::ShowBannerUi(WebappInstallSource install_source) {
 
   if (native_app_data_.is_null()) {
     ui_delegate_ = AppBannerUiDelegateAndroid::Create(
-        GetWeakPtr(),
+        weak_factory_.GetWeakPtr(),
         ShortcutHelper::CreateShortcutInfo(manifest_url_, manifest_,
                                            primary_icon_url_, badge_icon_url_),
         primary_icon_, badge_icon_, install_source, can_install_webapk_);
   } else {
     ui_delegate_ = AppBannerUiDelegateAndroid::Create(
-        GetWeakPtr(), native_app_title_,
+        weak_factory_.GetWeakPtr(), native_app_title_,
         base::android::ScopedJavaLocalRef<jobject>(native_app_data_),
         primary_icon_, native_app_package_);
   }
@@ -295,6 +263,27 @@ std::string AppBannerManagerAndroid::ExtractQueryValueForName(
   return std::string();
 }
 
+bool AppBannerManagerAndroid::ShouldPerformInstallableNativeAppCheck() {
+  return manifest_.prefer_related_applications &&
+         manifest_.related_applications.size() &&
+         !java_banner_manager_.is_null();
+}
+
+void AppBannerManagerAndroid::PerformInstallableNativeAppCheck() {
+  DCHECK(ShouldPerformInstallableNativeAppCheck());
+  InstallableStatusCode code = NO_ERROR_DETECTED;
+  for (const auto& application : manifest_.related_applications) {
+    std::string platform = base::UTF16ToUTF8(application.platform.string());
+    std::string id = base::UTF16ToUTF8(application.id.string());
+    code = QueryNativeApp(platform, application.url, id);
+    if (code == NO_ERROR_DETECTED)
+      return;
+  }
+
+  // We must have some error in |code| if we reached this point, so report it.
+  Stop(code);
+}
+
 InstallableStatusCode AppBannerManagerAndroid::QueryNativeApp(
     const std::string& platform,
     const GURL& url,
@@ -335,6 +324,26 @@ InstallableStatusCode AppBannerManagerAndroid::QueryNativeApp(
   return NO_ERROR_DETECTED;
 }
 
+void AppBannerManagerAndroid::OnNativeAppIconFetched(const SkBitmap& bitmap) {
+  if (bitmap.drawsNothing()) {
+    Stop(NO_ICON_AVAILABLE);
+    return;
+  }
+
+  primary_icon_ = bitmap;
+
+  // If we triggered the installability check on page load, then it's possible
+  // we don't have enough engagement yet. If that's the case, return here but
+  // don't call Terminate(). We wait for OnEngagementEvent to tell us that we
+  // should trigger.
+  if (!HasSufficientEngagement()) {
+    UpdateState(State::PENDING_ENGAGEMENT);
+    return;
+  }
+
+  SendBannerPromptRequest();
+}
+
 base::string16 AppBannerManagerAndroid::GetAppName() const {
   if (native_app_data_.is_null()) {
     // Prefer the short name if it's available. It's guaranteed that at least
@@ -361,9 +370,9 @@ void AppBannerManagerAndroid::MaybeShowAmbientBadge() {
   InfoBarService* infobar_service =
       InfoBarService::FromWebContents(web_contents());
   if (GetVisibleAmbientBadgeInfoBar(infobar_service) == nullptr) {
-    InstallableAmbientBadgeInfoBarDelegate::Create(web_contents(), GetWeakPtr(),
-                                                   GetAppName(), primary_icon_,
-                                                   manifest_.start_url);
+    InstallableAmbientBadgeInfoBarDelegate::Create(
+        web_contents(), weak_factory_.GetWeakPtr(), GetAppName(), primary_icon_,
+        manifest_.start_url);
   }
 }
 
@@ -377,6 +386,14 @@ void AppBannerManagerAndroid::HideAmbientBadge() {
     infobar_service->RemoveInfoBar(ambient_badge_infobar);
 }
 
+base::WeakPtr<AppBannerManager> AppBannerManagerAndroid::GetWeakPtr() {
+  return weak_factory_.GetWeakPtr();
+}
+
+void AppBannerManagerAndroid::InvalidateWeakPtrs() {
+  weak_factory_.InvalidateWeakPtrs();
+}
+
 // static
 AppBannerManager* AppBannerManager::FromWebContents(
     content::WebContents* web_contents) {

chrome/browser/banners/app_banner_manager_android.h

@@ -10,8 +10,10 @@
 
 #include "base/android/scoped_java_ref.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "base/strings/string16.h"
 #include "chrome/browser/banners/app_banner_manager.h"
+#include "chrome/browser/installable/installable_ambient_badge_infobar_delegate.h"
 #include "content/public/browser/web_contents_user_data.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "url/gurl.h"
@@ -41,6 +43,7 @@ class AppBannerUiDelegateAndroid;
 // together.
 class AppBannerManagerAndroid
     : public AppBannerManager,
+      public InstallableAmbientBadgeInfoBarDelegate::Client,
       public content::WebContentsUserData<AppBannerManagerAndroid> {
  public:
   explicit AppBannerManagerAndroid(content::WebContents* web_contents);
@@ -85,7 +88,7 @@ class AppBannerManagerAndroid
   // AppBannerManager overrides.
   void RequestAppBanner(const GURL& validated_url) override;
 
-  // InstallableAmbientBadgeInfoBarAndroid::Client overrides.
+  // InstallableAmbientBadgeInfoBarDelegate::Client overrides.
   void AddToHomescreenFromBadge() override;
   void BadgeDismissed() override;
 
@@ -97,13 +100,16 @@ class AppBannerManagerAndroid
                                    const GURL& validated_url,
                                    const GURL& start_url,
                                    const GURL& manifest_url) override;
-  InstallableParams ParamsToPerformInstallableCheck() override;
-  void PerformInstallableCheck() override;
-  void OnDidPerformInstallableCheck(const InstallableData& result) override;
-  void OnAppIconFetched(const SkBitmap& bitmap) override;
+  void PerformInstallableChecks() override;
+  InstallableParams ParamsToPerformInstallableWebAppCheck() override;
+  void PerformInstallableWebAppCheck() override;
+  void OnDidPerformInstallableWebAppCheck(
+      const InstallableData& result) override;
   void ResetCurrentPageData() override;
   void ShowBannerUi(WebappInstallSource install_source) override;
   void MaybeShowAmbientBadge() override;
+  base::WeakPtr<AppBannerManager> GetWeakPtr() override;
+  void InvalidateWeakPtrs() override;
 
  private:
   friend class content::WebContentsUserData<AppBannerManagerAndroid>;
@@ -115,6 +121,9 @@ class AppBannerManagerAndroid
   std::string ExtractQueryValueForName(const GURL& url,
                                        const std::string& name);
 
+  bool ShouldPerformInstallableNativeAppCheck();
+  void PerformInstallableNativeAppCheck();
+
   // Returns NO_ERROR_DETECTED if |platform|, |url|, and |id| are consistent and
   // can be used to query the Play Store for a native app. Otherwise returns the
   // error which prevents querying from taking place. The query may not
@@ -125,6 +134,11 @@ class AppBannerManagerAndroid
                                        const GURL& url,
                                        const std::string& id);
 
+  // Called when the download of a native app's icon is complete, as native
+  // banners use an icon provided from the Play Store rather than the web
+  // manifest.
+  void OnNativeAppIconFetched(const SkBitmap& bitmap);
+
   // Returns the appropriate app name based on whether we have a native/web app.
   base::string16 GetAppName() const override;
 
@@ -154,6 +168,8 @@ class AppBannerManagerAndroid
   // Whether WebAPKs can be installed.
   bool can_install_webapk_;
 
+  base::WeakPtrFactory<AppBannerManagerAndroid> weak_factory_{this};
+
   WEB_CONTENTS_USER_DATA_KEY_DECL();
 
   DISALLOW_COPY_AND_ASSIGN(AppBannerManagerAndroid);

chrome/browser/banners/app_banner_manager_browsertest.cc

@@ -111,6 +111,12 @@ class AppBannerManagerTest : public AppBannerManager {
     }
   }
 
+  base::WeakPtr<AppBannerManager> GetWeakPtr() override {
+    return weak_factory_.GetWeakPtr();
+  }
+
+  void InvalidateWeakPtrs() override { weak_factory_.InvalidateWeakPtrs(); }
+
  private:
   base::Closure on_done_;
 
@@ -121,6 +127,8 @@ class AppBannerManagerTest : public AppBannerManager {
   std::unique_ptr<bool> banner_shown_;
   std::unique_ptr<WebappInstallSource> install_source_;
 
+  base::WeakPtrFactory<AppBannerManagerTest> weak_factory_{this};
+
   DISALLOW_COPY_AND_ASSIGN(AppBannerManagerTest);
 };
 

chrome/browser/banners/app_banner_manager_desktop.cc

@@ -43,45 +43,12 @@ AppBannerManagerDesktop::AppBannerManagerDesktop(
 
 AppBannerManagerDesktop::~AppBannerManagerDesktop() { }
 
-void AppBannerManagerDesktop::CreateWebApp(WebappInstallSource install_source) {
-  content::WebContents* contents = web_contents();
-  DCHECK(contents);
-
-  // TODO(loyso): Take appropriate action if WebApps disabled for profile.
-  web_app::CreateWebAppFromManifest(
-      contents, install_source,
-      base::BindOnce(&AppBannerManager::DidFinishCreatingWebApp, GetWeakPtr()));
+base::WeakPtr<AppBannerManager> AppBannerManagerDesktop::GetWeakPtr() {
+  return weak_factory_.GetWeakPtr();
 }
 
-void AppBannerManagerDesktop::DidFinishCreatingWebApp(
-    const web_app::AppId& app_id,
-    web_app::InstallResultCode code) {
-  content::WebContents* contents = web_contents();
-  if (!contents)
-    return;
-
-  // BookmarkAppInstallManager returns kFailedUnknownReason for any error.
-  // We can't distinguish kUserInstallDeclined case so far.
-  // If kFailedUnknownReason, we assume that the confirmation dialog was
-  // cancelled. Alternatively, the web app installation may have failed, but
-  // we can't tell the difference here.
-  // TODO(crbug.com/789381): plumb through enough information to be able to
-  // distinguish between extension install failures and user-cancellations of
-  // the app install dialog.
-  if (code != web_app::InstallResultCode::kSuccess) {
-    SendBannerDismissed();
-    TrackUserResponse(USER_RESPONSE_WEB_APP_DISMISSED);
-    AppBannerSettingsHelper::RecordBannerDismissEvent(
-        contents, GetAppIdentifier(), AppBannerSettingsHelper::WEB);
-    return;
-  }
-
-  SendBannerAccepted();
-  AppBannerSettingsHelper::RecordBannerInstallEvent(
-      contents, GetAppIdentifier(), AppBannerSettingsHelper::WEB);
-
-  // OnInstall must be called last since it resets Mojo bindings.
-  OnInstall(false /* is_native app */, blink::kWebDisplayModeStandalone);
+void AppBannerManagerDesktop::InvalidateWeakPtrs() {
+  weak_factory_.InvalidateWeakPtrs();
 }
 
 bool AppBannerManagerDesktop::IsWebAppConsideredInstalled(
@@ -123,6 +90,48 @@ void AppBannerManagerDesktop::OnEngagementEvent(
   AppBannerManager::OnEngagementEvent(web_contents, url, score, type);
 }
 
+void AppBannerManagerDesktop::CreateWebApp(WebappInstallSource install_source) {
+  content::WebContents* contents = web_contents();
+  DCHECK(contents);
+
+  // TODO(loyso): Take appropriate action if WebApps disabled for profile.
+  web_app::CreateWebAppFromManifest(
+      contents, install_source,
+      base::BindOnce(&AppBannerManagerDesktop::DidFinishCreatingWebApp,
+                     weak_factory_.GetWeakPtr()));
+}
+
+void AppBannerManagerDesktop::DidFinishCreatingWebApp(
+    const web_app::AppId& app_id,
+    web_app::InstallResultCode code) {
+  content::WebContents* contents = web_contents();
+  if (!contents)
+    return;
+
+  // BookmarkAppInstallManager returns kFailedUnknownReason for any error.
+  // We can't distinguish kUserInstallDeclined case so far.
+  // If kFailedUnknownReason, we assume that the confirmation dialog was
+  // cancelled. Alternatively, the web app installation may have failed, but
+  // we can't tell the difference here.
+  // TODO(crbug.com/789381): plumb through enough information to be able to
+  // distinguish between extension install failures and user-cancellations of
+  // the app install dialog.
+  if (code != web_app::InstallResultCode::kSuccess) {
+    SendBannerDismissed();
+    TrackUserResponse(USER_RESPONSE_WEB_APP_DISMISSED);
+    AppBannerSettingsHelper::RecordBannerDismissEvent(
+        contents, GetAppIdentifier(), AppBannerSettingsHelper::WEB);
+    return;
+  }
+
+  SendBannerAccepted();
+  AppBannerSettingsHelper::RecordBannerInstallEvent(
+      contents, GetAppIdentifier(), AppBannerSettingsHelper::WEB);
+
+  // OnInstall must be called last since it resets Mojo bindings.
+  OnInstall(false /* is_native app */, blink::kWebDisplayModeStandalone);
+}
+
 WEB_CONTENTS_USER_DATA_KEY_IMPL(AppBannerManagerDesktop)
 
 }  // namespace banners

chrome/browser/banners/app_banner_manager_desktop.h

@@ -8,9 +8,14 @@
 #include <memory>
 
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "chrome/browser/banners/app_banner_manager.h"
 #include "content/public/browser/web_contents_user_data.h"
 
+namespace web_app {
+enum class InstallResultCode;
+}
+
 namespace banners {
 
 // Manages web app banners for desktop platforms.
@@ -30,12 +35,16 @@ class AppBannerManagerDesktop
   explicit AppBannerManagerDesktop(content::WebContents* web_contents);
 
   // AppBannerManager overrides.
-  void CreateWebApp(WebappInstallSource install_source) override;
-  void DidFinishCreatingWebApp(const web_app::AppId& app_id,
-                               web_app::InstallResultCode code) override;
+  base::WeakPtr<AppBannerManager> GetWeakPtr() override;
+  void InvalidateWeakPtrs() override;
+
+  // Called when the web app install initiated by a banner has completed.
+  virtual void DidFinishCreatingWebApp(const web_app::AppId& app_id,
+                                       web_app::InstallResultCode code);
 
  private:
   friend class content::WebContentsUserData<AppBannerManagerDesktop>;
+  friend class FakeAppBannerManagerDesktop;
 
   // AppBannerManager overrides.
   bool IsWebAppConsideredInstalled(content::WebContents* web_contents,
@@ -54,6 +63,10 @@ class AppBannerManagerDesktop
                          double score,
                          SiteEngagementService::EngagementType type) override;
 
+  void CreateWebApp(WebappInstallSource install_source);
+
+  base::WeakPtrFactory<AppBannerManagerDesktop> weak_factory_{this};
+
   WEB_CONTENTS_USER_DATA_KEY_DECL();
 
   DISALLOW_COPY_AND_ASSIGN(AppBannerManagerDesktop);

chrome/browser/banners/test_app_banner_manager_desktop.cc

@@ -57,9 +57,9 @@ void TestAppBannerManagerDesktop::OnDidGetManifest(
   if (!result.errors.empty())
     SetInstallable(false);
 }
-void TestAppBannerManagerDesktop::OnDidPerformInstallableCheck(
+void TestAppBannerManagerDesktop::OnDidPerformInstallableWebAppCheck(
     const InstallableData& result) {
-  AppBannerManagerDesktop::OnDidPerformInstallableCheck(result);
+  AppBannerManagerDesktop::OnDidPerformInstallableWebAppCheck(result);
   SetInstallable(result.errors.empty());
 }
 

chrome/browser/banners/test_app_banner_manager_desktop.h

@@ -34,7 +34,8 @@ class TestAppBannerManagerDesktop : public AppBannerManagerDesktop {
 
   // AppBannerManager:
   void OnDidGetManifest(const InstallableData& result) override;
-  void OnDidPerformInstallableCheck(const InstallableData& result) override;
+  void OnDidPerformInstallableWebAppCheck(
+      const InstallableData& result) override;
   void ResetCurrentPageData() override;
 
  private:

chrome/browser/ui/toolbar/app_menu_model.cc

@@ -124,7 +124,7 @@ base::Optional<base::string16> GetInstallPWAAppMenuItemName(Browser* browser) {
   if (!web_contents)
     return base::nullopt;
   base::string16 app_name =
-      banners::AppBannerManager::GetInstallableAppName(web_contents);
+      banners::AppBannerManager::GetInstallableWebAppName(web_contents);
   if (app_name.empty())
     return base::nullopt;
   return l10n_util::GetStringFUTF16(IDS_INSTALL_TO_OS_LAUNCH_SURFACE, app_name);

chrome/browser/ui/views/frame/browser_view.cc

@@ -3206,7 +3206,7 @@ void BrowserView::OnImmersiveModeControllerDestroyed() {
 
 ///////////////////////////////////////////////////////////////////////////////
 // BrowserView, banners::AppBannerManager::Observer implementation:
-void BrowserView::OnInstallabilityUpdated() {
+void BrowserView::OnInstallableWebAppStatusUpdated() {
   GetOmniboxPageActionIconContainer()->UpdatePageActionIcon(
       PageActionIconType::kPwaInstall);
 }

chrome/browser/ui/views/frame/browser_view.h

@@ -537,7 +537,7 @@ class BrowserView : public BrowserWindow,
   void OnImmersiveModeControllerDestroyed() override;
 
   // banners::AppBannerManager::Observer:
-  void OnInstallabilityUpdated() override;
+  void OnInstallableWebAppStatusUpdated() override;
 
   // Creates an accessible tab label for screen readers that includes the tab
   // status for the given tab index. This takes the form of

chrome/browser/ui/views/page_action/pwa_install_view.cc

@@ -35,7 +35,7 @@ bool PwaInstallView::Update() {
   if (!manager)
     return false;
 
-  bool is_probably_installable = manager->IsProbablyInstallable();
+  bool is_probably_installable = manager->IsProbablyInstallableWebApp();
   auto* tab_helper =
       web_app::WebAppTabHelperBase::FromWebContents(web_contents);
   bool is_installed = tab_helper && tab_helper->HasAssociatedApp();
@@ -74,5 +74,5 @@ base::string16 PwaInstallView::GetTextForTooltipAndAccessibleName() const {
     return base::string16();
   return l10n_util::GetStringFUTF16(
       IDS_OMNIBOX_PWA_INSTALL_ICON_TOOLTIP,
-      banners::AppBannerManager::GetInstallableAppName(web_contents));
+      banners::AppBannerManager::GetInstallableWebAppName(web_contents));
 }