Move ExtraData from WebRequest to WebDataSource.

This adds a DidCreateDataSource method to WebViewDelegate,
which the embedder uses to know if their LoadRequest
resulted in a navigation.  The embedder then associates the
ExtraData with the WebDataSource at that point.  The
WebDataSource then proceeds to be treated as the provisional
data source, possibly failing or being committed.  We then
inspect WebFrame::GetDataSource in DidCommitLoadForFrame to
recover the ExtraData.

We have to take care to handle reference fragment
navigations since those do not result in a new WebDataSource 
being created.  So, in DidChangeLocationWithinPage, we
update the ExtraData associated with the WebDataSource
returned by WebFrame::GetDataSource.

One thing that is important to note:  In
DidCommitLoadForFrame, we would previously always inspect
the value of GetDataSource returned from the main frame
instead of looking at the frame passed to
DidCommitLoadForFrame.  This explains why WebFrameImpl::
LoadRequest needed to cached ExtraData on the current
WebDataSource, and I think doing so is awkward and wrong.

My change is to always store the ExtraData on the first
WebDataSource to be created as a result of LoadRequest. 
Then in DidCommitLoadForFrame, I always inspect the
ExtraData from the given WebFrame.  This means that for
history navigations that only navigate a subframe, we'll end 
up with ExtraData associated with the WebDataSource of a
subframe.  I think this is OK even though the old code was
trying to avoid this scenario.  See the DCHECK removed in
RenderView::UpdateURL.

BUG=11423
R=brettw

Review URL: http://codereview.chromium.org/115575

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@16580 0039d316-1c4b-4281-b951-d872f2087c98

17 files changed, 167 insertions, 167 deletions

diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index 33a25de..79ddf45 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -140,14 +140,12 @@ static const char* const kUnreachableWebDataURL =
 
 static const char* const kBackForwardNavigationScheme = "history";
 
-namespace {
-
 // Associated with browser-initiated navigations to hold tracking data.
-class RenderViewExtraRequestData : public WebRequest::ExtraData {
+class RenderView::NavigationState : public WebDataSource::ExtraData {
  public:
-  RenderViewExtraRequestData(int32 pending_page_id,
-                             PageTransition::Type transition,
-                             Time request_time)
+  NavigationState(int32 pending_page_id,
+                  PageTransition::Type transition,
+                  Time request_time)
       : transition_type(transition),
         request_time(request_time),
         request_committed(false),
@@ -163,6 +161,8 @@ class RenderViewExtraRequestData : public WebRequest::ExtraData {
   // Contains the transition type that the browser specified when it
   // initiated the load.
   PageTransition::Type transition_type;
+
+  // The time that this navigation was requested.
   Time request_time;
 
   // True if we have already processed the "DidCommitLoad" event for this
@@ -172,11 +172,9 @@ class RenderViewExtraRequestData : public WebRequest::ExtraData {
  private:
   int32 pending_page_id_;
 
-  DISALLOW_COPY_AND_ASSIGN(RenderViewExtraRequestData);
+  DISALLOW_COPY_AND_ASSIGN(NavigationState);
 };
 
-}  // namespace
-
 ///////////////////////////////////////////////////////////////////////////////
 
 RenderView::RenderView(RenderThreadBase* render_thread)
@@ -807,8 +805,14 @@ void RenderView::OnNavigate(const ViewMsg_Navigate_Params& params) {
 
   scoped_ptr<WebRequest> request(WebRequest::Create(params.url));
   request->SetCachePolicy(cache_policy);
-  request->SetExtraData(new RenderViewExtraRequestData(
-      params.page_id, params.transition, params.request_time));
+
+  // A navigation resulting from loading a javascript URL should not be treated
+  // as a browser initiated event.  Instead, we want it to look as if the page
+  // initiated any load resulting from JS execution.
+  if (!params.url.SchemeIs(chrome::kJavaScriptScheme)) {
+    pending_navigation_state_.reset(new NavigationState(
+        params.page_id, params.transition, params.request_time));
+  }
 
   // If we are reloading, then WebKit will use the state of the current page.
   // Otherwise, we give it the state to navigate to.
@@ -821,6 +825,9 @@ void RenderView::OnNavigate(const ViewMsg_Navigate_Params& params) {
   }
 
   main_frame->LoadRequest(request.get());
+
+  // In case LoadRequest failed before DidCreateDataSource was called.
+  pending_navigation_state_.reset();
 }
 
 // Stop loading the current page
@@ -977,10 +984,9 @@ void RenderView::UpdateURL(WebFrame* frame) {
   const WebRequest& initial_request = ds->GetInitialRequest();
   const WebResponse& response = ds->GetResponse();
 
-  // We don't hold a reference to the extra data. The request's reference will
-  // be sufficient because we won't modify it during our call. MAY BE NULL.
-  RenderViewExtraRequestData* extra_data =
-      static_cast<RenderViewExtraRequestData*>(request.GetExtraData());
+  // This will be null if we did not initiate the navigation.
+  NavigationState* navigation_state =
+      static_cast<NavigationState*>(ds->GetExtraData());
 
   ViewHostMsg_FrameNavigate_Params params;
   params.http_status_code = response.GetHttpStatusCode();
@@ -1024,7 +1030,7 @@ void RenderView::UpdateURL(WebFrame* frame) {
   params.gesture = navigation_gesture_;
   navigation_gesture_ = NavigationGestureUnknown;
 
-  if (webview()->GetMainFrame() == frame) {
+  if (!frame->GetParent()) {
     // Top-level navigation.
 
     // Update contents MIME type for main frame.
@@ -1032,8 +1038,8 @@ void RenderView::UpdateURL(WebFrame* frame) {
 
     // We assume top level navigations initiated by the renderer are link
     // clicks.
-    params.transition = extra_data ?
-        extra_data->transition_type : PageTransition::LINK;
+    params.transition = navigation_state ?
+        navigation_state->transition_type : PageTransition::LINK;
     if (!PageTransition::IsMainFrame(params.transition)) {
       // If the main frame does a load, it should not be reported as a subframe
       // navigation.  This can occur in the following case:
@@ -1084,8 +1090,6 @@ void RenderView::UpdateURL(WebFrame* frame) {
     else
       params.transition = PageTransition::AUTO_SUBFRAME;
 
-    // The browser should never initiate a subframe navigation.
-    DCHECK(!extra_data);
     Send(new ViewHostMsg_FrameNavigate(routing_id_, params));
   }
 
@@ -1094,8 +1098,8 @@ void RenderView::UpdateURL(WebFrame* frame) {
 
   // If we end up reusing this WebRequest (for example, due to a #ref click),
   // we don't want the transition type to persist.
-  if (extra_data)
-    extra_data->transition_type = PageTransition::LINK;  // Just clear it.
+  if (navigation_state)
+    navigation_state->transition_type = PageTransition::LINK;  // Just clear it.
 
 #if defined(OS_WIN)
   if (web_accessibility_manager_.get()) {
@@ -1194,6 +1198,10 @@ void RenderView::DidStopLoading(WebView* webview) {
   ResetPendingUpload();
 }
 
+void RenderView::DidCreateDataSource(WebFrame* frame, WebDataSource* ds) {
+  ds->SetExtraData(pending_navigation_state_.release());
+}
+
 void RenderView::DidStartProvisionalLoadForFrame(
     WebView* webview,
     WebFrame* frame,
@@ -1207,12 +1215,10 @@ void RenderView::DidStartProvisionalLoadForFrame(
 
   WebDataSource* ds = frame->GetProvisionalDataSource();
   if (ds) {
-    const WebRequest& req = ds->GetRequest();
-    RenderViewExtraRequestData* extra_data =
-        static_cast<RenderViewExtraRequestData*>(req.GetExtraData());
-    if (extra_data) {
-      ds->SetRequestTime(extra_data->request_time);
-    }
+    NavigationState* navigation_state =
+        static_cast<NavigationState*>(ds->GetExtraData());
+    if (navigation_state)
+      ds->SetRequestTime(navigation_state->request_time);
   }
   Send(new ViewHostMsg_DidStartProvisionalLoadForFrame(
        routing_id_, webview->GetMainFrame() == frame,
@@ -1284,9 +1290,9 @@ void RenderView::DidFailProvisionalLoadWithError(WebView* webview,
   // 'replace' load.  This is necessary to avoid messing up session history.
   // Otherwise, we do a normal load, which simulates a 'go' navigation as far
   // as session history is concerned.
-  RenderViewExtraRequestData* extra_data =
-      static_cast<RenderViewExtraRequestData*>(failed_request.GetExtraData());
-  bool replace = extra_data && !extra_data->is_new_navigation();
+  NavigationState* navigation_state =
+      static_cast<NavigationState*>(ds->GetExtraData());
+  bool replace = navigation_state && !navigation_state->is_new_navigation();
 
   // Use the alternate error page service if this is a DNS failure or
   // connection failure.  ERR_CONNECTION_FAILED can be dropped once we no longer
@@ -1353,10 +1359,8 @@ void RenderView::LoadNavigationErrorPage(WebFrame* frame,
 
 void RenderView::DidCommitLoadForFrame(WebView *webview, WebFrame* frame,
                                        bool is_new_navigation) {
-  const WebRequest& request =
-      webview->GetMainFrame()->GetDataSource()->GetRequest();
-  RenderViewExtraRequestData* extra_data =
-      static_cast<RenderViewExtraRequestData*>(request.GetExtraData());
+  NavigationState* navigation_state = static_cast<NavigationState*>(
+      frame->GetDataSource()->GetExtraData());
 
   if (is_new_navigation) {
     // When we perform a new navigation, we need to update the previous session
@@ -1371,22 +1375,22 @@ void RenderView::DidCommitLoadForFrame(WebView *webview, WebFrame* frame,
                                           page_id_, true),
         kDelayForForcedCaptureMs);
   } else {
-    // Inspect the extra_data on the main frame (set in our Navigate method) to
-    // see if the navigation corresponds to a session history navigation...
-    // Note: |frame| may or may not be the toplevel frame, but for the case
-    // of capturing session history, the first committed frame suffices.  We
-    // keep track of whether we've seen this commit before so that only capture
-    // session history once per navigation.
+    // Inspect the navigation_state on the main frame (set in our Navigate
+    // method) to see if the navigation corresponds to a session history
+    // navigation...  Note: |frame| may or may not be the toplevel frame, but
+    // for the case of capturing session history, the first committed frame
+    // suffices.  We keep track of whether we've seen this commit before so
+    // that only capture session history once per navigation.
     //
     // Note that we need to check if the page ID changed. In the case of a
     // reload, the page ID doesn't change, and UpdateSessionHistory gets the
     // previous URL and the current page ID, which would be wrong.
-    if (extra_data && !extra_data->is_new_navigation() &&
-        !extra_data->request_committed &&
-        page_id_ != extra_data->pending_page_id()) {
+    if (navigation_state && !navigation_state->is_new_navigation() &&
+        !navigation_state->request_committed &&
+        page_id_ != navigation_state->pending_page_id()) {
       // This is a successful session history navigation!
       UpdateSessionHistory(frame);
-      page_id_ = extra_data->pending_page_id();
+      page_id_ = navigation_state->pending_page_id();
     }
   }
 
@@ -1395,8 +1399,8 @@ void RenderView::DidCommitLoadForFrame(WebView *webview, WebFrame* frame,
   // a session history navigation, because if we attempted a session history
   // navigation without valid HistoryItem state, WebCore will think it is a
   // new navigation.
-  if (extra_data)
-    extra_data->request_committed = true;
+  if (navigation_state)
+    navigation_state->request_committed = true;
 
   UpdateURL(frame);
 
@@ -1447,7 +1451,16 @@ void RenderView::DidHandleOnloadEventsForFrame(WebView* webview,
 void RenderView::DidChangeLocationWithinPageForFrame(WebView* webview,
                                                      WebFrame* frame,
                                                      bool is_new_navigation) {
+  // If this was a reference fragment navigation that we initiated, then we
+  // could end up having a non-null pending navigation state.  We just need to
+  // update the ExtraData on the datasource so that others who read the
+  // ExtraData will get the new NavigationState.  Similarly, if we did not
+  // initiate this navigation, then we need to take care to clear any pre-
+  // existing navigation state.
+  frame->GetDataSource()->SetExtraData(pending_navigation_state_.release());
+
   DidCommitLoadForFrame(webview, frame, is_new_navigation);
+
   const string16& title =
       webview->GetMainFrame()->GetDataSource()->GetPageTitle();
   UpdateTitle(frame, UTF16ToWideHack(title));
@@ -1550,27 +1563,30 @@ WindowOpenDisposition RenderView::DispositionForNavigationAction(
     WebNavigationType type,
     WindowOpenDisposition disposition,
     bool is_redirect) {
+  // GetExtraData is NULL when we did not issue the request ourselves (see
+  // OnNavigate), and so such a request may correspond to a link-click,
+  // script, or drag-n-drop initiated navigation.
+  bool is_content_initiated =
+      !frame->GetProvisionalDataSource()->GetExtraData();
+
   // Webkit is asking whether to navigate to a new URL.
   // This is fine normally, except if we're showing UI from one security
   // context and they're trying to navigate to a different context.
   const GURL& url = request->GetURL();
+
   // We only care about navigations that are within the current tab (as opposed
   // to, for example, opening a new window).
   // But we sometimes navigate to about:blank to clear a tab, and we want to
   // still allow that.
-  if (disposition == CURRENT_TAB && !(url.SchemeIs(chrome::kAboutScheme))) {
-    // GetExtraData is NULL when we did not issue the request ourselves (see
-    // OnNavigate), and so such a request may correspond to a link-click,
-    // script, or drag-n-drop initiated navigation.
-    if (frame == webview->GetMainFrame() && !request->GetExtraData()) {
-      // When we received such unsolicited navigations, we sometimes want to
-      // punt them up to the browser to handle.
-      if (BindingsPolicy::is_dom_ui_enabled(enabled_bindings_) ||
-          frame->GetInViewSourceMode() ||
-          url.SchemeIs(chrome::kViewSourceScheme)) {
-        OpenURL(webview, url, GURL(), disposition);
-        return IGNORE_ACTION;  // Suppress the load here.
-      }
+  if (disposition == CURRENT_TAB && is_content_initiated &&
+      frame->GetParent() == NULL && !url.SchemeIs(chrome::kAboutScheme)) {
+    // When we received such unsolicited navigations, we sometimes want to
+    // punt them up to the browser to handle.
+    if (BindingsPolicy::is_dom_ui_enabled(enabled_bindings_) ||
+        frame->GetInViewSourceMode() ||
+        url.SchemeIs(chrome::kViewSourceScheme)) {
+      OpenURL(webview, url, GURL(), disposition);
+      return IGNORE_ACTION;  // Suppress the load here.
     }
   }
 
@@ -1596,9 +1612,8 @@ WindowOpenDisposition RenderView::DispositionForNavigationAction(
       frame->GetOpener() == NULL &&
       // Must be a top-level frame.
       frame->GetParent() == NULL &&
-      // Must not have issued the request from this page.  GetExtraData is NULL
-      // when the navigation is being done by something outside the page.
-      !request->GetExtraData() &&
+      // Must not have issued the request from this page.
+      is_content_initiated &&
       // Must be targeted at the current tab.
       disposition == CURRENT_TAB &&
       // Must be a JavaScript navigation, which appears as "other".
@@ -2863,11 +2878,11 @@ void RenderView::DidAddHistoryItem() {
   WebDataSource* ds = main_frame->GetDataSource();
   DCHECK(ds != NULL);
 
-  const WebRequest& request = ds->GetRequest();
-  RenderViewExtraRequestData* extra_data =
-      static_cast<RenderViewExtraRequestData*>(request.GetExtraData());
+  NavigationState* navigation_state =
+      static_cast<NavigationState*>(ds->GetExtraData());
 
-  if (extra_data && extra_data->transition_type == PageTransition::START_PAGE)
+  if (navigation_state &&
+      navigation_state->transition_type == PageTransition::START_PAGE)
     return;
 
   history_back_list_count_++;
diff --git a/chrome/renderer/render_view.h b/chrome/renderer/render_view.h
index f42b8ff..4de8bb3 100644
--- a/chrome/renderer/render_view.h
+++ b/chrome/renderer/render_view.h
@@ -170,6 +170,7 @@ class RenderView : public RenderWidget,
 
   virtual void DidStartLoading(WebView* webview);
   virtual void DidStopLoading(WebView* webview);
+  virtual void DidCreateDataSource(WebFrame* frame, WebDataSource* ds);
   virtual void DidStartProvisionalLoadForFrame(
       WebView* webview,
       WebFrame* frame,
@@ -801,6 +802,13 @@ class RenderView : public RenderWidget,
   // The text selection the last time DidChangeSelection got called.
   std::string last_selection_;
 
+  // Holds state pertaining to a navigation that we initiated.  This is held by
+  // the WebDataSource::ExtraData attribute.  We use pending_navigation_state_
+  // as a temporary holder for the state until the WebDataSource corresponding
+  // to the new navigation is created.  See DidCreateDataSource.
+  class NavigationState;
+  scoped_ptr<NavigationState> pending_navigation_state_;
+
   DISALLOW_COPY_AND_ASSIGN(RenderView);
 };
 
diff --git a/webkit/api/public/WebDataSource.h b/webkit/api/public/WebDataSource.h
index ef96042..b163221 100644
--- a/webkit/api/public/WebDataSource.h
+++ b/webkit/api/public/WebDataSource.h
@@ -1,10 +1,10 @@
 /*
  * Copyright (C) 2009 Google Inc. All rights reserved.
- * 
+ *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
- * 
+ *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ -14,7 +14,7 @@
  *     * Neither the name of Google Inc. nor the names of its
  * contributors may be used to endorse or promote products derived from
  * this software without specific prior written permission.
- * 
+ *
  * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
  * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
  * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
@@ -58,7 +58,7 @@ namespace WebKit {
         // to a location specified by a redirect that was followed.
         virtual const WebURLRequest& request() const = 0;
 
-        // Returns the response associated to this datasource.
+        // Returns the response associated with this datasource.
         virtual const WebURLResponse& response() const = 0;
 
         // When this datasource was created as a result of WebFrame::loadData,
diff --git a/webkit/glue/webdatasource.h b/webkit/glue/webdatasource.h
index b0545925..53e9492 100644
--- a/webkit/glue/webdatasource.h
+++ b/webkit/glue/webdatasource.h
@@ -32,6 +32,13 @@ enum WebNavigationType {
 
 class WebDataSource {
  public:
+  // A base class for extra data that may be associated with this datasource.
+  // See Set/GetExtraData below.
+  class ExtraData {
+   public:
+    virtual ~ExtraData() {}
+  };
+
   virtual ~WebDataSource() {}
 
   // Returns a reference to the original request data that created the
@@ -119,6 +126,13 @@ class WebDataSource {
 
   // Returns the reason the document was loaded.
   virtual WebNavigationType GetNavigationType() const = 0;
+
+  // Extra data associated with this datasource.  If non-null, the extra data
+  // pointer will be deleted when the datasource is destroyed.  Setting the
+  // extra data pointer will cause any existing non-null extra data pointer to
+  // be deleted.
+  virtual ExtraData* GetExtraData() const = 0;
+  virtual void SetExtraData(ExtraData* extra_data) = 0;
 };
 
 #endif  // #ifndef WEBKIT_GLUE_WEBDATASOURCE_H_
diff --git a/webkit/glue/webdatasource_impl.cc b/webkit/glue/webdatasource_impl.cc
index fb1f1c2..86767ca 100644
--- a/webkit/glue/webdatasource_impl.cc
+++ b/webkit/glue/webdatasource_impl.cc
@@ -60,11 +60,6 @@ const WebResponse& WebDataSourceImpl::GetResponse() const {
   return response_;
 }
 
-void WebDataSourceImpl::SetExtraData(WebRequest::ExtraData* extra) {
-  initial_request_.SetExtraData(extra);
-  request_.SetExtraData(extra);
-}
-
 GURL WebDataSourceImpl::GetUnreachableURL() const {
   const WebCore::KURL& url = unreachableURL();
   return url.isEmpty() ? GURL() : webkit_glue::KURLToGURL(url);
@@ -130,6 +125,14 @@ WebNavigationType WebDataSourceImpl::GetNavigationType() const {
   return NavigationTypeToWebNavigationType(triggeringAction().type());
 }
 
+WebDataSource::ExtraData* WebDataSourceImpl::GetExtraData() const {
+  return extra_data_.get();
+}
+
+void WebDataSourceImpl::SetExtraData(ExtraData* extra_data) {
+  extra_data_.set(extra_data);
+}
+
 WebNavigationType WebDataSourceImpl::NavigationTypeToWebNavigationType(
     WebCore::NavigationType type) {
   switch (type) {
diff --git a/webkit/glue/webdatasource_impl.h b/webkit/glue/webdatasource_impl.h
index d4bef9c..912dffb 100644
--- a/webkit/glue/webdatasource_impl.h
+++ b/webkit/glue/webdatasource_impl.h
@@ -44,17 +44,12 @@ class WebDataSourceImpl : public WebCore::DocumentLoader, public WebDataSource {
   virtual base::Time GetFinishLoadTime() const;
   virtual base::Time GetFirstLayoutTime() const;
   virtual WebNavigationType GetNavigationType() const;
+  virtual ExtraData* GetExtraData() const;
+  virtual void SetExtraData(ExtraData*);
 
   static WebNavigationType NavigationTypeToWebNavigationType(
       WebCore::NavigationType type);
 
-  // Called after creating a new data source if there is request info
-  // available. Since we store copies of the WebRequests, the original
-  // WebRequest that the embedder created was lost, and the exra data would
-  // go with it. This preserves the request info so retrieving the requests
-  // later will have the same data.
-  void SetExtraData(WebRequest::ExtraData* extra);
-
   void ClearRedirectChain();
   void AppendRedirect(const GURL& url);
 
@@ -127,6 +122,8 @@ class WebDataSourceImpl : public WebCore::DocumentLoader, public WebDataSource {
   scoped_ptr<const SearchableFormData> searchable_form_data_;
   scoped_ptr<const PasswordForm> password_form_data_;
 
+  OwnPtr<ExtraData> extra_data_;
+
   bool form_submit_;
 
   // See webdatasource.h for a description of these time stamps.
diff --git a/webkit/glue/webframe_impl.cc b/webkit/glue/webframe_impl.cc
index 11675ce..ea5fcfb 100644
--- a/webkit/glue/webframe_impl.cc
+++ b/webkit/glue/webframe_impl.cc
@@ -462,13 +462,6 @@ void WebFrameImpl::InternalLoadRequest(const WebRequest* request,
   // WebFrameLoaderClient::createDocumentLoader.
   currently_loading_request_ = request;
 
-  // If we have a current datasource, save the request info on it immediately.
-  // This is because WebCore may not actually initiate a load on the toplevel
-  // frame for some subframe navigations, so we want to update its request.
-  WebDataSourceImpl* datasource = GetDataSourceImpl();
-  if (datasource)
-    CacheCurrentRequestInfo(datasource);
-
   if (data.isValid()) {
     frame_->loader()->load(resource_request, data, false);
     if (replace) {
@@ -659,20 +652,6 @@ WebDataSourceImpl* WebFrameImpl::GetProvisionalDataSourceImpl() const {
   return static_cast<WebDataSourceImpl*>(GetProvisionalDataSource());
 }
 
-void WebFrameImpl::CacheCurrentRequestInfo(WebDataSourceImpl* datasource) {
-  // Cache our current request info on the data source.  It contains its
-  // own requests, so the extra data needs to be transferred.
-  scoped_refptr<WebRequest::ExtraData> extra;
-
-  // Our extra data may come from a request issued via LoadRequest.
-  if (currently_loading_request_)
-    extra = currently_loading_request_->GetExtraData();
-
-  // We must only update this if it is valid, or the valid state will be lost.
-  if (extra)
-    datasource->SetExtraData(extra);
-}
-
 void WebFrameImpl::StopLoading() {
   if (!frame_)
     return;
diff --git a/webkit/glue/webframe_impl.h b/webkit/glue/webframe_impl.h
index 45cdc91..2d51ed6 100644
--- a/webkit/glue/webframe_impl.h
+++ b/webkit/glue/webframe_impl.h
@@ -221,10 +221,6 @@ class WebFrameImpl : public WebFrame, public base::RefCounted<WebFrameImpl> {
     return frame_ ? frame_->view() : NULL;
   }
 
-  // Update the given datasource with currently_loading_request's info.
-  // If currently_loading_request is NULL, does nothing.
-  void CacheCurrentRequestInfo(WebDataSourceImpl* datasource);
-
   // Getters for the impls corresponding to Get(Provisional)DataSource. They
   // may return NULL if there is no corresponding data source.
   WebDataSourceImpl* GetDataSourceImpl() const;
diff --git a/webkit/glue/webframeloaderclient_impl.cc b/webkit/glue/webframeloaderclient_impl.cc
index a1bed38..30aba5e 100644
--- a/webkit/glue/webframeloaderclient_impl.cc
+++ b/webkit/glue/webframeloaderclient_impl.cc
@@ -687,12 +687,6 @@ void WebFrameLoaderClient::dispatchDidChangeLocationWithinPage() {
     // Regardless of how we got here, we are navigating to a URL so we need to
     // add it to the redirect chain.
     ds->AppendRedirect(url);
-
-    // WebKit will re-use requests for in-page navigations, but we want to
-    // think of it as a new request that has a page ID in session history.
-    // This will set the proper page ID, etc. on the request so that the
-    // browser will treat it properly.
-    webframe_->CacheCurrentRequestInfo(ds);
   }
 
   bool is_new_navigation;
@@ -1347,7 +1341,9 @@ PassRefPtr<DocumentLoader> WebFrameLoaderClient::createDocumentLoader(
     const ResourceRequest& request,
     const SubstituteData& data) {
   RefPtr<WebDataSourceImpl> ds = WebDataSourceImpl::Create(request, data);
-  webframe_->CacheCurrentRequestInfo(ds.get());
+  WebViewDelegate* d = webframe_->GetWebViewImpl()->delegate();
+  if (d)
+    d->DidCreateDataSource(webframe_, ds.get());
   return ds.release();
 }
 
diff --git a/webkit/glue/weburlrequest.h b/webkit/glue/weburlrequest.h
index 94d14c4..d4c3596 100644
--- a/webkit/glue/weburlrequest.h
+++ b/webkit/glue/weburlrequest.h
@@ -27,35 +27,12 @@ class WebRequest {
  public:
   typedef std::map<std::string, std::string> HeaderMap;
 
-  // Extra information that is associated with a request. The embedder derives
-  // from this REFERENCE COUNTED class to associated data with a request and
-  // get it back when the page loads.
-  //
-  // Note that for reloads (and possibly things like back/forward), there is no
-  // way to specify the request that it will use, so the extra data pointer
-  // will be invalid. Users should always check for NULL.
-  class ExtraData : public base::RefCounted<ExtraData> {
-   public:
-    virtual ~ExtraData() {}
-  };
-
   // Creates a WebRequest.
   static WebRequest* Create(const GURL& url);
 
   // Creates a copy of this WebRequest.
   virtual WebRequest* Clone() const = 0;
 
-  // Sets the extra request info that the embedder can retrieve later. This
-  // will AddRef the ExtraData and store it with the request.
-  virtual void SetExtraData(ExtraData* extra) = 0;
-
-  // Returns any previously set request info. It does not AddRef, the caller
-  // is assumed to assign this to a RefPtr. This may return NULL if no extra
-  // data has been set on this request. Even if the embedder sets request info
-  // for every request, WebRequests can get created during reload operations
-  // so callers should not assume the data is always valid.
-  virtual ExtraData* GetExtraData() const = 0;
-
   // Get/set the URL.
   virtual GURL GetURL() const = 0;
   virtual void SetURL(const GURL& url) = 0;
diff --git a/webkit/glue/weburlrequest_impl.cc b/webkit/glue/weburlrequest_impl.cc
index 655eb0f..f8ff805 100644
--- a/webkit/glue/weburlrequest_impl.cc
+++ b/webkit/glue/weburlrequest_impl.cc
@@ -44,14 +44,6 @@ WebRequest* WebRequestImpl::Clone() const {
   return new WebRequestImpl(*this);
 }
 
-void WebRequestImpl::SetExtraData(ExtraData* extra) {
-  extra_data_ = extra;
-}
-
-WebRequest::ExtraData* WebRequestImpl::GetExtraData() const {
-  return extra_data_.get();
-}
-
 GURL WebRequestImpl::GetURL() const {
   return webkit_glue::KURLToGURL(request_.resourceRequest().url());
 }
diff --git a/webkit/glue/weburlrequest_impl.h b/webkit/glue/weburlrequest_impl.h
index 968dbc0..5a613b3 100644
--- a/webkit/glue/weburlrequest_impl.h
+++ b/webkit/glue/weburlrequest_impl.h
@@ -23,8 +23,6 @@ class WebRequestImpl : public WebRequest {
 
   // WebRequest
   virtual WebRequest* Clone() const;
-  virtual void SetExtraData(ExtraData* extra);
-  virtual ExtraData* GetExtraData() const;
   virtual void SetURL(const GURL& url);
   virtual GURL GetURL() const;
   virtual void SetMainDocumentURL(const GURL& url);
@@ -63,7 +61,6 @@ class WebRequestImpl : public WebRequest {
  protected:
   WebCore::FrameLoadRequest request_;
   RefPtr<WebCore::HistoryItem> history_item_;
-  scoped_refptr<ExtraData> extra_data_;
 };
 
 #endif  // #ifndef WEBKIT_GLUE_WEBURLREQUEST_IMPL_H_
diff --git a/webkit/glue/webview_delegate.h b/webkit/glue/webview_delegate.h
index 7d503ad..6be9b43 100644
--- a/webkit/glue/webview_delegate.h
+++ b/webkit/glue/webview_delegate.h
@@ -229,6 +229,11 @@ class WebViewDelegate : virtual public WebWidgetDelegate {
 
   // FrameLoadDelegate -------------------------------------------------------
 
+  // A datasource has been created for a new navigation.  The given datasource
+  // will become the provisional datasource for the frame.
+  virtual void DidCreateDataSource(WebFrame* frame, WebDataSource* ds) {
+  }
+
   // Notifies the delegate that the provisional load of a specified frame in a
   // given WebView has started. By the time the provisional load for a frame has
   // started, we know whether or not the current load is due to a client
diff --git a/webkit/tools/test_shell/test_navigation_controller.h b/webkit/tools/test_shell/test_navigation_controller.h
index d26e994..f0d9fca 100644
--- a/webkit/tools/test_shell/test_navigation_controller.h
+++ b/webkit/tools/test_shell/test_navigation_controller.h
@@ -12,17 +12,16 @@
 #include "base/linked_ptr.h"
 #include "base/ref_counted.h"
 #include "googleurl/src/gurl.h"
-#include "webkit/glue/weburlrequest.h"
+#include "webkit/glue/webdatasource.h"
 
 class GURL;
 class TestShell;
 
 // Associated with browser-initated navigations to hold tracking data.
-class TestShellExtraRequestData : public WebRequest::ExtraData {
+class TestShellExtraData : public WebDataSource::ExtraData {
  public:
-  TestShellExtraRequestData(int32 pending_page_id)
-      : WebRequest::ExtraData(),
-        pending_page_id(pending_page_id),
+  TestShellExtraData(int32 pending_page_id)
+      : pending_page_id(pending_page_id),
         request_committed(false) {
   }
 
diff --git a/webkit/tools/test_shell/test_shell.cc b/webkit/tools/test_shell/test_shell.cc
index 68247d9..f2d5b0c 100644
--- a/webkit/tools/test_shell/test_shell.cc
+++ b/webkit/tools/test_shell/test_shell.cc
@@ -510,8 +510,13 @@ bool TestShell::Navigate(const TestNavigationEntry& entry, bool reload) {
     if (!reload)
       request->SetHistoryState(entry.GetContentState());
 
-    request->SetExtraData(
-        new TestShellExtraRequestData(entry.GetPageID()));
+    // A navigation resulting from loading a javascript URL should not be
+    // treated as a browser initiated event.  Instead, we want it to look as if
+    // the page initiated any load resulting from JS execution.
+    if (!entry.GetURL().SchemeIs("javascript")) {
+      delegate_->set_pending_extra_data(
+          new TestShellExtraData(entry.GetPageID()));
+    }
 
     // Get the right target frame for the entry.
     WebFrame* frame = webView()->GetMainFrame();
@@ -521,6 +526,10 @@ bool TestShell::Navigate(const TestNavigationEntry& entry, bool reload) {
     // back/forward navigations maintain the target frame?
 
     frame->LoadRequest(request.get());
+
+    // In case LoadRequest failed before DidCreateDataSource was called.
+    delegate_->set_pending_extra_data(NULL);
+
     // Restore focus to the main frame prior to loading new request.
     // This makes sure that we don't have a focused iframe. Otherwise, that
     // iframe would keep focus when the SetFocus called immediately after
diff --git a/webkit/tools/test_shell/test_webview_delegate.cc b/webkit/tools/test_shell/test_webview_delegate.cc
index 6fb1b56..353f4fb 100644
--- a/webkit/tools/test_shell/test_webview_delegate.cc
+++ b/webkit/tools/test_shell/test_webview_delegate.cc
@@ -243,6 +243,11 @@ void TestWebViewDelegate::DidFailLoadingWithError(WebView* webview,
   resource_identifier_map_.erase(identifier);
 }
 
+void TestWebViewDelegate::DidCreateDataSource(WebFrame* frame,
+                                              WebDataSource* ds) {
+  ds->SetExtraData(pending_extra_data_.release());
+}
+
 void TestWebViewDelegate::DidStartProvisionalLoadForFrame(
     WebView* webview,
     WebFrame* frame,
@@ -296,13 +301,13 @@ void TestWebViewDelegate::DidFailProvisionalLoadWithError(
   if (error.GetErrorCode() == net::ERR_ABORTED)
     return;
 
-  const WebRequest& failed_request =
-      frame->GetProvisionalDataSource()->GetRequest();
-  TestShellExtraRequestData* extra_data =
-      static_cast<TestShellExtraRequestData*>(failed_request.GetExtraData());
+  const WebDataSource* failed_ds = frame->GetProvisionalDataSource();
+
+  TestShellExtraData* extra_data =
+      static_cast<TestShellExtraData*>(failed_ds->GetExtraData());
   bool replace = extra_data && extra_data->pending_page_id != -1;
 
-  scoped_ptr<WebRequest> request(failed_request.Clone());
+  scoped_ptr<WebRequest> request(failed_ds->GetRequest().Clone());
 
   std::string error_text =
       StringPrintf("Error %d when loading url %s", error.GetErrorCode(),
@@ -386,6 +391,8 @@ void TestWebViewDelegate::DidHandleOnloadEventsForFrame(WebView* webview,
 
 void TestWebViewDelegate::DidChangeLocationWithinPageForFrame(
     WebView* webview, WebFrame* frame, bool is_new_navigation) {
+  frame->GetDataSource()->SetExtraData(pending_extra_data_.release());
+
   if (shell_->ShouldDumpFrameLoadCallbacks()) {
     printf("%S - didChangeLocationWithinPageForFrame\n",
            GetFrameDescription(frame).c_str());
@@ -809,13 +816,9 @@ WebWidgetHost* TestWebViewDelegate::GetHostForWidget(WebWidget* webwidget) {
 
 void TestWebViewDelegate::UpdateForCommittedLoad(WebFrame* frame,
                                                  bool is_new_navigation) {
-  WebView* webview = shell_->webView();
-
   // Code duplicated from RenderView::DidCommitLoadForFrame.
-  const WebRequest& request =
-      webview->GetMainFrame()->GetDataSource()->GetRequest();
-  TestShellExtraRequestData* extra_data =
-      static_cast<TestShellExtraRequestData*>(request.GetExtraData());
+  TestShellExtraData* extra_data = static_cast<TestShellExtraData*>(
+      frame->GetDataSource()->GetExtraData());
 
   if (is_new_navigation) {
     // New navigation.
diff --git a/webkit/tools/test_shell/test_webview_delegate.h b/webkit/tools/test_shell/test_webview_delegate.h
index 16db870..9bf0245 100644
--- a/webkit/tools/test_shell/test_webview_delegate.h
+++ b/webkit/tools/test_shell/test_webview_delegate.h
@@ -22,6 +22,7 @@
 
 #include "base/basictypes.h"
 #include "base/ref_counted.h"
+#include "base/scoped_ptr.h"
 #include "webkit/glue/webcursor.h"
 #include "webkit/glue/webview_delegate.h"
 #include "webkit/glue/webwidget_delegate.h"
@@ -29,6 +30,7 @@
 #include "webkit/tools/test_shell/drag_delegate.h"
 #include "webkit/tools/test_shell/drop_delegate.h"
 #endif
+#include "webkit/tools/test_shell/test_navigation_controller.h"
 
 struct WebPreferences;
 class GURL;
@@ -123,6 +125,8 @@ class TestWebViewDelegate : public base::RefCounted<TestWebViewDelegate>,
                                int edit_flags,
                                const std::string& security_info,
                                const std::string& frame_charset);
+  virtual void DidCreateDataSource(WebFrame* frame,
+                                   WebDataSource* ds);
   virtual void DidStartProvisionalLoadForFrame(
     WebView* webview,
     WebFrame* frame,
@@ -261,6 +265,10 @@ class TestWebViewDelegate : public base::RefCounted<TestWebViewDelegate>,
     captured_context_menu_events_.clear();
   }
 
+  void set_pending_extra_data(TestShellExtraData* extra_data) {
+    pending_extra_data_.reset(extra_data);
+  }
+
   // Methods for modifying WebPreferences
   void SetUserStyleSheetEnabled(bool is_enabled);
   void SetUserStyleSheetLocation(const GURL& location);
@@ -327,6 +335,8 @@ class TestWebViewDelegate : public base::RefCounted<TestWebViewDelegate>,
   int page_id_;
   int last_page_id_updated_;
 
+  scoped_ptr<TestShellExtraData> pending_extra_data_;
+
   // Maps resource identifiers to a descriptive string.
   typedef std::map<uint32, std::string> ResourceMap;
   ResourceMap resource_identifier_map_;