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

chrome/browser/android/banners/app_banner_manager_android.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/android/banners/app_banner_manager_android.h"
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
 #include "base/command_line.h"
 #include "chrome/browser/android/banners/app_banner_data_fetcher_android.h"
@@ skipping 33 matching lines @@
                                                  jobject obj,
                                                  jobject jweb_contents) {
   content::WebContents* web_contents =
       content::WebContents::FromJavaWebContents(jweb_contents);
   AppBannerManager::ReplaceWebContents(web_contents);
 }
 
 bool AppBannerManagerAndroid::HandleNonWebApp(const std::string& platform,
                                               const GURL& url,
                                               const std::string& id) {
-  if (platform != kPlayPlatform || id.empty())
+  if (!CheckPlatformAndId(platform, id))
     return false;
 
   banners::TrackDisplayEvent(DISPLAY_EVENT_BANNER_REQUESTED);
 
   // Send the info to the Java side to get info about the app.
   JNIEnv* env = base::android::AttachCurrentThread();
   ScopedJavaLocalRef<jobject> jobj = weak_java_banner_view_manager_.get(env);
   if (jobj.is_null())
     return false;
 
   ScopedJavaLocalRef<jstring> jurl(
       ConvertUTF8ToJavaString(env, data_fetcher()->validated_url().spec()));
   ScopedJavaLocalRef<jstring> jpackage(
       ConvertUTF8ToJavaString(env, id));
   Java_AppBannerManager_fetchAppDetails(
       env, jobj.obj(), jurl.obj(), jpackage.obj(), ideal_icon_size());
   return true;
 }
 
+bool AppBannerManagerAndroid::CheckPlatformAndId(const std::string& platform,

[benwells, 2015/05/11 08:08:39]

Actually ... this probably shouldn't generate a 'not shown' message, as it might
be shown for the web platform (or not, if there are other problems).

Maybe it should just generate a message saying 'XXX platform application ignored
as not supported on Android' or 'play application ignored as there is no id'.

[dominickn (DO NOT USE), 2015/05/12 07:41:30]

Done.

+                                                 const std::string& id) {
+  if (platform != kPlayPlatform) {
+    banners::SendDebugMessage(web_contents(),
+                              "not shown: application not from Play platform");
+    return false;
+  } else if (id.empty()) {

[benwells, 2015/05/11 08:08:39]

Nit: no else after return (and ditto later).

+    banners::SendDebugMessage(web_contents(),
+                              "not shown: application id is empty");
+    return false;
+  }
+  return true;
+}
+
+bool AppBannerManagerAndroid::CheckFetcherMatchesContents() {
+  if (!data_fetcher()) {
+    banners::SendDebugMessage(web_contents(),
+                              "not shown: data fetcher is not active");

[benwells, 2015/05/11 08:08:39]

This error should be the same as the 'the user navigated' one.

[dominickn (DO NOT USE), 2015/05/12 07:41:30]

Done.

+    return false;
+  } else if (!web_contents()) {
+    return false;
+  } else if (data_fetcher()->validated_url() != web_contents()->GetURL()) {
+    banners::SendDebugMessage(web_contents(),
+                              "not shown: page URL does not match fetcher URL");

[benwells, 2015/05/11 08:08:39]

Same, here, I think this only happens when the user navigates.

[dominickn (DO NOT USE), 2015/05/12 07:41:30]

Done.

+    return false;
+  }
+  return true;
+}
+
 AppBannerDataFetcher* AppBannerManagerAndroid::CreateAppBannerDataFetcher(
     base::WeakPtr<Delegate> weak_delegate,
     const int ideal_icon_size) {
   return new AppBannerDataFetcherAndroid(web_contents(), weak_delegate,
                                          ideal_icon_size);
 }
 
 bool AppBannerManagerAndroid::OnAppDetailsRetrieved(JNIEnv* env,
                                                     jobject obj,
                                                     jobject japp_data,
                                                     jstring japp_title,
                                                     jstring japp_package,
                                                     jstring jicon_url) {
-  if (!data_fetcher() || !web_contents()
-      || data_fetcher()->validated_url() != web_contents()->GetURL()) {
+  if (!CheckFetcherMatchesContents())
     return false;
-  }
 
   base::android::ScopedJavaLocalRef<jobject> native_app_data;
   native_app_data.Reset(env, japp_data);
   GURL image_url = GURL(ConvertJavaStringToUTF8(env, jicon_url));
 
   AppBannerDataFetcherAndroid* android_fetcher =
       static_cast<AppBannerDataFetcherAndroid*>(data_fetcher().get());
   return android_fetcher->ContinueFetching(
       ConvertJavaStringToUTF16(env, japp_title),
       ConvertJavaStringToUTF8(env, japp_package),
@@ skipping 21 matching lines @@
 
 void DisableSecureSchemeCheckForTesting(JNIEnv* env, jclass clazz) {
   AppBannerManager::DisableSecureSchemeCheckForTesting();
 }
 
 jboolean IsEnabled(JNIEnv* env, jclass clazz) {
   return AppBannerManager::IsEnabled();
 }
 
 }  // namespace banners