[Webkit-unassigned] [Bug 89230] Add support for application cache prefer-online mode

[bugzilla-daemon at webkit.org]

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





--- Comment #8 from huangxueqing <huangxueqing at baidu.com>  2012-08-25 03:08:53 PST ---
(In reply to comment #7)
> (From update of attachment 157872 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=157872&action=review
> 
> I don't think that this is implementing anything like what the spec says. The prefer-online mode changes navigation behavior, not subresource fetching. If you are actually implementing the spec, please add per-function comments in ChangeLog, explaining why each change is being made.
> 
Yes, we should only change the master entry's fetching, I have added comments in ChangeLog.

> This patch didn't apply cleanly. It needs to be updated to apply to current tip of tree, so that EWS could check it.
> 
Done

> > LayoutTests/http/tests/appcache/prefer-online.html:5
> > +<div id="result"></duv>
> 
> Typo: duv.
> 
> > LayoutTests/http/tests/appcache/prefer-online.html:33
> > +    if (load("resources/counter.php") == load("resource/counter.php")) {
> 
> The second URL is wrong here.
> 
> It's not great that a mistake like this invalidates the whole test.
> 
Sorry for typo, I have checked whole patch.

> > LayoutTests/http/tests/appcache/resources/prefer-online.manifest:5
> > +prefer-online
> 
> Please add tests that verify how whitespace is handled. From the spec:
> 
> When the current section is the settings section, data lines must consist of zero or more U+0020 SPACE and U+0009 CHARACTER TABULATION (tab) characters, a setting, and then zero or more U+0020 SPACE and U+0009 CHARACTER TABULATION (tab) characters.
> 
> Also, please test what happens when unknown settings are encountered.
> 
> Please test empty lines in the SETTINGS section.
> 
Done

> > Source/WebCore/loader/appcache/ApplicationCacheHost.cpp:446
> > +    return resource ? true: false;
> 
> There is no need for the ternary operator, a pointer will be cast to a boolean implicitly.
> 
Done

> > Source/WebCore/loader/appcache/ApplicationCacheHost.cpp:466
> > +bool ApplicationCacheHost::scheduleLoadExcplicitResourceFromApplicationCache(ResourceLoader* loader, ApplicationCache* cache)
> 
> Typo: Excplicit.
> 
Done

> > Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp:619
> > +    executeSQLCommand("CREATE TABLE IF NOT EXISTS CacheModes (cacheMode INTERGER NOT NULL ON CONFLICT FAIL, cache INTERGER NOT NULL ON CONFLICT FAIL)");
> 
> Typo: INTERGER. How did you test this code?
A case was added for this code, html file has manifest was appended to document then was removed, which destroy ApplicationCacheGroup and ApplicationCache, append html file again will load cache from database.

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