customtabs: Report parallel request initial and final status.

Reports whether a parallel request successfully started, and its final
completion status.

Bug: 884751
Change-Id: Ic4c975bd22b7fca98c066970cb202b93e8c7ca8c
Reviewed-on: https://chromium-review.googlesource.com/c/1273150Reviewed-by: Alexandr Ilin <alexilin@chromium.org>
Commit-Queue: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598757}

chrome/android/java/src/org/chromium/chrome/browser/ChromeFeatureList.java

@@ -167,6 +167,8 @@ public abstract class ChromeFeatureList {
     public static final String CCT_POST_MESSAGE_API = "CCTPostMessageAPI";
     public static final String CCT_REDIRECT_PRECONNECT = "CCTRedirectPreconnect";
     public static final String CCT_RESOURCE_PREFETCH = "CCTResourcePrefetch";
+    public static final String CCT_REPORT_PARALLEL_REQUEST_STATUS =
+            "CCTReportParallelRequestStatus";
     public static final String CHROME_DUET = "ChromeDuet";
     // TODO(mdjones): Remove CHROME_HOME_SWIPE_VELOCITY_FEATURE or rename.
     public static final String CHROME_HOME_SWIPE_VELOCITY_FEATURE = "ChromeHomeSwipeLogicVelocity";

chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java

@@ -37,6 +37,7 @@ import org.chromium.base.ThreadUtils;
 import org.chromium.base.TimeUtils;
 import org.chromium.base.TraceEvent;
 import org.chromium.base.VisibleForTesting;
+import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.library_loader.LibraryProcessType;
 import org.chromium.base.library_loader.ProcessInitException;
@@ -96,6 +97,7 @@ public class CustomTabsConnection {
     private static final String TAG = "ChromeConnection";
     private static final String LOG_SERVICE_REQUESTS = "custom-tabs-log-service-requests";
 
+    // Callback names for |extraCallback()|.
     @VisibleForTesting
     static final String PAGE_LOAD_METRICS_CALLBACK = "NavigationMetrics";
     static final String BOTTOM_BAR_SCROLL_STATE_CALLBACK = "onBottomBarScrollStateChanged";
@@ -103,6 +105,10 @@ public class CustomTabsConnection {
     static final String OPEN_IN_BROWSER_CALLBACK = "onOpenInBrowser";
     @VisibleForTesting
     static final String ON_WARMUP_COMPLETED = "onWarmupCompleted";
+    @VisibleForTesting
+    static final String ON_DETACHED_REQUEST_REQUESTED = "onDetachedRequestRequested";
+    @VisibleForTesting
+    static final String ON_DETACHED_REQUEST_COMPLETED = "onDetachedRequestCompleted";
 
     // For CustomTabs.SpeculationStatusOnStart, see tools/metrics/enums.xml. Append only.
     private static final int SPECULATION_STATUS_ON_START_ALLOWED = 0;
@@ -876,6 +882,19 @@ public class CustomTabsConnection {
         if (mLogRequests) {
             Log.w(TAG, "handleParallelRequest() = " + PARALLEL_REQUEST_MESSAGES[status]);
         }
+
+        if ((status != ParallelRequestStatus.NO_REQUEST)
+                && (status != ParallelRequestStatus.FAILURE_NOT_INITIALIZED)
+                && (status != ParallelRequestStatus.FAILURE_NOT_AUTHORIZED)
+                && ChromeFeatureList.isEnabled(
+                           ChromeFeatureList.CCT_REPORT_PARALLEL_REQUEST_STATUS)) {
+            Bundle args = new Bundle();
+            Uri url = intent.getParcelableExtra(PARALLEL_REQUEST_URL_KEY);
+            args.putParcelable("url", url);
+            args.putInt("status", status);
+            safeExtraCallback(session, ON_DETACHED_REQUEST_REQUESTED, args);
+        }
+
         return status;
     }
 
@@ -914,11 +933,13 @@ public class CustomTabsConnection {
 
         String urlString = url.toString();
         String referrerString = referrer.toString();
-        nativeCreateAndStartDetachedResourceRequest(Profile.getLastUsedProfile(), urlString,
-                referrerString, policy, DetachedResourceRequestMotivation.PARALLEL_REQUEST);
+        nativeCreateAndStartDetachedResourceRequest(Profile.getLastUsedProfile(), session,
+                urlString, referrerString, policy,
+                DetachedResourceRequestMotivation.PARALLEL_REQUEST);
         if (mLogRequests) {
             Log.w(TAG, "startParallelRequest(%s, %s, %d)", urlString, referrerString, policy);
         }
+
         return ParallelRequestStatus.SUCCESS;
     }
 
@@ -951,8 +972,10 @@ public class CustomTabsConnection {
             String urlString = url.toString();
             if (urlString.isEmpty() || !isValid(url)) continue;
 
-            nativeCreateAndStartDetachedResourceRequest(Profile.getLastUsedProfile(), urlString,
-                    referrerString, policy, DetachedResourceRequestMotivation.RESOURCE_PREFETCH);
+            // Session is null because we don't need completion notifications.
+            nativeCreateAndStartDetachedResourceRequest(Profile.getLastUsedProfile(), null,
+                    urlString, referrerString, policy,
+                    DetachedResourceRequestMotivation.RESOURCE_PREFETCH);
             ++requestsSent;
 
             if (mLogRequests) {
@@ -1489,7 +1512,8 @@ public class CustomTabsConnection {
     }
 
     private static native void nativeCreateAndStartDetachedResourceRequest(Profile profile,
-            String url, String origin, @WebReferrerPolicy int referrerPolicy,
+            CustomTabsSessionToken session, String url, String origin,
+            @WebReferrerPolicy int referrerPolicy,
             @DetachedResourceRequestMotivation int motivation);
 
     public ModuleLoader getModuleLoader(ComponentName componentName) {
@@ -1506,4 +1530,16 @@ public class CustomTabsConnection {
             ActivityDelegate activityDelegate, int moduleVersion) {
         mClientManager.setActivityDelegateForSession(sessionToken, activityDelegate, moduleVersion);
     }
+
+    @CalledByNative
+    public static void notifyClientOfDetachedRequestCompletion(
+            CustomTabsSessionToken session, String url, int status) {
+        if (!ChromeFeatureList.isEnabled(ChromeFeatureList.CCT_REPORT_PARALLEL_REQUEST_STATUS)) {
+            return;
+        }
+        Bundle args = new Bundle();
+        args.putParcelable("url", Uri.parse(url));
+        args.putInt("net_error", status);
+        getInstance().safeExtraCallback(session, ON_DETACHED_REQUEST_COMPLETED, args);
+    }
 }

chrome/browser/android/chrome_feature_list.cc

@@ -85,6 +85,7 @@ const base::Feature* kFeaturesExposedToJava[] = {
     &kCCTParallelRequest,
     &kCCTPostMessageAPI,
     &kCCTRedirectPreconnect,
+    &kCCTReportParallelRequestStatus,
     &kCCTResourcePrefetch,
     &kChromeDuetFeature,
     &kChromeHomeSwipeLogic,
@@ -230,6 +231,9 @@ const base::Feature kCCTPostMessageAPI{"CCTPostMessageAPI",
 const base::Feature kCCTRedirectPreconnect{"CCTRedirectPreconnect",
                                            base::FEATURE_ENABLED_BY_DEFAULT};
 
+const base::Feature kCCTReportParallelRequestStatus{
+    "CCTReportParallelRequestStatus", base::FEATURE_DISABLED_BY_DEFAULT};
+
 const base::Feature kCCTResourcePrefetch{"CCTResourcePrefetch",
                                          base::FEATURE_ENABLED_BY_DEFAULT};
 

chrome/browser/android/chrome_feature_list.h

@@ -27,6 +27,7 @@ extern const base::Feature kCCTModuleCache;
 extern const base::Feature kCCTParallelRequest;
 extern const base::Feature kCCTPostMessageAPI;
 extern const base::Feature kCCTRedirectPreconnect;
+extern const base::Feature kCCTReportParallelRequestStatus;
 extern const base::Feature kCCTResourcePrefetch;
 extern const base::Feature kChromeDuetFeature;
 extern const base::Feature kChromeHomeSwipeLogic;

chrome/browser/android/customtabs/detached_resource_request.cc

@@ -139,7 +139,9 @@ void DetachedResourceRequest::OnResponseCallback(
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   int net_error = url_loader_->NetError();
   bool success = net_error == net::OK;
+  net_error = std::abs(net_error);
   auto duration = base::TimeTicks::Now() - start_time_;
+
   switch (motivation_) {
     case Motivation::kParallelRequest: {
       if (success) {
@@ -158,7 +160,7 @@ void DetachedResourceRequest::OnResponseCallback(
       }
 
       base::UmaHistogramSparse("CustomTabs.DetachedResourceRequest.FinalStatus",
-                               std::abs(net_error));
+                               net_error);
       break;
     }
     case Motivation::kResourcePrefetch: {
@@ -171,12 +173,12 @@ void DetachedResourceRequest::OnResponseCallback(
       }
 
       base::UmaHistogramSparse("CustomTabs.ResourcePrefetch.FinalStatus",
-                               std::abs(net_error));
+                               net_error);
       break;
     }
   }
 
-  std::move(cb_).Run(success);
+  std::move(cb_).Run(net_error);
 }
 
 }  // namespace customtabs

chrome/browser/android/customtabs/detached_resource_request.h

@@ -44,13 +44,13 @@ class DetachedResourceRequest {
   // GENERATED_JAVA_CLASS_NAME_OVERRIDE: DetachedResourceRequestMotivation
   enum class Motivation { kParallelRequest, kResourcePrefetch };
 
-  using OnResultCallback = base::OnceCallback<void(bool success)>;
+  using OnResultCallback = base::OnceCallback<void(int net_error)>;
 
   ~DetachedResourceRequest();
 
   // Creates a detached request to a |url|, with a given initiating URL,
   // |first_party_for_cookies|. Called on the UI thread.
-  // Optional |cb| to get notified about the fetch result, for testing.
+  // Optional |cb| to get notified about the fetch result.
   static void CreateAndStart(content::BrowserContext* browser_context,
                              const GURL& url,
                              const GURL& first_party_for_cookies,

chrome/browser/android/customtabs/detached_resource_request_android.cc

@@ -5,6 +5,7 @@
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
 #include "base/android/scoped_java_ref.h"
+#include "base/bind.h"
 #include "chrome/browser/android/customtabs/detached_resource_request.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_android.h"
@@ -15,10 +16,25 @@
 
 namespace customtabs {
 
+namespace {
+
+void NotifyClientOfDetachedRequestCompletion(
+    const base::android::ScopedJavaGlobalRef<jobject>& session,
+    const GURL& url,
+    int net_error) {
+  JNIEnv* env = base::android::AttachCurrentThread();
+  Java_CustomTabsConnection_notifyClientOfDetachedRequestCompletion(
+      env, session, base::android::ConvertUTF8ToJavaString(env, url.spec()),
+      net_error);
+}
+
+}  // namespace
+
 static void JNI_CustomTabsConnection_CreateAndStartDetachedResourceRequest(
     JNIEnv* env,
     const base::android::JavaParamRef<jclass>& jcaller,
     const base::android::JavaParamRef<jobject>& profile,
+    const base::android::JavaParamRef<jobject>& session,
     const base::android::JavaParamRef<jstring>& url,
     const base::android::JavaParamRef<jstring>& origin,
     jint referrer_policy,
@@ -39,9 +55,17 @@ static void JNI_CustomTabsConnection_CreateAndStartDetachedResourceRequest(
           static_cast<blink::WebReferrerPolicy>(referrer_policy));
   DetachedResourceRequest::Motivation request_motivation =
       static_cast<DetachedResourceRequest::Motivation>(motivation);
+
+  DetachedResourceRequest::OnResultCallback cb =
+      session.is_null()
+          ? base::DoNothing()
+          : base::BindOnce(&NotifyClientOfDetachedRequestCompletion,
+                           base::android::ScopedJavaGlobalRef<jobject>(session),
+                           native_url);
+
   DetachedResourceRequest::CreateAndStart(
       native_profile, native_url, native_origin, url_request_referrer_policy,
-      request_motivation);
+      request_motivation, std::move(cb));
 }
 
 }  // namespace customtabs

chrome/browser/android/customtabs/detached_resource_request_unittest.cc

@@ -228,8 +228,8 @@ class DetachedResourceRequestTest : public ::testing::Test {
 
     DetachedResourceRequest::CreateAndStart(
         browser_context(), url, site_for_cookies, policy, kMotivation,
-        base::BindLambdaForTesting([&](bool success) {
-          EXPECT_TRUE(success);
+        base::BindLambdaForTesting([&](int net_error) {
+          EXPECT_EQ(net::OK, net_error);
           request_completion_waiter.Quit();
         }));
     server_request_waiter.Run();
@@ -260,8 +260,8 @@ TEST_F(DetachedResourceRequestTest, Simple) {
   DetachedResourceRequest::CreateAndStart(
       browser_context(), url, site_for_cookies,
       content::Referrer::GetDefaultReferrerPolicy(), kMotivation,
-      base::BindLambdaForTesting([&](bool success) {
-        EXPECT_TRUE(success);
+      base::BindLambdaForTesting([&](int net_error) {
+        EXPECT_EQ(net::OK, net_error);
         request_completion_waiter.Quit();
       }));
   server_request_waiter.Run();
@@ -285,8 +285,8 @@ TEST_F(DetachedResourceRequestTest, SimpleFailure) {
   DetachedResourceRequest::CreateAndStart(
       browser_context(), url, site_for_cookies,
       content::Referrer::GetDefaultReferrerPolicy(), kMotivation,
-      base::BindLambdaForTesting([&](bool success) {
-        EXPECT_FALSE(success);
+      base::BindLambdaForTesting([&](int net_error) {
+        EXPECT_NE(net::OK, net_error);
         request_waiter.Quit();
       }));
   request_waiter.Run();
@@ -389,8 +389,8 @@ TEST_F(DetachedResourceRequestTest, NoContentCanSetCookie) {
   DetachedResourceRequest::CreateAndStart(
       browser_context(), url, site_for_cookies,
       content::Referrer::GetDefaultReferrerPolicy(), kMotivation,
-      base::BindLambdaForTesting([&](bool success) {
-        EXPECT_TRUE(success);
+      base::BindLambdaForTesting([&](int net_error) {
+        EXPECT_EQ(net::OK, net_error);
         request_completion_waiter.Quit();
       }));
 
@@ -450,8 +450,8 @@ TEST_F(DetachedResourceRequestTest, MultipleOrigins) {
   DetachedResourceRequest::CreateAndStart(
       browser_context(), url, site_for_cookies,
       content::Referrer::GetDefaultReferrerPolicy(), kMotivation,
-      base::BindLambdaForTesting([&](bool success) {
-        EXPECT_TRUE(success);
+      base::BindLambdaForTesting([&](int net_error) {
+        EXPECT_EQ(net::OK, net_error);
         detached_request_waiter.Quit();
       }));
   first_request_waiter.Run();
@@ -481,8 +481,8 @@ TEST_F(DetachedResourceRequestTest, ManyRedirects) {
   DetachedResourceRequest::CreateAndStart(
       browser_context(), url, site_for_cookies,
       content::Referrer::GetDefaultReferrerPolicy(), kMotivation,
-      base::BindLambdaForTesting([&](bool success) {
-        EXPECT_TRUE(success);
+      base::BindLambdaForTesting([&](int net_error) {
+        EXPECT_EQ(net::OK, net_error);
         request_waiter.Quit();
       }));
   request_waiter.Run();
@@ -505,8 +505,8 @@ TEST_F(DetachedResourceRequestTest, TooManyRedirects) {
   DetachedResourceRequest::CreateAndStart(
       browser_context(), url, site_for_cookies,
       content::Referrer::GetDefaultReferrerPolicy(), kMotivation,
-      base::BindLambdaForTesting([&](bool success) {
-        EXPECT_FALSE(success);
+      base::BindLambdaForTesting([&](int net_error) {
+        EXPECT_EQ(-net::ERR_TOO_MANY_REDIRECTS, net_error);
         request_waiter.Quit();
       }));
   request_waiter.Run();
@@ -534,8 +534,8 @@ TEST_F(DetachedResourceRequestTest, CachedResponse) {
   DetachedResourceRequest::CreateAndStart(
       browser_context(), url, site_for_cookies,
       content::Referrer::GetDefaultReferrerPolicy(), kMotivation,
-      base::BindLambdaForTesting([&](bool success) {
-        EXPECT_TRUE(success);
+      base::BindLambdaForTesting([&](int net_error) {
+        EXPECT_EQ(net::OK, net_error);
         first_request_waiter.Quit();
       }));
   first_request_waiter.Run();
@@ -543,8 +543,8 @@ TEST_F(DetachedResourceRequestTest, CachedResponse) {
   DetachedResourceRequest::CreateAndStart(
       browser_context(), url, site_for_cookies,
       content::Referrer::GetDefaultReferrerPolicy(), kMotivation,
-      base::BindLambdaForTesting([&](bool success) {
-        EXPECT_TRUE(success);
+      base::BindLambdaForTesting([&](int net_error) {
+        EXPECT_EQ(net::OK, net_error);
         second_request_waiter.Quit();
       }));
   second_request_waiter.Run();