Bug 55129 – is this a bug?

• Status: RESOLVED
• Resolution: INVALID
• Component: WebCore Misc.

[zywu]

Hi,

Source code url:
http://trac.webkit.org/browser/trunk/Source/WebCore/loader/FrameLoader.cpp

2326	
2327	    // FIXME: POST documents are always Reloads, but their subresources should still be Revalidate.
2328	    // If we bring the CachePolicy.h and ResourceRequest cache policy enums in sync with each other and
2329	    // remember "Revalidate" in ResourceRequests, we can remove this "POST" check and return either "Reload"
2330	    // or "Revalidate" if the DocumentLoader was requested with either.
2331	    const ResourceRequest& request(documentLoader()->request());
2332	    if (request.cachePolicy() == ReloadIgnoringCacheData && !equalIgnoringCase(request.httpMethod(), "post"))

I think the line 2332 should change to 

-2332	    if (request.cachePolicy() == ReloadIgnoringCacheData && !equalIgnoringCase(request.httpMethod(), "post"))
+2332        if (request.cachePolicy() == ReloadIgnoringCacheData && equalIgnoringCase(request.httpMethod(), "post"))

I'm not sure this is a bug or not. can someone take a look at this issue?
Thanks.

[Alexey Proskuryakov]

This line of code works as originally designed. This function returns cache policy for subresources, and the comment explains why POST is a special case.

However, please note that this behavior is changing now anyway in bug 38690.

[zywu]

Hi alexy,

THanks for your reply.

I know the purpose of this line,
but the question is 
equalIgnoringCase(request.httpMethod(), "post") will return true is the request.httpMethod() is equal to "post". otherwise is it will return false.

if we want "post" method special handled.
then I think 
!equalIgnoringCase(request.httpMethod(), "post")
should change to 
equalIgnoringCase(request.httpMethod(), "post")

am i missing anything?


Thanks again.

[Alexey Proskuryakov]

If you think that there is a bug, please provide a test case with steps to reproduce. I don't know how to prove correctness of a single code line, I only know how to prove incorrectness with a test case that works wrong.

[zywu]

thanks for your reply.

we are using webkit in a embedded system. so it may hard for you to reproduce.
Basically, this will affect second time load the same page by using "GET" method.

reproduce step.
1) load one page.
2) load the same page again.

you will notice webkit will revalidate all of sub resource by checking the http return code.
if the http return code is 304, then it will use the resource in cache.
for PC, you maybe not notice this issue. but for embedded system, it cause reload same page speed issue.

Thanks!

[Alexey Proskuryakov]

Yes, this is expected behavior - when a user manually loads the same URL in the same browser window again, we decide that a refresh is desired, and do just that. It would be a disservice to just show old cached data when the user explicitly performs an action proving that they hope to see updated data.

[zywu]

Thanks for your reply.

1)
According to the comment "// FIXME: POST documents are always Reloads, but their subresources should still be Revalidate."

so if the request method is "post",then subresourceCachePolicy() should return
"CachePolicyRevalidate".

But i tried, the "post" method, return "CachePolicyVerify".

I've added debug log as following 
=============================================================
CachePolicy FrameLoader::subresourceCachePolicy() const
{
    if (m_isComplete)
	{
		printf("zywu01:CachePolicyVerify\r\n");
        return CachePolicyVerify;
	}

    if (m_loadType == FrameLoadTypeReloadFromOrigin)
	{
		printf("zywu02:CachePolicyReload\r\n");
        return CachePolicyReload;
	}

    if (Frame* parentFrame = m_frame->tree()->parent()) {
        CachePolicy parentCachePolicy = parentFrame->loader()->subresourceCachePolicy();
        if (parentCachePolicy != CachePolicyVerify)
		{
			if(parentCachePolicy == CachePolicyCache)
				printf("zywu03:CachePolicyCache\r\n");
			else if(parentCachePolicy == CachePolicyReload)
				printf("zywu03:CachePolicyReload\r\n");
			else if(parentCachePolicy == CachePolicyRevalidate)
				printf("zywu03:CachePolicyRevalidate\r\n");
			else if(parentCachePolicy ==CachePolicyAllowStale)
				printf("zywu03:CachePolicyAllowStale\r\n");
            return parentCachePolicy;
		}
		else
			printf("foo\r\n");
    }

    // FIXME: POST documents are always Reloads, but their subresources should still be Revalidate.
    // If we bring the CachePolicy.h and ResourceRequest cache policy enums in sync with each other and
    // remember "Revalidate" in ResourceRequests, we can remove this "POST" check and return either "Reload" 
    // or "Revalidate" if the DocumentLoader was requested with either.
    const ResourceRequest& request(documentLoader()->request());
	printf("the request method is : %s\r\n", request.httpMethod().latin1().data());
    if (request.cachePolicy() == ReloadIgnoringCacheData && !equalIgnoringCase(request.httpMethod(), "post"))
    //if (request.cachePolicy() == ReloadIgnoringCacheData && equalIgnoringCase(request.httpMethod(), "post"))
	{
		printf("zywu04:CachePolicyRevalidate, %s\r\n", request.httpMethod().latin1().data());
        return CachePolicyRevalidate;
	}

    if (m_loadType == FrameLoadTypeReload)
	{
		printf("zywu05:CachePolicyRevalidate\r\n");
        return CachePolicyRevalidate;
	}

    if (request.cachePolicy() == ReturnCacheDataElseLoad)
	{
		printf("zywu06:CachePolicyAllowStale\r\n");
        return CachePolicyAllowStale;
	}

	printf("zywu07:CachePolicyVerify\r\n");
    return CachePolicyVerify;
}

test webpage is 
============================================================
<html xmlns='http://www.w3.org/1999/xhtml'>
   <head >
      <meta http-equiv='Content-Type' content='text/html; charset=utf-8'/>
      <title >Form Page: sampleform</title>
   </head>
<body>
<h1>Sample form page</h1>

<style type = "text/css">

:focus {
	outline-style:auto;
	outline-color:red;
}
</style>
 

<form id='sampleform' method='post' action='https://www.google.com/accounts/ServiceLoginAuth?method=submit' >
   <p>
   Name: <input type='text' name='Name' value='aaa' />
   </p>
   <p>
   Email: <input type='text' name='Email' value='bbb'/>
   </p>
   <p>
    <input type='button' class="firstInLine" onclick='window.sampleform.submit()' value='submit' />
   </p>
</form>
 
</body>
</html>

Here is the output log:
=============================================================
a the request method is : POST
zywu07:CachePolicyVerify
CachePolicyVerify:https://www.google.com/accounts/mail.gif
a the request method is : POST
zywu07:CachePolicyVerify
CachePolicyVerify:https://www.google.com/accounts/msh.gif
a the request method is : POST
zywu07:CachePolicyVerify
CachePolicyVerify:https://www.google.com/accounts/ig.gif
a the request method is : POST
zywu07:CachePolicyVerify
CachePolicyVerify:https://www.google.com/accounts/sierra.gif
a the request method is : POST
zywu07:CachePolicyVerify
CachePolicyVerify:https://www.google.com/accounts/google_transparent.gif

[Alexey Proskuryakov]

> According to the comment "// FIXME: POST documents are always Reloads, but their subresources should still be Revalidate."
> 
> so if the request method is "post",then subresourceCachePolicy() should return
> "CachePolicyRevalidate".

This is a FIXME comment, meaning that it doesn't describe what the code does. It describes what it should do. This comment is not very clear.

Please consider the following:
1) As I mentioned in comment 1, this code is going to change very soon anyway.
2) We track bugs and enhancements in Bugzilla, this is not a place to discuss what existing code means if that's not in a context of a proposed patch.

[zywu]

Okay, i see, thanks.