Bug 85533 – Don't require FrameLoaderClient to manufacture a commitData() call for empty documents

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

[Nate Chapin]

In http://trac.webkit.org/changeset/115654, I added a call to DocumentLoader::commitData() in DocumentLoader::finishedLoading() if no data was yet received. This is necessary to ensure DocumentWriter actually creates the new Document.

This removes the need for each FrameLoaderClient implementation to manufacture the commitData() call instead.

[Nate Chapin]

Created attachment 140071 [details]
patch

[Gustavo Noronha (kov)]

Comment on attachment 140071 [details]
patch

Attachment 140071 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12600338

[Nate Chapin]

Created attachment 140084 [details]
fix gtk compile

[Alexey Proskuryakov]

Comment on attachment 140084 [details]
fix gtk compile

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

It's surprising that many clients had this m_hasRepresentation logic. Looks good to me, except for WK1 part that I couldn't understand.

> Source/WebKit/mac/ChangeLog:9
> +        * WebView/WebHTMLRepresentation.mm:
> +        (-[WebHTMLRepresentation finishedLoadingWithDataSource:]):

Could you add an explanation of what this is changing? It's not as closely connected to the goal of this patch as some other ports, so the change looks a bit confusing.

[Nate Chapin]

Created attachment 140250 [details]
Patch for landing

[Alexey Proskuryakov]

Well, the part I didn't understand was why there was a special case for web archives, and why it's no longer necessary.

[Nate Chapin]

(In reply to comment #6)
> Well, the part I didn't understand was why there was a special case for web archives, and why it's no longer necessary.

Ah....from what I can tell from implementing the commitData() call in DocumentLoader, web archives do not like having a document created before the archive gets fully swapped in. I'm guessing the special case is because sending that vacuous commitData call cause strange behavior if done for webarchives, so it just gets sent for everything else.

[Darin Adler]

The title of this bug is wrong. It describes the change made before this bug was even filed, where FrameLoader no longer requires the commitData calls we were doing.

This bug tracks removing the now-unneeded commitData calls, so the title should somehow describe that. Maybe "WebKit contains unneeded commitData calls for empty documents”.

The unclear title makes things a little confusing, but since everything is landed, we can just think that through better next time.

[WebKit Review Bot]

Comment on attachment 140250 [details]
Patch for landing

Clearing flags on attachment: 140250

Committed r116121: <http://trac.webkit.org/changeset/116121>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.