Move ExtraData from WebRequest to WebDataSource....

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


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=16580

[brettw, 2009-05-20 22:23:06]

http://codereview.chromium.org/115575/diff/43/1022
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/115575/diff/43/1022#newcode812
Line 812: if (!params.url.SchemeIs("javascript")) {
chrome::kJavascriptScheme instead of hard-coding?

http://codereview.chromium.org/115575/diff/43/1022#newcode1033
Line 1033: if (!frame->GetParent()) {
Is there a reason you changed it? It's possible to have disembodied HTML
elements, I wonder if a subframe removed from the document could load and have
no parent?

http://codereview.chromium.org/115575/diff/43/1022#newcode1378
Line 1378: // Inspect the navigation_state on the main frame (set in our
Navigate method) to
You made this line too long.

http://codereview.chromium.org/115575/diff/43/1022#newcode1458
Line 1458:
frame->GetDataSource()->SetExtraData(pending_navigation_state_.release());
This could be NULL, though, right? Do you think it's worth mentioning that
possibility?

http://codereview.chromium.org/115575/diff/43/1012
File webkit/glue/webdatasource.h (right):

http://codereview.chromium.org/115575/diff/43/1012#newcode35
Line 35: class ExtraData {
Can you provide a comment for this? Also, indent to Google-style. Maybe it just
says "base class for blah, see blah below."

http://codereview.chromium.org/115575/diff/43/1012#newcode133
Line 133: virtual void SetExtraData(ExtraData*) = 0;
Usually we give names to these params.

[brettw, 2009-05-20 23:41:04]

LGTM, good luck!

http://codereview.chromium.org/115575/diff/50/67
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/115575/diff/50/67#newcode1033
Line 1033: if (!frame->GetParent()) {
This is another place you changed the parent thing.

[darin (slow to review), 2009-05-21 00:06:02]

http://codereview.chromium.org/115575/diff/43/1022
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/115575/diff/43/1022#newcode1033
Line 1033: if (!frame->GetParent()) {
I made this change to be consistent with "is top most frame" checks elsewhere in
this file.

If it did not have a parent, then it would not have a WebView, and there would
be no WebViewDelegate.  I think it is also not possible to navigate an IFRAME
that is detached from the DOM.

http://codereview.chromium.org/115575/diff/43/1022#newcode1458
Line 1458:
frame->GetDataSource()->SetExtraData(pending_navigation_state_.release());
pending_navigation_state_ will be null when the browser did not initiate the
navigation.  in that case, we want to clear the extradata so that we don't
interpret pre-existing extradata.  i'll add a comment to that effect.

http://codereview.chromium.org/115575/diff/43/1012
File webkit/glue/webdatasource.h (right):

http://codereview.chromium.org/115575/diff/43/1012#newcode35
Line 35: class ExtraData {
argh... too much webkit coding for me lately :(

http://codereview.chromium.org/115575/diff/50/67
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/115575/diff/50/67#newcode1033
Line 1033: if (!frame->GetParent()) {
Yeah... cross your fingers for me!