Bug 22295 – track which history items are from page load failures

• Status: RESOLVED
• Resolution: FIXED
• Component: History

[Darin Adler]

It's useful to distinguish history items for failures such as "domain not found" from history items that represent successful navigation and loading of a website.

[Darin Adler]

Created attachment 25197 [details]
patch

Comment on attachment 25197 [details]
patch

> +#include "CachedPage.h"
> +#include "CString.h"

Should headers #includes be sorted alphabetically or in ASCII order? I thought the latter.

It seems a little dangerous that the presence of the lastVisitWasFailure key in the dictionary representation always means "true".

r=me

[Darin Adler]

(In reply to comment #2)
> It seems a little dangerous that the presence of the lastVisitWasFailure key in
> the dictionary representation always means "true".

I could change the rule to be that it has to be exactly "boolean true". Or that it can be anything except "boolean false". Or that any numeric thing that's non-zero is counted as true.

Do you have a preference?

[Darin Adler]

(In reply to comment #2)
> > +#include "CachedPage.h"
> > +#include "CString.h"
> 
> Should headers #includes be sorted alphabetically or in ASCII order? I thought
> the latter.

I used the Xcode sort, which I thought was the same as the command line sort. I'll redo it. The ASCII order you get from the sort tool is our standard I think.

[Darin Adler]

http://trac.webkit.org/changeset/38449

[Darin Adler]

This only worked for back/forward history, not global history. Oops!

[Darin Adler]

Created attachment 25446 [details]
patch

Comment on attachment 25446 [details]
patch

Can you assert that the url parameter to updateGlobalHistory is equal to the documentLoader's urlForHistory()?

> +    [[WebHistory optionalSharedHistory] _visitedURL:cocoaURL withTitle:title wasFailure:wasFailure];

I think you can get rid of cocoaURL and pass url directly.

r=me

[Darin Adler]

(In reply to comment #8)
> Can you assert that the url parameter to updateGlobalHistory is equal to the
> documentLoader's urlForHistory()?

I'd much rather remove the url parameter altogether. In a future patch. You think that's OK?

> > +    [[WebHistory optionalSharedHistory] _visitedURL:cocoaURL withTitle:title wasFailure:wasFailure];
> 
> I think you can get rid of cocoaURL and pass url directly.

I'll try it and land it that way if it compiles.

(In reply to comment #9)
> (In reply to comment #8)
> > Can you assert that the url parameter to updateGlobalHistory is equal to the
> > documentLoader's urlForHistory()?
> 
> I'd much rather remove the url parameter altogether. In a future patch. You
> think that's OK?

Yes.

[Darin Adler]

http://trac.webkit.org/changeset/38728