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
Product: WebKit
Classification: Unclassified
Component: WebCore Misc.
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To: Nate Chapin
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-03 12:56 PDT by Nate Chapin
Modified: 2012-05-04 10:36 PDT (History)
6 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Nate Chapin 2012-05-03 12:56:03 PDT
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 Nate Chapin 2012-05-03 13:01:39 PDT
Created attachment 140071 [details]
patch
Comment 2 Gustavo Noronha (kov) 2012-05-03 13:27:17 PDT
Comment on attachment 140071 [details]
patch

Attachment 140071 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12600338
Comment 3 Nate Chapin 2012-05-03 13:52:07 PDT
Created attachment 140084 [details]
fix gtk compile
Comment 4 Alexey Proskuryakov 2012-05-03 14:55:25 PDT
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.
Comment 5 Nate Chapin 2012-05-04 09:20:51 PDT
Created attachment 140250 [details]
Patch for landing
Comment 6 Alexey Proskuryakov 2012-05-04 09:58:49 PDT
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 Nate Chapin 2012-05-04 10:06:23 PDT
(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 Darin Adler 2012-05-04 10:36:08 PDT
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 WebKit Review Bot 2012-05-04 10:36:26 PDT
Comment on attachment 140250 [details]
Patch for landing

Clearing flags on attachment: 140250

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