Commit 0b93e0d3 authored by dominickn's avatar dominickn Committed by Commit bot

Log messages regarding why app banners aren't displayed to the console

App banners require several engagement, service worker, and manifest
checks to pass before being displayed. This allows developers to see
in the console the reasons why banners are not displayed, given the
switch --bypass-app-banner-engagement-checks on chrome startup

BUG=458866

Review URL: https://codereview.chromium.org/1129103003

Cr-Commit-Position: refs/heads/master@{#329751}
parent 2275446a
...@@ -51,7 +51,7 @@ void AppBannerManagerAndroid::ReplaceWebContents(JNIEnv* env, ...@@ -51,7 +51,7 @@ void AppBannerManagerAndroid::ReplaceWebContents(JNIEnv* env,
bool AppBannerManagerAndroid::HandleNonWebApp(const std::string& platform, bool AppBannerManagerAndroid::HandleNonWebApp(const std::string& platform,
const GURL& url, const GURL& url,
const std::string& id) { const std::string& id) {
if (platform != kPlayPlatform || id.empty()) if (!CheckPlatformAndId(platform, id))
return false; return false;
banners::TrackDisplayEvent(DISPLAY_EVENT_BANNER_REQUESTED); banners::TrackDisplayEvent(DISPLAY_EVENT_BANNER_REQUESTED);
...@@ -71,6 +71,33 @@ bool AppBannerManagerAndroid::HandleNonWebApp(const std::string& platform, ...@@ -71,6 +71,33 @@ bool AppBannerManagerAndroid::HandleNonWebApp(const std::string& platform,
return true; return true;
} }
bool AppBannerManagerAndroid::CheckPlatformAndId(const std::string& platform,
const std::string& id) {
if (platform != kPlayPlatform) {
banners::OutputDeveloperDebugMessage(
web_contents(), platform + banners::kIgnoredNotSupportedOnAndroid);
return false;
}
if (id.empty()) {
banners::OutputDeveloperDebugMessage(web_contents(), banners::kIgnoredNoId);
return false;
}
return true;
}
bool AppBannerManagerAndroid::CheckFetcherMatchesContents() {
if (!web_contents()) {
return false;
}
if (!data_fetcher() ||
data_fetcher()->validated_url() != web_contents()->GetURL()) {
banners::OutputDeveloperNotShownMessage(
web_contents(), banners::kUserNavigatedBeforeBannerShown);
return false;
}
return true;
}
AppBannerDataFetcher* AppBannerManagerAndroid::CreateAppBannerDataFetcher( AppBannerDataFetcher* AppBannerManagerAndroid::CreateAppBannerDataFetcher(
base::WeakPtr<Delegate> weak_delegate, base::WeakPtr<Delegate> weak_delegate,
const int ideal_icon_size) { const int ideal_icon_size) {
...@@ -84,10 +111,8 @@ bool AppBannerManagerAndroid::OnAppDetailsRetrieved(JNIEnv* env, ...@@ -84,10 +111,8 @@ bool AppBannerManagerAndroid::OnAppDetailsRetrieved(JNIEnv* env,
jstring japp_title, jstring japp_title,
jstring japp_package, jstring japp_package,
jstring jicon_url) { jstring jicon_url) {
if (!data_fetcher() || !web_contents() if (!CheckFetcherMatchesContents())
|| data_fetcher()->validated_url() != web_contents()->GetURL()) {
return false; return false;
}
base::android::ScopedJavaLocalRef<jobject> native_app_data; base::android::ScopedJavaLocalRef<jobject> native_app_data;
native_app_data.Reset(env, japp_data); native_app_data.Reset(env, japp_data);
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/android/jni_weak_ref.h" #include "base/android/jni_weak_ref.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "chrome/browser/android/banners/app_banner_data_fetcher_android.h" #include "chrome/browser/android/banners/app_banner_data_fetcher_android.h"
#include "chrome/browser/banners/app_banner_debug_log.h"
#include "chrome/browser/banners/app_banner_manager.h" #include "chrome/browser/banners/app_banner_manager.h"
namespace banners { namespace banners {
...@@ -58,6 +59,10 @@ class AppBannerManagerAndroid : public AppBannerManager { ...@@ -58,6 +59,10 @@ class AppBannerManagerAndroid : public AppBannerManager {
const GURL& url, const GURL& url,
const std::string& id) override; const std::string& id) override;
bool CheckPlatformAndId(const std::string& platform, const std::string& id);
bool CheckFetcherMatchesContents();
// AppBannerManager on the Java side. // AppBannerManager on the Java side.
JavaObjectWeakGlobalRef weak_java_banner_view_manager_; JavaObjectWeakGlobalRef weak_java_banner_view_manager_;
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "chrome/browser/banners/app_banner_debug_log.h"
#include "chrome/browser/banners/app_banner_metrics.h" #include "chrome/browser/banners/app_banner_metrics.h"
#include "chrome/browser/banners/app_banner_settings_helper.h" #include "chrome/browser/banners/app_banner_settings_helper.h"
#include "chrome/browser/bitmap_fetcher/bitmap_fetcher.h" #include "chrome/browser/bitmap_fetcher/bitmap_fetcher.h"
...@@ -140,13 +141,15 @@ void AppBannerDataFetcher::OnBannerPromptReply( ...@@ -140,13 +141,15 @@ void AppBannerDataFetcher::OnBannerPromptReply(
int request_id, int request_id,
blink::WebAppBannerPromptReply reply) { blink::WebAppBannerPromptReply reply) {
content::WebContents* web_contents = GetWebContents(); content::WebContents* web_contents = GetWebContents();
if (!is_active_ || !web_contents || request_id != event_request_id_) { if (!CheckFetcherIsStillAlive(web_contents) ||
request_id != event_request_id_) {
Cancel(); Cancel();
return; return;
} }
// The renderer might have requested the prompt to be canceled. // The renderer might have requested the prompt to be canceled.
if (reply == blink::WebAppBannerPromptReply::Cancel) { if (reply == blink::WebAppBannerPromptReply::Cancel) {
OutputDeveloperNotShownMessage(web_contents, kRendererRequestCancel);
Cancel(); Cancel();
return; return;
} }
...@@ -231,7 +234,12 @@ void AppBannerDataFetcher::RecordDidShowBanner(const std::string& event_name) { ...@@ -231,7 +234,12 @@ void AppBannerDataFetcher::RecordDidShowBanner(const std::string& event_name) {
void AppBannerDataFetcher::OnDidGetManifest( void AppBannerDataFetcher::OnDidGetManifest(
const content::Manifest& manifest) { const content::Manifest& manifest) {
content::WebContents* web_contents = GetWebContents(); content::WebContents* web_contents = GetWebContents();
if (!is_active_ || !web_contents || manifest.IsEmpty()) { if (!CheckFetcherIsStillAlive(web_contents)) {
Cancel();
return;
}
if (manifest.IsEmpty()) {
OutputDeveloperNotShownMessage(web_contents, kManifestEmpty);
Cancel(); Cancel();
return; return;
} }
...@@ -246,7 +254,7 @@ void AppBannerDataFetcher::OnDidGetManifest( ...@@ -246,7 +254,7 @@ void AppBannerDataFetcher::OnDidGetManifest(
} }
} }
if (!IsManifestValidForWebApp(manifest)) { if (!IsManifestValidForWebApp(manifest, web_contents)) {
Cancel(); Cancel();
return; return;
} }
...@@ -274,7 +282,7 @@ void AppBannerDataFetcher::OnDidGetManifest( ...@@ -274,7 +282,7 @@ void AppBannerDataFetcher::OnDidGetManifest(
void AppBannerDataFetcher::OnDidCheckHasServiceWorker( void AppBannerDataFetcher::OnDidCheckHasServiceWorker(
bool has_service_worker) { bool has_service_worker) {
content::WebContents* web_contents = GetWebContents(); content::WebContents* web_contents = GetWebContents();
if (!is_active_ || !web_contents) { if (!CheckFetcherIsStillAlive(web_contents)) {
Cancel(); Cancel();
return; return;
} }
...@@ -290,8 +298,10 @@ void AppBannerDataFetcher::OnDidCheckHasServiceWorker( ...@@ -290,8 +298,10 @@ void AppBannerDataFetcher::OnDidCheckHasServiceWorker(
FetchIcon(icon_url); FetchIcon(icon_url);
return; return;
} }
OutputDeveloperNotShownMessage(web_contents, kCannotDetermineBestIcon);
} else { } else {
TrackDisplayEvent(DISPLAY_EVENT_LACKS_SERVICE_WORKER); TrackDisplayEvent(DISPLAY_EVENT_LACKS_SERVICE_WORKER);
OutputDeveloperNotShownMessage(web_contents, kNoMatchingServiceWorker);
} }
Cancel(); Cancel();
...@@ -307,13 +317,21 @@ void AppBannerDataFetcher::OnFetchComplete(const GURL& url, ...@@ -307,13 +317,21 @@ void AppBannerDataFetcher::OnFetchComplete(const GURL& url,
void AppBannerDataFetcher::ShowBanner(const SkBitmap* icon) { void AppBannerDataFetcher::ShowBanner(const SkBitmap* icon) {
content::WebContents* web_contents = GetWebContents(); content::WebContents* web_contents = GetWebContents();
if (!is_active_ || !web_contents || !icon) { if (!CheckFetcherIsStillAlive(web_contents)) {
Cancel();
return;
}
if (!icon) {
OutputDeveloperNotShownMessage(web_contents, kNoIconAvailable);
Cancel(); Cancel();
return; return;
} }
RecordCouldShowBanner(); RecordCouldShowBanner();
if (!CheckIfShouldShowBanner()) { if (!CheckIfShouldShowBanner()) {
// At this point, the only possible case is that the banner has been added
// to the homescreen, given all of the other checks that have been made.
OutputDeveloperNotShownMessage(web_contents, kBannerAlreadyAdded);
Cancel(); Cancel();
return; return;
} }
...@@ -345,17 +363,40 @@ bool AppBannerDataFetcher::CheckIfShouldShowBanner() { ...@@ -345,17 +363,40 @@ bool AppBannerDataFetcher::CheckIfShouldShowBanner() {
web_contents, validated_url_, GetAppIdentifier(), GetCurrentTime()); web_contents, validated_url_, GetAppIdentifier(), GetCurrentTime());
} }
bool AppBannerDataFetcher::CheckFetcherIsStillAlive(
content::WebContents* web_contents) {
if (!is_active_) {
OutputDeveloperNotShownMessage(web_contents,
kUserNavigatedBeforeBannerShown);
return false;
}
if (!web_contents) {
return false; // We cannot show a message if |web_contents| is null
}
return true;
}
// static // static
bool AppBannerDataFetcher::IsManifestValidForWebApp( bool AppBannerDataFetcher::IsManifestValidForWebApp(
const content::Manifest& manifest) { const content::Manifest& manifest,
if (manifest.IsEmpty()) content::WebContents* web_contents) {
if (manifest.IsEmpty()) {
OutputDeveloperNotShownMessage(web_contents, kManifestEmpty);
return false; return false;
if (!manifest.start_url.is_valid()) }
if (!manifest.start_url.is_valid()) {
OutputDeveloperNotShownMessage(web_contents, kStartURLNotValid);
return false; return false;
if (manifest.name.is_null() && manifest.short_name.is_null()) }
if (manifest.name.is_null() && manifest.short_name.is_null()) {
OutputDeveloperNotShownMessage(web_contents,
kManifestMissingNameOrShortName);
return false; return false;
if (!DoesManifestContainRequiredIcon(manifest)) }
if (!DoesManifestContainRequiredIcon(manifest)) {
OutputDeveloperNotShownMessage(web_contents, kManifestMissingSuitableIcon);
return false; return false;
}
return true; return true;
} }
......
...@@ -138,9 +138,14 @@ class AppBannerDataFetcher ...@@ -138,9 +138,14 @@ class AppBannerDataFetcher
// Returns whether the banner should be shown. // Returns whether the banner should be shown.
bool CheckIfShouldShowBanner(); bool CheckIfShouldShowBanner();
// Returns whether the fetcher is active and web contents have not been
// closed.
bool CheckFetcherIsStillAlive(content::WebContents* web_contents);
// Returns whether the given Manifest is following the requirements to show // Returns whether the given Manifest is following the requirements to show
// a web app banner. // a web app banner.
static bool IsManifestValidForWebApp(const content::Manifest& manifest); static bool IsManifestValidForWebApp(const content::Manifest& manifest,
content::WebContents* web_contents);
const int ideal_icon_size_; const int ideal_icon_size_;
const base::WeakPtr<Delegate> weak_delegate_; const base::WeakPtr<Delegate> weak_delegate_;
......
...@@ -33,7 +33,10 @@ class AppBannerDataFetcherUnitTest : public testing::Test { ...@@ -33,7 +33,10 @@ class AppBannerDataFetcherUnitTest : public testing::Test {
} }
static bool IsManifestValid(const content::Manifest& manifest) { static bool IsManifestValid(const content::Manifest& manifest) {
return AppBannerDataFetcher::IsManifestValidForWebApp(manifest); // The second argument is the web_contents pointer, which is used for
// developer debug logging to the console. The logging is skipped inside the
// method if a null web_contents pointer is provided, so this is safe.
return AppBannerDataFetcher::IsManifestValidForWebApp(manifest, nullptr);
} }
}; };
......
// 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_debug_log.h"
#include "base/command_line.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/render_messages.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_contents_observer.h"
namespace banners {
const char kRendererRequestCancel[] =
"renderer has requested the banner prompt be cancelled";
const char kManifestEmpty[] = "manifest is empty";
const char kCannotDetermineBestIcon[] =
"could not determine the best icon to use";
const char kNoMatchingServiceWorker[] =
"no matching service worker detected. You may need to reload the page, or "
"check that the service worker for the current page also controls the "
"start URL from the manifest";
const char kNoIconAvailable[] = "no icon available to display";
const char kBannerAlreadyAdded[] =
"the banner has already been added to the homescreen";
const char kUserNavigatedBeforeBannerShown[] =
"the user navigated before the banner could be shown";
const char kStartURLNotValid[] = "start URL in manifest is not valid";
const char kManifestMissingNameOrShortName[] =
"one of manifest name or short name must be specified";
const char kManifestMissingSuitableIcon[] =
"manifest does not contain a suitable icon - PNG format of at least "
"144x144px is required";
const char kNotServedFromSecureOrigin[] =
"page not served from a secure origin";
// The leading space is intentional as another string is prepended.
const char kIgnoredNotSupportedOnAndroid[] =
" application ignored: not supported on Android";
const char kIgnoredNoId[] = "play application ignored: no id provided";
void OutputDeveloperNotShownMessage(content::WebContents* web_contents,
const std::string& message) {
OutputDeveloperDebugMessage(web_contents, "not shown: " + message);
}
void OutputDeveloperDebugMessage(content::WebContents* web_contents,
const std::string& message) {
std::string log_message = "App banner " + message;
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kBypassAppBannerEngagementChecks) && web_contents) {
web_contents->GetMainFrame()->Send(
new ChromeViewMsg_AppBannerDebugMessageRequest(
web_contents->GetMainFrame()->GetRoutingID(), log_message));
}
}
} // namespace banners
// 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.
#ifndef CHROME_BROWSER_BANNERS_APP_BANNER_DEBUG_LOG_H_
#define CHROME_BROWSER_BANNERS_APP_BANNER_DEBUG_LOG_H_
#include <string>
namespace content {
class WebContents;
} // namespace content
namespace banners {
// Error message strings used to notify developers via the console.
extern const char kRendererRequestCancel[];
extern const char kManifestEmpty[];
extern const char kCannotDetermineBestIcon[];
extern const char kNoMatchingServiceWorker[];
extern const char kNoIconAvailable[];
extern const char kBannerAlreadyAdded[];
extern const char kUserNavigatedBeforeBannerShown[];
extern const char kStartURLNotValid[];
extern const char kManifestMissingNameOrShortName[];
extern const char kManifestMissingSuitableIcon[];
extern const char kNotServedFromSecureOrigin[];
extern const char kIgnoredNotSupportedOnAndroid[];
extern const char kIgnoredNoId[];
// Logs a message to the main console if a banner could not be shown
// and the engagement checks have been bypassed.
void OutputDeveloperNotShownMessage(content::WebContents* web_contents,
const std::string& message);
// Logs a debugging message to the main console if the engagement checks have
// been bypassed.
void OutputDeveloperDebugMessage(content::WebContents* web_contents,
const std::string& message);
} // namespace banners
#endif // CHROME_BROWSER_BANNERS_APP_BANNER_DEBUG_LOG_H_
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include "base/metrics/field_trial.h" #include "base/metrics/field_trial.h"
#include "chrome/browser/banners/app_banner_data_fetcher.h" #include "chrome/browser/banners/app_banner_data_fetcher.h"
#include "chrome/browser/banners/app_banner_debug_log.h"
#include "chrome/browser/banners/app_banner_settings_helper.h" #include "chrome/browser/banners/app_banner_settings_helper.h"
#include "content/public/browser/navigation_details.h" #include "content/public/browser/navigation_details.h"
#include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_frame_host.h"
...@@ -50,8 +51,11 @@ void AppBannerManager::DidFinishLoad( ...@@ -50,8 +51,11 @@ void AppBannerManager::DidFinishLoad(
// A secure origin is required to show banners, so exit early if we see the // A secure origin is required to show banners, so exit early if we see the
// URL is invalid. // URL is invalid.
if (!content::IsOriginSecure(validated_url) && !gDisableSecureCheckForTesting) if (!content::IsOriginSecure(validated_url) &&
!gDisableSecureCheckForTesting) {
OutputDeveloperNotShownMessage(web_contents(), kNotServedFromSecureOrigin);
return; return;
}
// Kick off the data retrieval pipeline. // Kick off the data retrieval pipeline.
data_fetcher_ = CreateAppBannerDataFetcher(weak_factory_.GetWeakPtr(), data_fetcher_ = CreateAppBannerDataFetcher(weak_factory_.GetWeakPtr(),
......
...@@ -210,6 +210,8 @@ ...@@ -210,6 +210,8 @@
'browser/autofill/validation_rules_storage_factory.h', 'browser/autofill/validation_rules_storage_factory.h',
'browser/banners/app_banner_data_fetcher.cc', 'browser/banners/app_banner_data_fetcher.cc',
'browser/banners/app_banner_data_fetcher.h', 'browser/banners/app_banner_data_fetcher.h',
'browser/banners/app_banner_debug_log.cc',
'browser/banners/app_banner_debug_log.h',
'browser/banners/app_banner_manager.cc', 'browser/banners/app_banner_manager.cc',
'browser/banners/app_banner_manager.h', 'browser/banners/app_banner_manager.h',
'browser/banners/app_banner_metrics.cc', 'browser/banners/app_banner_metrics.cc',
......
...@@ -462,6 +462,11 @@ IPC_MESSAGE_ROUTED2(ChromeViewMsg_AppBannerAccepted, ...@@ -462,6 +462,11 @@ IPC_MESSAGE_ROUTED2(ChromeViewMsg_AppBannerAccepted,
IPC_MESSAGE_ROUTED1(ChromeViewMsg_AppBannerDismissed, IPC_MESSAGE_ROUTED1(ChromeViewMsg_AppBannerDismissed,
int32_t /* request_id */) int32_t /* request_id */)
// Asks the renderer to log a debug message to console regarding an
// app banner.
IPC_MESSAGE_ROUTED1(ChromeViewMsg_AppBannerDebugMessageRequest,
std::string /* message */)
// Notification that the page has an OpenSearch description document // Notification that the page has an OpenSearch description document
// associated with it. // associated with it.
IPC_MESSAGE_ROUTED3(ChromeViewHostMsg_PageHasOSDD, IPC_MESSAGE_ROUTED3(ChromeViewHostMsg_PageHasOSDD,
......
...@@ -100,6 +100,8 @@ bool ChromeRenderFrameObserver::OnMessageReceived(const IPC::Message& message) { ...@@ -100,6 +100,8 @@ bool ChromeRenderFrameObserver::OnMessageReceived(const IPC::Message& message) {
OnPrintNodeUnderContextMenu) OnPrintNodeUnderContextMenu)
IPC_MESSAGE_HANDLER(ChromeViewMsg_AppBannerPromptRequest, IPC_MESSAGE_HANDLER(ChromeViewMsg_AppBannerPromptRequest,
OnAppBannerPromptRequest) OnAppBannerPromptRequest)
IPC_MESSAGE_HANDLER(ChromeViewMsg_AppBannerDebugMessageRequest,
OnAppBannerDebugMessageRequest)
IPC_MESSAGE_UNHANDLED(handled = false) IPC_MESSAGE_UNHANDLED(handled = false)
IPC_END_MESSAGE_MAP() IPC_END_MESSAGE_MAP()
...@@ -208,3 +210,9 @@ void ChromeRenderFrameObserver::OnAppBannerPromptRequest( ...@@ -208,3 +210,9 @@ void ChromeRenderFrameObserver::OnAppBannerPromptRequest(
Send(new ChromeViewHostMsg_AppBannerPromptReply( Send(new ChromeViewHostMsg_AppBannerPromptReply(
routing_id(), request_id, reply)); routing_id(), request_id, reply));
} }
void ChromeRenderFrameObserver::OnAppBannerDebugMessageRequest(
const std::string& message) {
render_frame()->GetWebFrame()->addMessageToConsole(blink::WebConsoleMessage(
blink::WebConsoleMessage::LevelDebug, base::UTF8ToUTF16(message)));
}
...@@ -31,6 +31,7 @@ class ChromeRenderFrameObserver : public content::RenderFrameObserver { ...@@ -31,6 +31,7 @@ class ChromeRenderFrameObserver : public content::RenderFrameObserver {
const gfx::Size& thumbnail_max_size_pixels); const gfx::Size& thumbnail_max_size_pixels);
void OnPrintNodeUnderContextMenu(); void OnPrintNodeUnderContextMenu();
void OnAppBannerPromptRequest(int request_id, const std::string& platform); void OnAppBannerPromptRequest(int request_id, const std::string& platform);
void OnAppBannerDebugMessageRequest(const std::string& message);
DISALLOW_COPY_AND_ASSIGN(ChromeRenderFrameObserver); DISALLOW_COPY_AND_ASSIGN(ChromeRenderFrameObserver);
}; };
......
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