Bug 62852 – cannot handle status codes which have no response body

• Component: Page Loading
• Status: CLOSED WORKSFORME

[Keunsoon Lee]

Hi,

This bug report is derived from https://bugs.webkit.org/show_bug.cgi?id=60875.

I found that FrameLoader tries to download if server or client error code without response body comes on EFL and GTK port.
But it seems all other ports have same problem.

So, I created a new thread to solve this problem for all ports and FrameLoader.

For solution, I simply called didFail() for such cases on soup port.
But, Robert Hogan and James Simonsen said didFail() is only for Network error and not for such status code error cases.

If so, we might need to create a new flow on Loader, because there is no way to handle this on Webkit.
And, each port might be hard to handle this within themselves.

What do you thing so?

[Keunsoon Lee]

Hi,

I still don't know what is the best way to handle this.
So, I researched several browsers' acting for this.

As you can see, only Opera and Chromium can show page information to user.
And, others show just blank page.

I prefer showing information than blank page.
Then, we need to create the information page for each status code somewhere, though.

How about you?

--------------------------------------------------------------------------------------------------------------------------------------------------------------------
Testing site : http://spe.mobilephone.net/wit/commonv1/httperror.xhtml

** Linux PC
   Opera
      making a error page for each status code

   Chromium
      making a error page for each status code

   Firefox
      showing blank page

** Windows PC
   Safari
      showing blank page

   IE 
      cannot test because it is not able to handle xhtml

** Mobile
   Android Browser
      showing blank page

   iPhone 4 Safari
      showing blank page

[Alexey Proskuryakov]

Clearly, different vendors can have different opinion on this matter.

Could you implement this in your browser without changing WebKit?

[Keunsoon Lee]

(In reply to comment #2)
> Clearly, different vendors can have different opinion on this matter.
> 
> Could you implement this in your browser without changing WebKit?

It is pretty simple, if network layer (e.g. soup, curl etc.) creates the info page and provides it as if receiving from Server.
But I don't think it is a good idea, because each port cannot choose their behavior for that case.
Furthermore, each port might want to insert their logo on the information page.

To support the specialized page, network layer can request to create the page for each port.
But, I still don't like it, because it makes a dependency between network layer and each port.

So, I'm looking for the way to give a choice to each port.
That is, notifying this situation to each port and showing a blank page if they don't do anything would be better.

It is not simple, because ResourceLoader always expects MIME-Type on ResourceResponse, though.


Any comment and idea will be welcome.

[Keunsoon Lee]

Created attachment 102090 [details]
Add new FrameLoaderClient function to notify a case finished without response body.

Sorry for this late update.

Rough scenario for added flow is like follow;

1. DocumentLoader::finishedLoading() found m_gotFirstByte is false, which is the target situation.
2. Notifying the situation to FrameLoaderClient through dispatchDidFinishLoadingWithoutBody(), newly added function.
3. If FrameLoaderClient wants to show informational page, it should create substitute data and send it to FrameLoader.
	3-1. If FrameLoaderClient does nothing for the function, FrameLoader commits blank page.
4. FrameLoadTypeReload will be set for the substitute data for Back/Forward.
5. The original DocumentLoader will be stopped while loading the substitute data.


There is a premise for this flow. 
PolicyUse should be selected for this.
That means, MIME Type from server or http stack (in case of content sniffing) should be exist and a type for PolicyUse on FrameLoader.
I will consider to solve this after finishing this flow.

Finally, I'm not confident of the new function's name, dispatchDidFinishLoadingWithoutBody.
It will notify to FrameLoaderClient that loading is finished but it has no response body.
I feel this has similar meaning with dispatchDidFinishLoading(), so I chose the name.
But, I'm not sure what different is dispatchDidFinishLoading from dispatchDidFinishLoad.
I'd appreciate it if anyone can give me advice for this.

Any advice and comment will be welcome.

Thank you.

[Adam Barth]

Comment on attachment 102090 [details]
Add new FrameLoaderClient function to notify a case finished without response body.

View in context: https://bugs.webkit.org/attachment.cgi?id=102090&action=review

> Source/WebCore/loader/FrameLoader.cpp:3299
> +    m_delegateIsFinishingWithoutBody = true;
> +    m_client->dispatchDidFinishLoadingWithoutBody(dl);
> +    m_delegateIsFinishingWithoutBody = false;

Frown.  We use this pattern a bunch in FrameLoader, but it's really fragile.  If we're being re-entered by the client, who's to say this function can't be re-entered?  The net result of patches like this are that we have skads of these bools hanging off FrameLoader and the class is a bear to work with.

[Adam Barth]

Comment on attachment 102090 [details]
Add new FrameLoaderClient function to notify a case finished without response body.

View in context: https://bugs.webkit.org/attachment.cgi?id=102090&action=review

R- for lack of tests.  I'm not sure excited about the approach either.

> Source/WebCore/ChangeLog:8
> +        No new tests. (OOPS!)

No tests?

[Keunsoon Lee]

Oh, thank you for your review!

I'm going to modify it according your comment with test.

Thank you.

[Alexey Proskuryakov]

Why is this new client call necessary? Can't the client just check main resource data from didFinish?

[Keunsoon Lee]

(In reply to comment #8)
> Why is this new client call necessary? Can't the client just check main resource data from didFinish?

If you mean FrameLoader::didFinishLoad(), it is called after committing.
At that time, there is no sign for the situation unless we create a new flag for that.

I also considered creating a new flag on FrameLoader or DocumentLoader.
(The name could be isFinishedWithoutBody and so on.)
But, I felt adding new FrameLoaderClient function is more natural for loader's structure.

Furthermore, I think it is more convenient for each port to support specific informational page.

That's why I added this new function.

Thank you.

[Alexey Proskuryakov]

What kind of information are you looking for that is not currently available?

For instance, a FrameLoaderClient gets a pointer to document loader in dispatchDidLoadMainResource(). It also gets it in committedLoad(), together with the data that triggered the commit. Also, from virtually any callback it can access its frame's active document loader, and any information associated with it.

The problem I see with implementing the behavior you described is one of choice - there are so many ways to do it that it's hard to pick one that works best in edge cases. Adding one more similar callback will only make the problem worse.

[Keunsoon Lee]

(In reply to comment #10)
> What kind of information are you looking for that is not currently available?
> 
> For instance, a FrameLoaderClient gets a pointer to document loader in dispatchDidLoadMainResource(). It also gets it in committedLoad(), together with the data that triggered the commit. Also, from virtually any callback it can access its frame's active document loader, and any information associated with it.
> 
> The problem I see with implementing the behavior you described is one of choice - there are so many ways to do it that it's hard to pick one that works best in edge cases. Adding one more similar callback will only make the problem worse.

I thought giving an exact notice for the situation via separate function would be the best way.
But, now I agree with you that adding a similar function can make client confused rather than help.

Furthermore, Adam Barth said (on chat) that different behavior for specific status code on FrameLoader would not be a good choice.

Now, I'm studying Chromium port to solve this situation because I heard it can solve this problem without any touch on FrameLoader.


Thank you for your comment!