Bug 85533 - Don't require FrameLoaderClient to manufacture a commitData() call for empty documents
: Don't require FrameLoaderClient to manufacture a commitData() call for empty ...
Status: RESOLVED FIXED
: WebKit
WebCore Misc.
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2012-05-03 12:56 PST by
Modified: 2012-05-04 10:36 PST (History)


Attachments
patch (26.43 KB, patch)
2012-05-03 13:01 PST, Nate Chapin
gns: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
fix gtk compile (26.44 KB, patch)
2012-05-03 13:52 PST, Nate Chapin
no flags Review Patch | Details | Formatted Diff | Diff
Patch for landing (27.73 KB, patch)
2012-05-04 09:20 PST, Nate Chapin
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-05-03 12:56:03 PST
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.
------- Comment #1 From 2012-05-03 13:01:39 PST -------
Created an attachment (id=140071) [details]
patch
------- Comment #2 From 2012-05-03 13:27:17 PST -------
(From update of attachment 140071 [details])
Attachment 140071 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12600338
------- Comment #3 From 2012-05-03 13:52:07 PST -------
Created an attachment (id=140084) [details]
fix gtk compile
------- Comment #4 From 2012-05-03 14:55:25 PST -------
(From update of attachment 140084 [details])
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.
------- Comment #5 From 2012-05-04 09:20:51 PST -------
Created an attachment (id=140250) [details]
Patch for landing
------- Comment #6 From 2012-05-04 09:58:49 PST -------
Well, the part I didn't understand was why there was a special case for web archives, and why it's no longer necessary.
------- Comment #7 From 2012-05-04 10:06:23 PST -------
(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.
------- Comment #8 From 2012-05-04 10:36:08 PST -------
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.
------- Comment #9 From 2012-05-04 10:36:26 PST -------
(From update of attachment 140250 [details])
Clearing flags on attachment: 140250

Committed r116121: <http://trac.webkit.org/changeset/116121>
------- Comment #10 From 2012-05-04 10:36:31 PST -------
All reviewed patches have been landed.  Closing bug.