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

content/renderer/render_frame_impl_unittest.cc

Index: content/renderer/render_frame_impl_unittest.cc
diff --git a/content/renderer/render_frame_impl_unittest.cc b/content/renderer/render_frame_impl_unittest.cc
index 30c04f61724b55c33d71ce9b5eb936b10dccd7ed..05e9d49825c0c4f3d65cd6fb664f8d612fcdf17c 100644
--- a/content/renderer/render_frame_impl_unittest.cc
+++ b/content/renderer/render_frame_impl_unittest.cc
@@ -6,10 +6,95 @@
 #include "content/public/common/context_menu_params.h"
 #include "content/renderer/render_frame_impl.h"
 #include "testing/gtest/include/gtest/gtest.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/platform/WebVector.h"
+#include "third_party/WebKit/public/web/WebDataSource.h"
+#include "third_party/WebKit/public/web/WebNavigationType.h"
 #include "ui/gfx/range/range.h"
+#include "url/gurl.h"
+
+using blink::WebURL;
+using blink::WebURLRequest;
+using blink::WebURLResponse;
+using blink::WebVector;
+using blink::WebDataSource;
+using blink::WebNavigationType;
+using blink::WebNavigationTypeOther;
 
 namespace content {
 
+class WebDataSourceMock : public WebDataSource {

[darin (slow to review), 2014/03/11 20:09:52]

Note: This is not obvious, but if you implement WebDataSource in the Chromium
repository, then it makes it a bit harder to evolve the interface.

Interfaces intended to be implemented only by Blink have pure virtual methods,
and interfaces only intended to be implemented by Chromium have non-pure virtual
methods. This helps save some steps when evolving interfaces.

I'm also not sure that this unit test really provides that much value. A
slightly larger test that invokes loadData() and observes the resulting original
URL might be better.

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

Thanks for note!
In order to call loadData() and observe the original URL, I think I will make a
browser test. I will loadData from the web contents, or content shell level,
then I will observe the top navigation entry and ensure the entry's original url
is the one we passed in as "history url"
On 2014/03/11 20:09:52, darin wrote:

+ public:
+  void setUnreachableURL(const WebURL url) {
+    unreachable_url_ = url;
+  }
+  void setOriginalRequest(WebURLRequest request) {
+    original_request_ = request;
+  }
+
+  // Override
+  const WebURLRequest& originalRequest() const {
+    return original_request_;
+  }
+  // Override
+  bool hasUnreachableURL() const {
+    return !unreachable_url_.isEmpty();
+  }
+  // Override
+  WebURL unreachableURL() const {
+    return unreachable_url_;
+  }
+  // Override
+  void appendRedirect(const WebURL& url) {
+    redirects_.push_back(url);
+  }
+  // Override
+  void redirectChain(WebVector<WebURL>& result) const {
+    result = redirects_;
+  }
+  // Override
+  bool isClientRedirect() const {
+    return true;
+  }
+  // Override
+  bool replacesCurrentHistoryItem() const {
+    return unreachable_url_.isEmpty();
+  }
+  // Override
+  WebNavigationType navigationType() const {
+    return WebNavigationTypeOther;
+  }
+  // Override
+  double triggeringEventTime() const {
+    return 0.0;
+  }
+  // Override
+  ExtraData* extraData() const {
+    return new ExtraData();
+  }
+  // Override
+  void setExtraData(ExtraData* extraData) {}
+  // Override
+  void setNavigationStartTime(double startTime) {}
+  // Override
+  const WebURLRequest& request() const {
+    return request_;
+  }
+  // Override
+  const WebURLResponse& response() const {
+    return response_;
+  }
+
+ private:
+  WebURL unreachable_url_;
+  WebURLRequest original_request_;
+  WebURLRequest request_;
+  WebURLResponse response_;
+  std::vector<WebURL> redirects_;
+};
+
 TEST(RenderFrameImplTest, ShouldUpdateSelectionTextFromContextMenuParams) {
   struct {
     const char* selection_text;
@@ -38,4 +123,43 @@ TEST(RenderFrameImplTest, ShouldUpdateSelectionTextFromContextMenuParams) {
   }
 }
 
+TEST(RenderFrameImplTest, GetOriginalRequestUrlViaLoadData) {
+  WebDataSourceMock ds = WebDataSourceMock();
+  WebURL unreachable_url, base_url;
+  unreachable_url = GURL("http://i.am.history.url.in.load.data");
+  base_url = GURL("http://i.am.base.url.in.load.data");
+
+  // When you loadData with base_url and history_url, WebDataSource will regard
+  // history_url as unreachable_url and it will push base_url to the redirect
+  // chain. But you actually didn't load base_url, because it is only used
+  // as the base url for relative paths in data. So history url should be
+  // treated as the original url in this case.
+  ds.setUnreachableURL(unreachable_url);
+  ds.appendRedirect(base_url);
+
+  // history url should be treated as the original url
+  EXPECT_EQ(unreachable_url, RenderFrameImpl::GetOriginalRequestUrl(&ds));
+}
+
+TEST(RenderFrameImplTest, GetOriginalRequestUrlViaLoadUrlWithoutRedirects) {
+  // loadUrl scenario, no redirects
+  WebDataSourceMock ds = WebDataSourceMock();
+  WebURL url = GURL("http://www.example.com");
+  ds.setOriginalRequest(WebURLRequest(url));
+  ds.appendRedirect(url);
+
+  EXPECT_EQ(url, RenderFrameImpl::GetOriginalRequestUrl(&ds));
+}
+
+TEST(RenderFrameImplTest, GetOriginalRequestUrlViaLoadUrlWithRedirects) {
+  // loadUrl scenario, with redirects
+  WebDataSourceMock ds = WebDataSourceMock();
+  WebURL url1 = GURL("http://url.redirected.from");
+  WebURL url2 = GURL("http://url.redirected.to");
+  ds.appendRedirect(url1);
+  ds.appendRedirect(url2);
+  ds.setOriginalRequest(WebURLRequest(url1));
+  EXPECT_EQ(url1, RenderFrameImpl::GetOriginalRequestUrl(&ds));
+}
+
 }  // namespace content