Bug 42777

Summary: WebKit2 needs layoutTestController.setDeferMainResourceDataLoad
Product: WebKit Reporter: Sam Weinig <sam>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, jbedard, mikhail.pozdnyakov, ryanhaddad
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Sam Weinig 2010-07-21 12:51:47 PDT
WebKit2 needs layoutTestController.setDeferMainResourceDataLoad
Comment 1 Sam Weinig 2010-07-21 12:53:49 PDT
<rdar://problem/8218168>
Comment 2 Jonathan Bedard 2016-08-17 16:13:18 PDT
Created attachment 286339 [details]
Patch
Comment 3 Jonathan Bedard 2016-08-17 16:14:28 PDT
Note that only one test actually called setDeferMainResourceDataLoad, and even in DumpRenderTree, this test would pass if the function was not called.
Comment 4 Darin Adler 2016-08-20 19:12:32 PDT
Comment on attachment 286339 [details]
Patch

This does not seem right. We should fix the test so it tests what it intended to, rather than removing it. I see no rationale here for removing the test other than “it never worked”, but that is not sufficient.
Comment 5 Darin Adler 2016-08-20 19:21:38 PDT
One path here is to come up for a better rationale for why it's OK to remove this test.

Unfortunately, the fact that the test passes when setDeferMainResourceDataLoad does nothing does *not* mean the test had no value. It's entirely possible that it can detect bugs that other tests cannot. It's also possible, though, that it is true that the test no longer has value.
Comment 6 Jonathan Bedard 2016-08-22 08:48:10 PDT
Another possibility is to keep the tests but remove calls to setDeferMainResourceDataLoad, but keep the test.  Looking at the test, it seems that deferring load is the default behavior.
Comment 7 Jonathan Bedard 2016-08-22 17:01:12 PDT
Created attachment 286639 [details]
Patch
Comment 8 Darin Adler 2016-08-24 10:14:42 PDT
Comment on attachment 286639 [details]
Patch

I looked back at when we originally added the test:

https://bugs.webkit.org/show_bug.cgi?id=30879

I read it carefully, and I see that this was a test specific to a peculiar way that WebKit was used by the now-long-obsolete Qt port. Given that, I changed my mind and I think that we *can* remove the test.
Comment 9 Jonathan Bedard 2016-08-24 10:42:22 PDT
Created attachment 286864 [details]
Patch
Comment 10 WebKit Commit Bot 2016-08-24 12:29:09 PDT
Comment on attachment 286864 [details]
Patch

Clearing flags on attachment: 286864

Committed r204918: <http://trac.webkit.org/changeset/204918>
Comment 11 WebKit Commit Bot 2016-08-24 12:29:13 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Ryan Haddad 2016-08-24 14:36:38 PDT
Attempted Windows build fix in http://trac.webkit.org/projects/webkit/changeset/204933