PlzNavigate: Invoke didStartProvisionalLoad() when the renderer initiates a navigation in startLoad(

components/test_runner/web_frame_test_client.cc

 // 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.
 
 #include "components/test_runner/web_frame_test_client.h"
 
 #include <memory>
 
 #include "base/logging.h"
 #include "base/strings/string_piece.h"
@@ skipping 11 matching lines @@
 #include "components/test_runner/web_frame_test_proxy.h"
 #include "components/test_runner/web_test_delegate.h"
 #include "components/test_runner/web_view_test_proxy.h"
 #include "components/test_runner/web_widget_test_proxy.h"
 #include "net/base/net_errors.h"
 #include "third_party/WebKit/public/platform/WebString.h"
 #include "third_party/WebKit/public/platform/WebURL.h"
 #include "third_party/WebKit/public/platform/WebURLRequest.h"
 #include "third_party/WebKit/public/platform/WebURLResponse.h"
 #include "third_party/WebKit/public/web/WebConsoleMessage.h"
+#include "third_party/WebKit/public/web/WebDataSource.h"
 #include "third_party/WebKit/public/web/WebElement.h"
 #include "third_party/WebKit/public/web/WebFrame.h"
 #include "third_party/WebKit/public/web/WebLocalFrame.h"
 #include "third_party/WebKit/public/web/WebNavigationPolicy.h"
 #include "third_party/WebKit/public/web/WebPluginParams.h"
 #include "third_party/WebKit/public/web/WebUserGestureIndicator.h"
 #include "third_party/WebKit/public/web/WebView.h"
 #include "url/gurl.h"
 #include "url/url_constants.h"
 
@@ skipping 364 matching lines @@
 }
 
 void WebFrameTestClient::loadErrorPage(int reason) {
   if (test_runner()->shouldDumpFrameLoadCallbacks()) {
     delegate_->PrintMessage(base::StringPrintf(
         "- loadErrorPage: %s\n", net::ErrorToString(reason).c_str()));
   }
 }
 
 void WebFrameTestClient::didStartProvisionalLoad(blink::WebLocalFrame* frame) {
+  // PlzNavigate
+  // A provisional load notification is received when a frame navigation is
+  // sent to the browser. We don't want to log it again during commit.
+  // Please see the RenderFrameImpl::BeginNavigation() function.

[clamy, 2017/01/31 00:20:01]

I'm a bit worried about this. Considering at which time we send the
didStartProvisionalLoad, we haven't set up a provisional DataSource (ie a
provisional DocumentLoader). So what we get here is a nullptr I imagine. The
provisional DataSource will not be created until the navigation is ready to
commit, which IMO is fairly confusing, since you have no way to actually get at
the request for which you called DidStartProvisionalLoad. Maybe it would be
better to add the WebURLRequest explicitly as a parameter to
didStartProvisionalLoad? I understand this works, but I'm worried about other
users of the API getting confused by weird behavior.

Additionally, I'm wondering how this works with renderer-initiated navigations
that transfer: we will end up committing the navigation in a different renderer.
I imagine that for the purpose of outputting DidStartProvisionalLoad only once
that would work though.

[ananta, 2017/01/31 03:37:07]

I added the request as a parameter to WebFrameClient. The
didStartProvisionalLoad() call in BeginNavigate() serves the purpose of the
layout test only. The problem you pointed out with the navigation getting
committed  in a different renderer exists. However in that  case we would end up
calling didStartProvisionalLoad() again anyways in the commit code path

+  blink::WebDataSource* ds = frame->provisionalDataSource();
+  if (ds && delegate_->IsNavigationInitiatedByRenderer(ds->getRequest()))
+    return;
+
   test_runner()->tryToSetTopLoadingFrame(frame);
 
   if (test_runner()->shouldDumpFrameLoadCallbacks()) {
     PrintFrameDescription(delegate_, frame);
     delegate_->PrintMessage(" - didStartProvisionalLoadForFrame\n");
   }
 
   if (test_runner()->shouldDumpUserGestureInFrameLoadCallbacks()) {
     PrintFrameuserGestureStatus(delegate_, frame,
                                 " - in didStartProvisionalLoadForFrame\n");
@@ skipping 290 matching lines @@
 blink::WebEffectiveConnectionType
 WebFrameTestClient::getEffectiveConnectionType() {
   return test_runner()->effective_connection_type();
 }
 
 TestRunner* WebFrameTestClient::test_runner() {
   return web_view_test_proxy_base_->test_interfaces()->GetTestRunner();
 }
 
 }  // namespace test_runner