WebView loadDataWithBaseURL() was determined as a same page navigation

third_party/WebKit/Source/core/loader/FrameLoader.cpp

 /*
  * Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights
  * reserved.
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved.
  * (http://www.torchmobile.com/)
  * Copyright (C) 2008 Alp Toker <alp@atoker.com>
  * Copyright (C) Research In Motion Limited 2009. All rights reserved.
  * Copyright (C) 2011 Kris Jordan <krisjordan@gmail.com>
  * Copyright (C) 2011 Google Inc. All rights reserved.
@@ skipping 921 matching lines @@
     return FrameLoadTypeReloadBypassingCache;
   // From the HTML5 spec for location.assign():
   // "If the browsing context's session history contains only one Document,
   // and that was the about:blank Document created when the browsing context
   // was created, then the navigation must be done with replacement enabled."
   if (request.replacesCurrentItem() ||
       (!m_stateMachine.committedMultipleRealLoads() &&
        equalIgnoringCase(m_frame->document()->url(), blankURL())))
     return FrameLoadTypeReplaceCurrentItem;
 
-  if (request.resourceRequest().url() == m_documentLoader->urlForHistory()) {
+  // TODO(crbug.com/662823): WebFrameLoaderImpl::loadData() sets baseUrl for

[Charlie Reis, 2016/11/28 20:07:27]

Can you clarify what still needs to be done here?  I would also suggest filing a
separate bug for it, since I expect that we'll be closing 662823 when this CL
lands.

[Takashi Toyoshima, 2016/11/29 09:39:36]

Filed a new bug, crbug.com/669408.

Actually, this comment wasn't enough to explain everything. Sorry for that.

As I told torne@, WebView spec looks a little unclear to me. The document does
not explain what should be the page URL for this API. Since we need the page URL
to detect the same page navigation correctly, I just tried to exclude all
loadDataWithBaseURL() cases from the same page navigation in this patch as a
workaround.

If the URL should be the same URL with baseUrl, current behavior should be a
right expected one.

If the URL should be encoded content in a data URL format, we may need
exceptional code to send the baseURL as a referer for this WebView API.

IMO, assuming baseUrl as a page URL sounds more reasonable.

+  // ResourceRequest's URL, but we need to compare Data URL encoded contents
+  // to determine the same page navigation for Data URLs.
+  bool isDataUrlWithEmulatedBaseUrl = request.substituteData().isValid();

[Charlie Reis, 2016/11/28 20:07:27]

nit: isDataUrlWithBaseUrl seems like it would be more consistent with the API
name.

I'm mildly concerned that checking for this via substituteData might affect more
cases than just loadDataWithBaseURL.  I'm not familiar with what SubstituteData
is used for, so I can't say. 

[dcheng, 2016/11/28 20:12:55]

I echo Charlie's concern here: is this actually selective to only the data URL
with base URL case? I see SubstituteData() created in a number of other places
(https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/fetch/Sub...),
so it's not clear to me that this filtering is selective enough (or maybe it is,
but needs comments to explain why)

[Takashi Toyoshima, 2016/11/29 09:39:36]

Agreed.
Probably, we should discuss what is the right behavior first.

Then, if we decided to handle loadDataWithBaseURL() cases in a special way like
this, I'd introduce an explicit flag in the SubstituteData() to say this content
is generated by loadDataWithBaseURL().

+  if (!isDataUrlWithEmulatedBaseUrl &&
+      request.resourceRequest().url() == m_documentLoader->urlForHistory()) {
     if (!request.originDocument())
       return FrameLoadTypeReloadMainResource;
     return request.resourceRequest().httpMethod() == HTTPNames::POST
                ? FrameLoadTypeStandard
                : FrameLoadTypeReplaceCurrentItem;
   }
 
   if (request.substituteData().failingURL() ==
           m_documentLoader->urlForHistory() &&
       m_loadType == FrameLoadTypeReload)
@@ skipping 999 matching lines @@
                          m_documentLoader ? m_documentLoader->url() : String());
   return tracedValue;
 }
 
 inline void FrameLoader::takeObjectSnapshot() const {
   TRACE_EVENT_OBJECT_SNAPSHOT_WITH_ID("loading", "FrameLoader", this,
                                       toTracedValue());
 }
 
 }  // namespace blink