Expose SPDY pushes in DevTools

chrome/browser/devtools/devtools_sanity_browsertest.cc

 // Copyright (c) 2012 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 <stddef.h>
 
 #include "base/bind.h"
 #include "base/cancelable_callback.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
@@ skipping 46 matching lines @@
 #include "content/public/common/content_switches.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/notification_types.h"
 #include "extensions/common/switches.h"
 #include "extensions/common/value_builder.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/spawned_test_server/spawned_test_server.h"
+#include "net/url_request/url_request_context.h"
+#include "net/url_request/url_request_filter.h"
+#include "net/url_request/url_request_http_job.h"
 #include "third_party/WebKit/public/web/WebInputEvent.h"
 #include "ui/compositor/compositor_switches.h"
 #include "ui/gl/gl_switches.h"
 
 using app_modal::AppModalDialog;
 using app_modal::JavaScriptAppModalDialog;
 using app_modal::NativeAppModalDialog;
 using content::BrowserThread;
 using content::DevToolsAgentHost;
 using content::NavigationController;
@@ skipping 10 matching lines @@
     "files/devtools/pause_when_loading_devtools.html";
 const char kPauseWhenScriptIsRunning[] =
     "files/devtools/pause_when_script_is_running.html";
 const char kPageWithContentScript[] =
     "files/devtools/page_with_content_script.html";
 const char kNavigateBackTestPage[] =
     "files/devtools/navigate_back.html";
 const char kWindowOpenTestPage[] = "files/devtools/window_open.html";
 const char kLatencyInfoTestPage[] = "files/devtools/latency_info.html";
 const char kChunkedTestPage[] = "chunked";
+const char kPushTestPage[] = "files/devtools/push_test_page.html";
+// The resource is not really pushed, but mock url request job pretends it is.
+const char kPushTestResource[] = "files/devtools/image.png";
 const char kSlowTestPage[] =
     "chunked?waitBeforeHeaders=100&waitBetweenChunks=100&chunksNumber=2";
 const char kSharedWorkerTestPage[] =
     "files/workers/workers_ui_shared_worker.html";
 const char kSharedWorkerTestWorker[] =
     "files/workers/workers_ui_shared_worker.js";
 const char kReloadSharedWorkerTestPage[] =
     "files/workers/debug_shared_worker_initialization.html";
 const char kReloadSharedWorkerTestWorker[] =
     "files/workers/debug_shared_worker_initialization.js";
@@ skipping 51 matching lines @@
                             const Extension* devtools_extension,
                             const char* panel_name) {
   // The full name is the concatenation of the extension URL (stripped of its
   // trailing '/') and the |panel_name| that was passed to panels.create().
   std::string prefix = base::TrimString(devtools_extension->url().spec(), "/",
                                         base::TRIM_TRAILING)
                            .as_string();
   SwitchToPanel(window, (prefix + panel_name).c_str());
 }
 
+class PushTimesMockURLRequestJob : public net::URLRequestHttpJob {

[mmenke, 2016/04/08 16:21:36]

I'd rather this not inherit from URLRequestHttpJob, which it should be noted is
not intended for public consumption (See the "NET_EXPORT_PRIVATE" as a hint). 
We should just inherit from URLRequestMockHTTPJob (Which does require we give
the file path to the mock job).

[caseq, 2016/04/08 22:39:28]

Done. Note I also took a liberty to nuke another subclass of it,
URLRequestNonsecureConnection as a drive by.

+ public:
+  PushTimesMockURLRequestJob(net::URLRequest* request,
+                             net::NetworkDelegate* network_delegate)
+      : net::URLRequestHttpJob(request,
+                               network_delegate,
+                               request->context()->http_user_agent_settings()) {
+  }
+
+  void GetLoadTimingInfo(net::LoadTimingInfo* load_timing_info) const override {
+    net::URLRequestHttpJob::GetLoadTimingInfo(load_timing_info);
+    if (push_end_.is_null())
+      push_end_ = base::TimeTicks::Now();
+    load_timing_info->push_start =
+        request()->creation_time() - base::TimeDelta::FromMilliseconds(10);
+    load_timing_info->push_end = push_end_;
+  }
+
+ private:
+  mutable base::TimeTicks push_end_;
+  DISALLOW_COPY_AND_ASSIGN(PushTimesMockURLRequestJob);
+};
+
+class TestInterceptor : public net::URLRequestInterceptor {
+ public:
+  explicit TestInterceptor(const GURL& url) : url_(url) {}
+
+  // Registers |this| with the URLRequestFilter, which takes ownership of it.
+  void Register() {

[mmenke, 2016/04/08 16:21:36]

Suggest making this static, and having it create the TestInterceptor, and give
the filter ownership of it.  scoped_ptr(this) is just weird.

[caseq, 2016/04/08 22:39:28]

Done.

+    EXPECT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::IO));
+    net::URLRequestFilter::GetInstance()->AddUrlInterceptor(
+        url_, scoped_ptr<net::URLRequestInterceptor>(this));

[mmenke, 2016/04/08 16:21:36]

scoped_ptr<net::URLRequestInterceptor> -> make_scoped_ptr?

[caseq, 2016/04/08 22:39:28]

Done.

+  }
+
+  // Unregisters |this| with the URLRequestFilter, which should then delete
+  // |this|.
+  void Unregister() {

[mmenke, 2016/04/08 16:21:36]

Again, suggest making this static.  Does mean you'll need to pass in URL again,
or just clear all handlers.

[caseq, 2016/04/08 22:39:28]

Done.

+    EXPECT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::IO));
+    net::URLRequestFilter::GetInstance()->RemoveUrlHandler(url_);
+  }
+
+  // net::URLRequestJobFactory::ProtocolHandler implementation:
+  net::URLRequestJob* MaybeInterceptRequest(
+      net::URLRequest* request,
+      net::NetworkDelegate* network_delegate) const override {
+    EXPECT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::IO));
+    return new PushTimesMockURLRequestJob(request, network_delegate);
+  }
+
+ private:
+  GURL url_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestInterceptor);
+};
+
 }  // namespace
 
 class DevToolsSanityTest : public InProcessBrowserTest {
  public:
   DevToolsSanityTest() : window_(NULL) {}
 
   void SetUpOnMainThread() override {
     host_resolver()->AddRule("*", "127.0.0.1");
   }
 
@@ skipping 757 matching lines @@
 // Tests raw headers text.
 IN_PROC_BROWSER_TEST_F(DevToolsSanityTest, TestNetworkSyncSize) {
   RunTest("testNetworkSyncSize", kChunkedTestPage);
 }
 
 // Tests raw headers text.
 IN_PROC_BROWSER_TEST_F(DevToolsSanityTest, TestNetworkRawHeadersText) {
   RunTest("testNetworkRawHeadersText", kChunkedTestPage);
 }
 
+// Tests raw headers text.
+IN_PROC_BROWSER_TEST_F(DevToolsSanityTest, TestNetworkPushTime) {
+  OpenDevToolsWindow(kPushTestPage, false);
+  GURL push_url = spawned_test_server()->GetURL(kPushTestResource);
+  TestInterceptor* test_interceptor = new TestInterceptor(push_url);
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                          base::Bind(&TestInterceptor::Register,
+                                     base::Unretained(test_interceptor)));
+
+  DispatchOnTestSuite(window_, "testPushTimes", push_url.spec().c_str());

[mmenke, 2016/04/08 16:21:36]

Can we check that the push time is less that send_start...Or better, get a time
before the navigation starts, subtract 10 seconds from it, and check that the
push time is before the earliest normally reported time (navigation start?  Some
other start time?  Not too familiar with the timing APIs)

[caseq, 2016/04/08 22:39:28]

That's exactly what happens in testPushTime() -- have you had a chance to look
at Tests.js?
Well, as for navigation start -- I would expect it to always come before the
push time unless the page is being reloaded, since we would normally push only
after the main resource was requested and the connection was opened.

[mmenke, 2016/04/11 17:45:52]

Wait....So a chrome/browser/devtools/ test is using test code in
third_party/WebKit/Source/devtools/front_end/Tests.js?  That seems very unusual.
 Chrome browsertest-specific Javascript usually lives in chrome/test/data.

I think someone else should review the wiring to the Javascript test stuff here.

[caseq, 2016/04/13 21:48:45]

It's been this way since about the time we introduced browser tests for
devtools. The original motivation is that the JS code there is tightly coupled
to the devtools front-end and less coupled to the test invocations on the
browser side. It's less of a matter since blink repo merge, but I wouldn't want
to change this in this CL.
Understood -- I think dgozman@ has good expertise there and he's already on the
list of reviewers.

[mmenke, 2016/04/14 15:50:33]

The question isn't what's common, or what we expect, but what's possible.  Bugs
often crop up in obscure cases that are unexpected, but still possible.

[mmenke, 2016/04/14 15:50:33]

Happy to defer to him on that, but he should take another look, now that you've
added tests.

+
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                          base::Bind(&TestInterceptor::Unregister,
+                                     base::Unretained(test_interceptor)));
+  CloseDevToolsWindow();
+}
+
 // Tests that console messages are not duplicated on navigation back.
 #if defined(OS_WIN)
 // Flaking on windows swarm try runs: crbug.com/409285.
 #define MAYBE_TestConsoleOnNavigateBack DISABLED_TestConsoleOnNavigateBack
 #else
 #define MAYBE_TestConsoleOnNavigateBack TestConsoleOnNavigateBack
 #endif
 IN_PROC_BROWSER_TEST_F(DevToolsSanityTest, MAYBE_TestConsoleOnNavigateBack) {
   RunTest("testConsoleOnNavigateBack", kNavigateBackTestPage);
 }
@@ skipping 286 matching lines @@
 
   SimulateTapAt(web_contents, gfx::Point(30, 60));
   DispatchInPageAndWait("waitForEvent", "gesturetap");
 
   DispatchAndWait("stopTimeline");
   RunTestMethod("checkInputEventsPresent", "MouseMove", "MouseDown",
                 "MouseWheel", "GestureTap");
 
   CloseDevToolsWindow();
 }