Convert app banners to use Mojo.

third_party/WebKit/Source/web/WebLocalFrameImpl.cpp

Index: third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
diff --git a/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp b/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
index 2c3891b0241afdff6d33ae526e37e046c29b42e1..3e2adbe03aa4c2cb0ff35709b99815485bc22a5f 100644
--- a/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
+++ b/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
@@ -177,6 +177,7 @@
 #include "platform/weborigin/SchemeRegistry.h"
 #include "platform/weborigin/SecurityPolicy.h"
 #include "public/platform/InterfaceProvider.h"
+#include "public/platform/InterfaceRegistry.h"
 #include "public/platform/WebDoubleSize.h"
 #include "public/platform/WebFloatPoint.h"
 #include "public/platform/WebFloatRect.h"
@@ -228,6 +229,7 @@
 #include "wtf/PtrUtil.h"
 #include <algorithm>
 #include <memory>
+#include <utility>
 
 namespace blink {
 
@@ -1614,11 +1616,16 @@ void WebLocalFrameImpl::initializeCoreFrame(FrameHost* host,
   // during init(). Note that this may dispatch JS events; the frame may be
   // detached after init() returns.
   frame()->init();
-  if (frame() &&
-      frame()->loader().stateMachine()->isDisplayingInitialEmptyDocument() &&
-      !parent() && !opener() &&
-      frame()->settings()->shouldReuseGlobalForUnownedMainFrame())
-    frame()->document()->getSecurityOrigin()->grantUniversalAccess();
+  if (frame()) {
+    if (frame()->loader().stateMachine()->isDisplayingInitialEmptyDocument() &&
+        !parent() && !opener() &&
+        frame()->settings()->shouldReuseGlobalForUnownedMainFrame()) {
+      frame()->document()->getSecurityOrigin()->grantUniversalAccess();
+    }
+
+    frame()->interfaceRegistry()->addInterface(
+        WTF::bind(&AppBannerController::create, wrapWeakPersistent(frame())));

[haraken, 2016/10/19 09:57:05]

I'm just curious but what happens if we use wrapPersistent(frame()) here?

If it leaks the frame, I'm worried that people may misuse wrapPersistent and
leak the frame in the future.

Maybe should we have logic to clear the interface registry when the frame gets
detached?

[dominickn, 2016/10/20 04:54:58]

Changed to wrapPersistent. Seems okay...

[haraken, 2016/10/20 08:43:18]

Are you sure that the persistents are cleared before the frame gets detached?

If the persistents are not cleared, it will cause memory leaks.

[dominickn, 2016/10/20 11:41:34]

Do you have a leak bot of some kind to point me towards? I don't know this stuff
nearly well enough to answer your question. Looking at crrev.com/2401333002, the
InterfaceRegistry lives on a WebFrameClient, but about as far as I can
competently answer.

Is there some reason that this CL can't use wrapWeakPersistent? If there is a
case for misusing wrapPersistent, then I think the fix for that should happen
separately to this.

[haraken, 2016/10/20 15:51:23]

We have a leak detector bot on ToT. If you leak a frame, it will be caught.
It totally depends on what you want to do. If you're expecting that the frame
should be alive while the Mojo callback is outstanding, it should be Persistent.
Otherwise (if you're okay with not running the Mojo callback after the frame is
gone), it should be WeakPersistent.

Also I agree with you that my question is rather about the semantics of
InterfaceRegistry than what you do in this CL. Maybe sammc@ has some thought.

[dominickn, 2016/10/20 23:41:18]

The correct thing to do then is to make it weakPersistent, it doesn't make sense
for this to run with no frame there (or if the frame has already been freed).
This shouldn't keep the frame alive.

+  }
 }
 
 LocalFrame* WebLocalFrameImpl::createChildFrame(
@@ -2089,17 +2096,6 @@ void WebLocalFrameImpl::sendOrientationChangeEvent() {
     frame()->localDOMWindow()->sendOrientationChangeEvent();
 }
 
-void WebLocalFrameImpl::willShowInstallBannerPrompt(
-    int requestId,
-    const WebVector<WebString>& platforms,
-    WebAppBannerPromptReply* reply) {
-  if (!frame())
-    return;
-
-  AppBannerController::willShowInstallBannerPrompt(
-      requestId, client()->appBannerClient(), frame(), platforms, reply);
-}
-
 void WebLocalFrameImpl::requestRunTask(WebSuspendableTask* task) const {
   DCHECK(frame());
   DCHECK(frame()->document());