Don't try to report metrics when there's no URLRequest

components/cronet/android/test/javatests/src/org/chromium/net/RequestFinishedInfoTest.java

 // Copyright 2016 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.
 
 package org.chromium.net;
 
 import static org.chromium.base.CollectionUtil.newHashSet;
 
 import android.os.ConditionVariable;
 import android.test.suitebuilder.annotation.SmallTest;
@@ skipping 274 matching lines @@
         RequestFinishedInfo requestInfo = requestFinishedListener.getRequestInfo();
         MetricsTestUtil.checkRequestFinishedInfo(requestInfo, mUrl, startTime, endTime);
         assertEquals(RequestFinishedInfo.CANCELED, requestInfo.getFinishedReason());
         MetricsTestUtil.checkHasConnectTiming(requestInfo.getMetrics(), startTime, endTime, false);
 
         assertEquals(newHashSet("request annotation", this), // Use sets for unordered comparison.
                 new HashSet<Object>(requestInfo.getAnnotations()));
         mTestFramework.mCronetEngine.shutdown();
     }
 
+    // Checks that CronetURLRequestAdapter::DestroyOnNetworkThread() doesn't crash when metrics
+    // collection is enabled and the URLRequest hasn't been created. See http://crbug.com/675629.
+    @SmallTest
+    @OnlyRunNativeCronet
+    @Feature({"Cronet"})
+    public void testExceptionInRequestStart() throws Exception {
+        mTestFramework = startCronetTestFramework();
+        TestRequestFinishedListener requestFinishedListener = new TestRequestFinishedListener();

[xunjieli, 2016/12/19 21:18:31]

Can you make sure the executor backing this listener doesn't have any task
posted? could have a simple Executor that rejects all tasks.

[mgersh, 2016/12/19 21:37:25]

Oh, that's a better way to do it, thanks. Done.

+        mTestFramework.mCronetEngine.addRequestFinishedListener(requestFinishedListener);
+        TestUrlRequestCallback callback = new TestUrlRequestCallback();
+        ExperimentalUrlRequest.Builder urlRequestBuilder =
+                mTestFramework.mCronetEngine.newUrlRequestBuilder(
+                        mUrl, callback, callback.getExecutor());
+        // Empty headers are invalid and will cause start() to throw an exception.
+        UrlRequest request = urlRequestBuilder.addHeader("", "")
+                                     .addRequestAnnotation("request annotation")
+                                     .addRequestAnnotation(this)

[xunjieli, 2016/12/19 21:18:31]

are these two annotations needed? 

[mgersh, 2016/12/19 21:37:25]

Nope, just left over from an earlier version. Removed them.

+                                     .build();
+        try {
+            request.start();
+            fail("UrlRequest.start() should throw IllegalArgumentException");
+        } catch (IllegalArgumentException e) {
+            assertEquals("Invalid header =", e.getMessage());
+        }
+
+        mTestFramework.mCronetEngine.shutdown();
+    }
+
     @SmallTest
     @Feature({"Cronet"})
     public void testMetricsGetters() throws Exception {
         long requestStart = 1;
         long dnsStart = 2;
         long dnsEnd = -1;
         long connectStart = 4;
         long connectEnd = 5;
         long sslStart = 6;
         long sslEnd = 7;
@@ skipping 20 matching lines @@
         assertEquals(new Date(sslEnd), metrics.getSslEnd());
         assertEquals(new Date(pushStart), metrics.getPushStart());
         assertEquals(new Date(pushEnd), metrics.getPushEnd());
         assertEquals(new Date(responseStart), metrics.getResponseStart());
         assertEquals(new Date(requestEnd), metrics.getRequestEnd());
         assertEquals(socketReused, metrics.getSocketReused());
         assertEquals(sentByteCount, (long) metrics.getSentByteCount());
         assertEquals(receivedByteCount, (long) metrics.getReceivedByteCount());
     }
 }