NavigationSimulator: add support for GlobalRequestIds

content/public/test/navigation_simulator.cc

 // Copyright 2017 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 "content/public/test/navigation_simulator.h"
 
 #include "base/bind.h"
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
 #include "content/browser/frame_host/navigation_handle_impl.h"
 #include "content/browser/frame_host/navigation_request.h"
 #include "content/common/frame_messages.h"
+#include "content/public/browser/global_request_id.h"
 #include "content/public/browser/navigation_throttle.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/browser_side_navigation_policy.h"
 #include "content/test/test_navigation_url_loader.h"
 #include "content/test/test_render_frame_host.h"
 #include "net/base/load_flags.h"
 #include "net/url_request/redirect_info.h"
 
 namespace content {
 
@@ skipping 221 matching lines @@
     if (!handle_) {
       state_ = FAILED;
       return;
     }
   }
 
   // Call NavigationHandle::WillProcessResponse if needed.
   // Note that the handle's state can be CANCELING if a throttle cancelled it
   // synchronously in PrepareForCommit.
   if (handle_->state_for_testing() < NavigationHandleImpl::CANCELING) {
+    static int request_id = 0;

[Charlie Reis, 2017/05/15 20:07:43]

Are there any concerns with this overlapping with other request IDs in tests? 
Or is that not a concern since this only gets used in unit tests?

[Charlie Harrison, 2017/05/15 21:45:54]

I think this could theoretically overlap with request IDs in unit tests. I'm not
sure how much of a problem it is though, especially since consumers know that
these navigations are not really backed by URLRequests.

We could do something fancy like start at INT_MAX and subtract if you'd like.

[Charlie Reis, 2017/05/15 22:02:38]

Maybe just start it at 1000 to reduce the chance of overlap in the common case? 
(That's assuming there isn't any code comparing request IDs to each other, of
course.)

No big deal either way-- just trying to make it slightly less likely to
encounter odd unit test problems.

[Charlie Harrison, 2017/05/16 03:41:42]

Done.

+    GlobalRequestID global_id(render_frame_host_->GetProcess()->GetID(),
+                              ++request_id);
     handle_->WillProcessResponse(
         render_frame_host_, scoped_refptr<net::HttpResponseHeaders>(),
-        net::HttpResponseInfo::ConnectionInfo(), SSLStatus(), GlobalRequestID(),
+        net::HttpResponseInfo::ConnectionInfo(), SSLStatus(), global_id,
         false /* should_replace_current_entry */, false /* is_download */,
         false /* is_stream */, base::Closure(),
         base::Callback<void(NavigationThrottle::ThrottleCheckResult)>());
   }
 
   WaitForThrottleChecksComplete();
 
   if (GetLastThrottleCheckResult() != NavigationThrottle::PROCEED) {
     FailFromThrottleCheck(GetLastThrottleCheckResult());
     return;
@@ skipping 342 matching lines @@
   if (!should_result_in_error_page) {
     render_frame_host_->OnMessageReceived(
         FrameHostMsg_DidStopLoading(render_frame_host_->GetRoutingID()));
     CHECK_EQ(1, num_did_finish_navigation_called_);
   } else {
     CHECK_EQ(0, num_did_finish_navigation_called_);
   }
 }
 
 }  // namespace content