PlzNavigate: preserve SourceLocation when navigating

content/renderer/render_frame_impl.cc

 // Copyright 2013 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/renderer/render_frame_impl.h"
 
 #include <map>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 657 matching lines @@
   // because it is loaded immediately by the FrameLoader.
   FrameMsg_Navigate_Type::Value navigation_type =
       FrameMsg_Navigate_Type::DIFFERENT_DOCUMENT;
   if (info.navigationType == blink::WebNavigationTypeReload) {
     if (load_flags & net::LOAD_BYPASS_CACHE)
       navigation_type = FrameMsg_Navigate_Type::RELOAD_BYPASSING_CACHE;
     else
       navigation_type = FrameMsg_Navigate_Type::RELOAD;
   }
 
+  base::Optional<SourceLocation> source_location;
+  if (info.hasSourceLocation) {
+    source_location = SourceLocation(info.sourceLocation.url.latin1(),
+                                     info.sourceLocation.lineNumber,
+                                     info.sourceLocation.columnNumber);
+  }
+
   const RequestExtraData* extra_data =
       static_cast<RequestExtraData*>(info.urlRequest.getExtraData());
   DCHECK(extra_data);
   return CommonNavigationParams(
       info.urlRequest.url(), referrer, extra_data->transition_type(),
       navigation_type, true, info.replacesCurrentHistoryItem, ui_timestamp,
       report_type, GURL(), GURL(),
       static_cast<PreviewsState>(info.urlRequest.getPreviewsState()),
       base::TimeTicks::Now(), info.urlRequest.httpMethod().latin1(),
-      GetRequestBodyForWebURLRequest(info.urlRequest));
+      GetRequestBodyForWebURLRequest(info.urlRequest), source_location);
 }
 
 media::Context3D GetSharedMainThreadContext3D(
     scoped_refptr<ui::ContextProviderCommandBuffer> provider) {
   if (!provider)
     return media::Context3D();
   return media::Context3D(provider->ContextGL(), provider->GrContext());
 }
 
 WebFrameLoadType ReloadFrameLoadTypeFor(
@@ skipping 2710 matching lines @@
         navigation_state->request_params().navigation_timing.fetch_start);
 
     datasource->updateNavigation(
         redirect_start, redirect_end, fetch_start,
         !navigation_state->request_params().redirects.empty());
     // TODO(clamy) We need to provide additional timing values for the
     // Navigation Timing API to work with browser-side navigations.
     // UnloadEventStart and UnloadEventEnd are still missing.
   }
 
+  // PlzNavigate: update the source location before processing the navigation
+  // commit.
+  if (IsBrowserSideNavigationEnabled() &&
+      navigation_state->common_params().source_location.has_value()) {
+    blink::WebSourceLocation source_location;
+    source_location.url = WebString::fromLatin1(
+        navigation_state->common_params().source_location->url);
+    source_location.lineNumber =
+        navigation_state->common_params().source_location->line_number;
+    source_location.columnNumber =
+        navigation_state->common_params().source_location->column_number;
+    datasource->setSourceLocation(source_location);
+  }
+
   // Create the serviceworker's per-document network observing object if it
   // does not exist (When navigation happens within a page, the provider already
   // exists).
   if (ServiceWorkerNetworkProvider::FromDocumentState(
           DocumentState::FromDataSource(datasource)))
     return;
 
   ServiceWorkerNetworkProvider::AttachToDocumentState(
       DocumentState::FromDataSource(datasource),
       ServiceWorkerNetworkProvider::CreateForNavigation(
@@ skipping 2644 matching lines @@
     // false, and the frame may not have been set in a loading state. Do not
     // send a stop message if a history navigation is loading in this frame
     // nonetheless. This behavior will go away with subframe navigation
     // entries.
     if (frame_ && !frame_->isLoading() && !has_history_navigation_in_frame)
       Send(new FrameHostMsg_DidStopLoading(routing_id_));
   }
 
   // In case LoadRequest failed before didCreateDataSource was called.
   pending_navigation_params_.reset();
+
+  // PlzNavigate: reset the source location now that the commit checks have been
+  // processed.
+  if (IsBrowserSideNavigationEnabled()) {
+    frame_->dataSource()->resetSourceLocation();

[Nate Chapin, 2017/03/01 23:22:19]

Rather than needing to expose resetSourceLocation() and calling it here, could
we just clear it in DocumentLoader on commit?

[clamy, 2017/03/02 13:34:32]

The problem is that we're going to do an IPC before committing the navigation.
If any other Javascript runs in the meantime and ends up emitting a console
message, we will have the wrong SourceLocation set.

[Nate Chapin, 2017/03/03 00:17:05]

So the flow of that case would be:
* Start navigation that sets a SourceLocation on the provisionalDataSource().
* setTimeout() timer fires, JS runs and sends console message
* Data arrives from browser process, data source commits.

Is that right? If so, wouldn't the JS be associated with the committed
WebDataSource/DocumentLoader, instead of the provisional, so it would safely
ignore the SourceLocation? Or did I misunderstand?

[clamy, 2017/03/06 13:27:53]

That's the case I'm concerned about. Given how the code is written, I don't see
why we would associate it with the committed DataSource, since we override the
location when the provisional DataSource has a SourceLocation.

I do think that the correct thing to do is to only have the SourceLocation info
in the provisionalDataSource for one iteration of the message loop, which is
what this code is doing. After all, IIUC in the current code, you would not get
the SourceLocation for the navigation after the Javascript that triggered it
stopped executing, that is we switched to a new iteration in the message loop?

[Nate Chapin, 2017/03/13 17:45:42]

Ok, I think I can live with this. The combination of the special case in
FrameConsole.cpp and restricting it to a single message loop here is a bit
difficult to reason about. But I think the alternative is to audit all callers
of FrameConsole::addMessage() and figure out which ones should be sensitive to
the provisionalDocumentLoader()'s SourceLocation, and there may be some gray
areas.

+    if (frame_->provisionalDataSource())
+      frame_->provisionalDataSource()->resetSourceLocation();
+  }
 }
 
 void RenderFrameImpl::UpdateEncoding(WebFrame* frame,
                                      const std::string& encoding_name) {
   // Only update main frame's encoding_name.
   if (!frame->parent())
     Send(new FrameHostMsg_UpdateEncoding(routing_id_, encoding_name));
 }
 
 void RenderFrameImpl::SyncSelectionIfRequired() {
@@ skipping 757 matching lines @@
   // event target. Potentially a Pepper plugin will receive the event.
   // In order to tell whether a plugin gets the last mouse event and which it
   // is, we set |pepper_last_mouse_event_target_| to null here. If a plugin gets
   // the event, it will notify us via DidReceiveMouseEvent() and set itself as
   // |pepper_last_mouse_event_target_|.
   pepper_last_mouse_event_target_ = nullptr;
 #endif
 }
 
 }  // namespace content