Set the original url correctly if the frame is loaded via loadData

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 "base/command_line.h"
@@ skipping 2088 matching lines @@
   FOR_EACH_OBSERVER(RenderFrameObserver, observers_, OnStop());
 }
 
 // Tell the embedding application that the URL of the active page has changed.
 void RenderFrameImpl::UpdateURL(blink::WebFrame* frame) {
   DCHECK(!frame_ || frame_ == frame);
   WebDataSource* ds = frame->dataSource();
   DCHECK(ds);
 
   const WebURLRequest& request = ds->request();
-  const WebURLRequest& original_request = ds->originalRequest();
   const WebURLResponse& response = ds->response();
 
   DocumentState* document_state = DocumentState::FromDataSource(ds);
   NavigationState* navigation_state = document_state->navigation_state();
   InternalDocumentStateData* internal_data =
       InternalDocumentStateData::FromDocumentState(document_state);
 
   FrameHostMsg_DidCommitProvisionalLoad_Params params;
   params.http_status_code = response.httpStatusCode();
   params.is_post = false;
@@ skipping 100 matching lines @@
       params.is_post = true;
       params.post_id = ExtractPostId(item);
     }
 
     // Send the user agent override back.
     params.is_overriding_user_agent = internal_data->is_overriding_user_agent();
 
     // Track the URL of the original request.  We use the first entry of the
     // redirect chain if it exists because the chain may have started in another
     // process.
-    if (params.redirects.size() > 0)
-      params.original_request_url = params.redirects.at(0);
-    else
-      params.original_request_url = original_request.url();
+    params.original_request_url = GetOriginalRequestUrl(ds);
 
     params.history_list_was_cleared =
         navigation_state->history_list_was_cleared();
 
     // Save some histogram data so we can compute the average memory used per
     // page load of the glyphs.
     UMA_HISTOGRAM_COUNTS_10000("Memory.GlyphPagesPerLoad",
                                blink::WebGlyphCache::pageCount());
 
     // This message needs to be sent before any of allowScripts(),
@@ skipping 30 matching lines @@
 }
 
 void RenderFrameImpl::didStartLoading() {
   Send(new FrameHostMsg_DidStartLoading(routing_id_));
 }
 
 void RenderFrameImpl::didStopLoading() {
   Send(new FrameHostMsg_DidStopLoading(routing_id_));
 }
 
+GURL RenderFrameImpl::GetOriginalRequestUrl(WebDataSource* ds) {

[darin (slow to review), 2014/03/11 20:04:32]

nit: We aren't terribly consistent, but a quick grep reveals that we have more
method names including "URL" than "Url". I'd probably go with "URL".

[hush (inactive), 2014/03/12 21:34:21]

Done

+  std::vector<GURL> redirects;

[darin (slow to review), 2014/03/11 20:04:32]

nit: I'd move this below the hasUnreachableURL branch. That way you don't bother
getting the redirect chain unless it is needed.

[hush (inactive), 2014/03/12 21:34:21]

Done

+  GetRedirectChain(ds, &redirects);
+
+  // The frame is loaded via WebFrame::loadData, where we pass the

[darin (slow to review), 2014/03/11 20:04:32]

nit: This comment needs some work. It appears to be referring to some variable
names "history_url_for_data_url" that don't have much meaning in the context of
this function.

I think it is enough to indicate that the unreachableURL parameter of
WebFrame::loadData() should be treated as the original URL if set.

[hush (inactive), 2014/03/12 21:34:21]

Okay. I just reworded the comments by only referring to baseURL and
unreachableURL. These two are the input params to WebFrame::loadData.

+  // history_url_for_data_url as unreachableURL in blink. The
+  // base_url_for_data_url will be in the redirect chain, but we
+  // never actually visited base_url_for_data_url. So we need to set
+  // history_url_for_data_url as the original_request_url.
+  if (ds->hasUnreachableURL())
+    return ds->unreachableURL();
+
+  if (redirects.size() > 0)

[darin (slow to review), 2014/03/11 20:04:32]

nit: I'd use the .empty() method here instead: if (!redirects.empty())

[hush (inactive), 2014/03/12 21:34:21]

Done

+    return redirects.at(0);
+
+  return ds->originalRequest().url();
+}
+
 }  // namespace content