Commit c6292001 authored by Dominick Ng's avatar Dominick Ng Committed by Commit Bot

Remove the need to store an AppBannerControllerPtr member on AppBannerManager.

This CL passes a local AppBannerControllerPtr variable into the callback
sent through Mojo. This keeps the Mojo connection alive and removes the
need to store an AppBannerControllerPtr member.

BUG=None

Change-Id: Id67ba09ee438f9b455a43d4c2b89b6f8f75b27a8
Reviewed-on: https://chromium-review.googlesource.com/828240Reviewed-by: default avatarBen Wells <benwells@chromium.org>
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524316}
parent 90ebce39
...@@ -409,14 +409,16 @@ void AppBannerManager::SendBannerPromptRequest() { ...@@ -409,14 +409,16 @@ void AppBannerManager::SendBannerPromptRequest() {
// Any existing binding is invalid when we send a new beforeinstallprompt. // Any existing binding is invalid when we send a new beforeinstallprompt.
ResetBindings(); ResetBindings();
blink::mojom::AppBannerControllerPtr controller;
web_contents()->GetMainFrame()->GetRemoteInterfaces()->GetInterface( web_contents()->GetMainFrame()->GetRemoteInterfaces()->GetInterface(
mojo::MakeRequest(&controller_)); mojo::MakeRequest(&controller));
blink::mojom::AppBannerServicePtr banner_proxy; blink::mojom::AppBannerServicePtr banner_proxy;
binding_.Bind(mojo::MakeRequest(&banner_proxy)); binding_.Bind(mojo::MakeRequest(&banner_proxy));
controller_->BannerPromptRequest( controller->BannerPromptRequest(
std::move(banner_proxy), mojo::MakeRequest(&event_), {GetBannerType()}, std::move(banner_proxy), mojo::MakeRequest(&event_), {GetBannerType()},
base::Bind(&AppBannerManager::OnBannerPromptReply, GetWeakPtr())); base::BindOnce(&AppBannerManager::OnBannerPromptReply, GetWeakPtr(),
base::Passed(&controller)));
} }
void AppBannerManager::UpdateState(State state) { void AppBannerManager::UpdateState(State state) {
...@@ -541,7 +543,6 @@ bool AppBannerManager::IsExperimentalAppBannersEnabled() { ...@@ -541,7 +543,6 @@ bool AppBannerManager::IsExperimentalAppBannersEnabled() {
void AppBannerManager::ResetBindings() { void AppBannerManager::ResetBindings() {
binding_.Close(); binding_.Close();
controller_.reset();
event_.reset(); event_.reset();
} }
...@@ -610,12 +611,9 @@ bool AppBannerManager::CheckIfShouldShowBanner() { ...@@ -610,12 +611,9 @@ bool AppBannerManager::CheckIfShouldShowBanner() {
} }
void AppBannerManager::OnBannerPromptReply( void AppBannerManager::OnBannerPromptReply(
blink::mojom::AppBannerControllerPtr controller,
blink::mojom::AppBannerPromptReply reply, blink::mojom::AppBannerPromptReply reply,
const std::string& referrer) { const std::string& referrer) {
// We don't need the controller any more, so reset it so the Blink-side object
// is destroyed.
controller_.reset();
// The renderer might have requested the prompt to be canceled. They may // The renderer might have requested the prompt to be canceled. They may
// request that it is redisplayed later, so don't Terminate() here. However, // request that it is redisplayed later, so don't Terminate() here. However,
// log that the cancelation was requested, so Terminate() can be called if a // log that the cancelation was requested, so Terminate() can be called if a
......
...@@ -290,8 +290,10 @@ class AppBannerManager : public content::WebContentsObserver, ...@@ -290,8 +290,10 @@ class AppBannerManager : public content::WebContentsObserver,
// Called after the manager sends a message to the renderer regarding its // Called after the manager sends a message to the renderer regarding its
// intention to show a prompt. The renderer will send a message back with the // intention to show a prompt. The renderer will send a message back with the
// opportunity to cancel. // opportunity to cancel.
virtual void OnBannerPromptReply(blink::mojom::AppBannerPromptReply reply, virtual void OnBannerPromptReply(
const std::string& referrer); blink::mojom::AppBannerControllerPtr controller,
blink::mojom::AppBannerPromptReply reply,
const std::string& referrer);
// Does the non-platform specific parts of showing the app banner. // Does the non-platform specific parts of showing the app banner.
void ShowBanner(); void ShowBanner();
...@@ -323,7 +325,6 @@ class AppBannerManager : public content::WebContentsObserver, ...@@ -323,7 +325,6 @@ class AppBannerManager : public content::WebContentsObserver,
// Mojo bindings and interface pointers. // Mojo bindings and interface pointers.
mojo::Binding<blink::mojom::AppBannerService> binding_; mojo::Binding<blink::mojom::AppBannerService> binding_;
blink::mojom::AppBannerEventPtr event_; blink::mojom::AppBannerEventPtr event_;
blink::mojom::AppBannerControllerPtr controller_;
// If a banner is requested before the page has finished loading, defer // If a banner is requested before the page has finished loading, defer
// triggering the pipeline until the load is complete. // triggering the pipeline until the load is complete.
......
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#include <utility>
#include <vector> #include <vector>
#include "base/bind.h" #include "base/bind.h"
...@@ -116,9 +117,11 @@ class AppBannerManagerTest : public AppBannerManager { ...@@ -116,9 +117,11 @@ class AppBannerManagerTest : public AppBannerManager {
} }
} }
void OnBannerPromptReply(blink::mojom::AppBannerPromptReply reply, void OnBannerPromptReply(blink::mojom::AppBannerControllerPtr controller,
blink::mojom::AppBannerPromptReply reply,
const std::string& referrer) override { const std::string& referrer) override {
AppBannerManager::OnBannerPromptReply(reply, referrer); AppBannerManager::OnBannerPromptReply(std::move(controller), reply,
referrer);
if (on_banner_prompt_reply_) { if (on_banner_prompt_reply_) {
base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
on_banner_prompt_reply_); on_banner_prompt_reply_);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment