Convert app banners to use Mojo.

chrome/browser/banners/app_banner_manager.cc

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 a46c9dd012bfe2d0e632f0e29f8051c6811b7250..c510be59d93c2480e9de3ab6cd3abfb2151a5231 100644
--- a/chrome/browser/banners/app_banner_manager.cc
+++ b/chrome/browser/banners/app_banner_manager.cc
@@ -4,26 +4,26 @@
 
 #include "chrome/browser/banners/app_banner_manager.h"
 
+#include <algorithm>
+
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/command_line.h"
+#include "base/lazy_instance.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/time/time.h"
 #include "chrome/browser/banners/app_banner_metrics.h"
 #include "chrome/browser/banners/app_banner_settings_helper.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/engagement/site_engagement_service.h"
-#include "chrome/browser/installable/installable_logging.h"
-#include "chrome/browser/installable/installable_manager.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_switches.h"
-#include "chrome/common/render_messages.h"
 #include "components/rappor/rappor_utils.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/origin_util.h"
-#include "third_party/WebKit/public/platform/modules/app_banner/WebAppBannerPromptReply.h"
+#include "services/shell/public/cpp/interface_provider.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
@@ -129,11 +129,31 @@ void AppBannerManager::RequestAppBanner(const GURL& validated_url,
   if (validated_url_.is_empty())
     validated_url_ = validated_url;
 
+  // Any existing binding is invalid if we re-request the banner.
+  if (binding_.is_bound())

[benwells, 2016/10/12 03:00:17]

Why does the service have to be invalidated after each request?

[dominickn, 2016/10/13 00:18:15]

AppBannerManager only ever manages one in-flight request at a time (we stop in
RequestAppBanner if we're already checking this URL). So if we've issued a
binding to an event previously, that binding is invalid once we get here.

+    binding_.Close();
+
   manager_->GetData(
       ParamsToGetManifest(),
       base::Bind(&AppBannerManager::OnDidGetManifest, GetWeakPtr()));
 }
 
+void AppBannerManager::SendBannerAccepted(int request_id) {
+  if (request_id != gCurrentRequestID)
+    return;
+
+  DCHECK(event_.is_bound());
+  event_->BannerAccepted(GetBannerType());
+}
+
+void AppBannerManager::SendBannerDismissed(int request_id) {
+  if (request_id != gCurrentRequestID)
+    return;
+
+  DCHECK(event_.is_bound());
+  event_->BannerDismissed();
+}
+
 base::Closure AppBannerManager::FetchWebappSplashScreenImageCallback(
     const std::string& webapp_id) {
   return base::Closure();
@@ -144,6 +164,7 @@ AppBannerManager::AppBannerManager(content::WebContents* web_contents)
       SiteEngagementObserver(nullptr),
       manager_(nullptr),
       event_request_id_(-1),
+      binding_(this),
       is_active_(false),
       banner_request_queued_(false),
       load_finished_(false),
@@ -329,9 +350,14 @@ void AppBannerManager::SendBannerPromptRequest() {
 
   TrackBeforeInstallEvent(BEFORE_INSTALL_EVENT_CREATED);
   event_request_id_ = ++gCurrentRequestID;
-  content::RenderFrameHost* frame = web_contents()->GetMainFrame();
-  frame->Send(new ChromeViewMsg_AppBannerPromptRequest(
-      frame->GetRoutingID(), event_request_id_, GetBannerType()));
+
+  web_contents()->GetMainFrame()->GetRemoteInterfaces()->GetInterface(
+      mojo::GetProxy(&client_));
+
+  client_->BannerPromptRequest(
+      binding_.CreateInterfacePtrAndBind(), mojo::GetProxy(&event_),
+      GetBannerType(),
+      base::Bind(&AppBannerManager::OnBannerPromptReply, GetWeakPtr()));
 }
 
 void AppBannerManager::DidStartNavigation(content::NavigationHandle* handle) {
@@ -460,31 +486,10 @@ bool AppBannerManager::CheckIfShouldShowBanner() {
   return false;
 }
 
-bool AppBannerManager::OnMessageReceived(
-    const IPC::Message& message,
-    content::RenderFrameHost* render_frame_host) {
-  bool handled = true;
-
-  IPC_BEGIN_MESSAGE_MAP_WITH_PARAM(AppBannerManager, message, render_frame_host)
-    IPC_MESSAGE_HANDLER(ChromeViewHostMsg_AppBannerPromptReply,
-                        OnBannerPromptReply)
-    IPC_MESSAGE_HANDLER(ChromeViewHostMsg_RequestShowAppBanner,
-                        OnRequestShowAppBanner)
-    IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
-
-  return handled;
-}
-
 void AppBannerManager::OnBannerPromptReply(
-    content::RenderFrameHost* render_frame_host,
-    int request_id,
-    blink::WebAppBannerPromptReply reply,
-    std::string referrer) {
+    blink::mojom::AppBannerPromptReply reply,
+    const std::string& referrer) {
   content::WebContents* contents = web_contents();
-  if (request_id != event_request_id_)

[benwells, 2016/10/12 03:00:17]

I assume this check here was to guarantee something to prevent races. How come
it is safe to delete it now?

[dominickn, 2016/10/13 00:18:15]

This is implicitly handled because OnBannerPromptReply can only be called as the
reply from calling client_->BannerPromptRequest, which sets event_request_id_
for this request. If we terminate the banner flow before the reply comes back,
Stop() calls InvalidateWeakPtrs which will prevent this method from being run.

Previously with IPCs, it was possible for the IPC to still be sent even though
the AppBannerManager had moved on to a new request because there wasn't a way to
tell the renderer to stop.

It may even be possible to get rid of the event_request_ids entirely from the
browser side too, though I don't want to do that in this CL.

-    return;
-
   // The renderer might have requested the prompt to be canceled.
   // They may request that it is redisplayed later, so don't Stop() here.
   // However, log that the cancelation was requested, so Stop() can be
@@ -494,7 +499,7 @@ void AppBannerManager::OnBannerPromptReply(
   // request may be received *before* the Cancel prompt reply (e.g. if redisplay
   // is requested in the beforeinstallprompt event handler).
   referrer_ = referrer;
-  if (reply == blink::WebAppBannerPromptReply::Cancel &&
+  if (reply == blink::mojom::AppBannerPromptReply::CANCEL &&
       !page_requested_prompt_) {
     TrackBeforeInstallEvent(BEFORE_INSTALL_EVENT_PREVENT_DEFAULT_CALLED);
     was_canceled_by_page_ = true;
@@ -525,14 +530,11 @@ void AppBannerManager::OnBannerPromptReply(
   is_active_ = false;
 }
 
-void AppBannerManager::OnRequestShowAppBanner(
-    content::RenderFrameHost* render_frame_host,
-    int request_id) {
+void AppBannerManager::DisplayAppBanner() {
   if (was_canceled_by_page_) {
     // Simulate a non-canceled OnBannerPromptReply to show the delayed banner.
     // Don't reset |was_canceled_by_page_| yet for metrics purposes.
-    OnBannerPromptReply(render_frame_host, request_id,
-                        blink::WebAppBannerPromptReply::None, referrer_);
+    OnBannerPromptReply(blink::mojom::AppBannerPromptReply::NONE, referrer_);
   } else {
     // Log that the prompt request was made for when we get the prompt reply.
     page_requested_prompt_ = true;