Bug 22295 - track which history items are from page load failures
Summary: track which history items are from page load failures
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: History (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Enhancement
Assignee: Darin Adler
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-11-16 10:20 PST by Darin Adler
Modified: 2008-11-24 14:49 PST (History)
1 user (show)

See Also:


Attachments
patch (22.18 KB, patch)
2008-11-16 10:30 PST, Darin Adler
no flags Details | Formatted Diff | Diff
patch (12.13 KB, patch)
2008-11-24 13:39 PST, Darin Adler
mitz: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2008-11-16 10:20:22 PST
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.
Comment 1 Darin Adler 2008-11-16 10:30:27 PST
Created attachment 25197 [details]
patch
Comment 2 mitz 2008-11-16 11:12:03 PST
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
Comment 3 Darin Adler 2008-11-16 11:14:27 PST
(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?
Comment 4 Darin Adler 2008-11-16 11:15:40 PST
(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.
Comment 5 Darin Adler 2008-11-16 11:39:42 PST
http://trac.webkit.org/changeset/38449
Comment 6 Darin Adler 2008-11-24 13:26:46 PST
This only worked for back/forward history, not global history. Oops!
Comment 7 Darin Adler 2008-11-24 13:39:40 PST
Created attachment 25446 [details]
patch
Comment 8 mitz 2008-11-24 13:47:07 PST
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
Comment 9 Darin Adler 2008-11-24 13:57:38 PST
(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.
Comment 10 mitz 2008-11-24 13:59:04 PST
(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.
Comment 11 Darin Adler 2008-11-24 14:49:01 PST
http://trac.webkit.org/changeset/38728