Implement "appinstalled" event on Android.

chrome/browser/banners/app_banner_manager.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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/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/profiles/profile.h"
 #include "chrome/common/chrome_switches.h"
 #include "components/rappor/public/rappor_utils.h"
 #include "components/rappor/rappor_service_impl.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 "mojo/public/cpp/bindings/interface_request.h"

[dominickn, 2016/12/09 06:08:17]

Is this include necessary? We're already calling GetInterface in here without
it.

[Matt Giuca, 2016/12/12 04:28:19]

Done.

 #include "services/service_manager/public/cpp/interface_provider.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 
 namespace {
 
 int gCurrentRequestID = -1;
 int gTimeDeltaInDaysForTesting = 0;
 
@@ skipping 89 matching lines @@
 
   // Any existing binding is invalid when we request a new banner.
   if (binding_.is_bound())
     binding_.Close();
 
   manager_->GetData(
       ParamsToGetManifest(),
       base::Bind(&AppBannerManager::OnDidGetManifest, GetWeakPtr()));
 }
 
+void AppBannerManager::OnInstall() {
+  if (!installation_service_) {

[dominickn, 2016/12/09 06:08:17]

As noted in the header file, just made this a local variable in here.

[Matt Giuca, 2016/12/12 04:28:20]

Are there performance issues about calling GetMainFrame, GetRemoteInterfaces,
GetInterface and GetProxy every time?

(Can you link to a precedent for not caching this?)

[dominickn, 2016/12/12 05:03:30]

1) this isn't a frequently called method, but adding a member variable bloats
the size of this object, one of which lives on every WebContents. I don't think
performance overhead is really an issue here - I'd be surprised if users often
added to homescreen from the exact same tab multiple times.

2) some example of not caching include:

chrome/browser/android/contextualsearch/contextual_search_manager.cc
chrome/browser/task_manager/providers/child_process_task.cc
chrome/renderer/chrome_content_renderer_client.cc
chrome/renderer/chrome_render_thread_observer.cc

And app_banner_manager.cc too.

3) make it a local variable here and in AddToHomescreenManager. :)

[Matt Giuca, 2016/12/14 03:39:40]

Done.

+    web_contents()->GetMainFrame()->GetRemoteInterfaces()->GetInterface(
+        mojo::GetProxy(&installation_service_));
+    DCHECK(installation_service_);
+  }
+  installation_service_->OnInstall();
+}
+
 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)
@@ skipping 395 matching lines @@
     // Simulate a non-canceled OnBannerPromptReply to show the delayed banner.
     // Don't reset |was_canceled_by_page_| yet for metrics purposes.
     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;
   }
 }
 
 }  // namespace banners