[Webkit-unassigned] [Bug 27444] Wrong FrameLoader::activeDocumentLoader when loading an invalid URL.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=27444





--- Comment #4 from Brady Eidson <beidson at apple.com>  2009-07-20 14:22:38 PDT ---
(In reply to comment #3)
> > Also, if a layout test times out with your patch in but runs correctly without
> > your patch, that *is* indicative of a regression with your patch.  I bet the
> > test is catching exactly the WebKit API breakage that I alluded to.
> 
> i mis-explained maybe, but there is one less failing test w/ the patch.

Ah, yes I understood you to mean that your patch introduced a failure.  Since
the Mac bots are all running green right now, I assume you mean there's a Qt
failure that this cleans up.  But have you tried running the Mac tests with
your patch in?

> > Most importantly, a Frame shouldn't ever consider a failed, non-committed load
> > as it's current DocumentLoader.  Making this change would be a change in
> > behavior that many WebKit applications do not expect. Most notably, Safari
> > would break with this change.
> 
> is there any regression test for it ?

I don't know - Do you have a Mac machine to try with your patch? 

It's quite possible there isn't, as one thing I know this patch breaks - which
is the WebKit API [WebDataSource unreachableURL] - has historically been
difficult to test automatically.

If your patch *doesn't* break any Mac layout tests, then we need to take the
time to add one.

> > Can you explain an example of an application you're working on where this
> > change is desirable?  And why?
> 
> I should had mentioned it before, yes. I am working on
> https://bugs.webkit.org/show_bug.cgi?id=25867 (see patch 0.4) , which provides
> the url originally loaded by the frame.
> 
> in case of load failures, frameLoader->activeDocumentLoader points to a
> documentLoader instance of a (successful) previous load (see bug description).
> In this case, if i 
> 
> 1) load google.com
> 2) load a known unexistent url
> 3) call originalUrl()
> 4) it calls FrameLoader::originalRequest() , which calls
> activeDocumentLoader()->originalRequest.url() => http://google.com
> 
> since activeDocumentLoader is not referring to the latest load but to the
> latest successful one.
> 
> so how do access the documentLoader that actually refers to the latest load
> (being in successful or not) ?

I followed your description here and looked at the other bug.  Amusingly, the
main thing this patch would break is what happens in the error case, which
covered by DocumentLoader::unreachableURL().

When you say "latest load", you are overloading some highly overloaded
terminology.  In FrameLoader/DocumentLoader-land, the "current load" is the
most recent main resource load to have been committed.  When a load fails
because the url wasn't even valid, it was never committed, so the current
activeDocumentLoader() remains unchanged.

Alas, information about this attempted-load-not-even-getting-started is tacked
on the DocumentLoader.  Based on reading the other bug, you are happy with
url() and originalURL(), but just want to know about failure cases.  What
happens if you try using activeDocumentLoader()->mainDocumentError()?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.