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

Sam Weinig
Reported 2010-07-21 12:51:47 PDT
WebKit2 needs layoutTestController.setDeferMainResourceDataLoad
Attachments
Patch (9.75 KB, patch)
2016-08-17 16:13 PDT, Jonathan Bedard 		no flags
DetailsFormatted DiffDiff
Patch (9.21 KB, patch)
2016-08-22 17:01 PDT, Jonathan Bedard 		no flags
DetailsFormatted DiffDiff
Patch (9.76 KB, patch)
2016-08-24 10:42 PDT, Jonathan Bedard 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Sam Weinig
Comment 1 2010-07-21 12:53:49 PDT
<rdar://problem/8218168>
Jonathan Bedard
Comment 2 2016-08-17 16:13:18 PDT
Created attachment 286339 [details] Patch
Jonathan Bedard
Comment 3 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.
Darin Adler
Comment 4 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.
Darin Adler
Comment 5 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.
Jonathan Bedard
Comment 6 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.
Jonathan Bedard
Comment 7 2016-08-22 17:01:12 PDT
Created attachment 286639 [details] Patch
Darin Adler
Comment 8 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.
Jonathan Bedard
Comment 9 2016-08-24 10:42:22 PDT
Created attachment 286864 [details] Patch
WebKit Commit Bot
Comment 10 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>
WebKit Commit Bot
Comment 11 2016-08-24 12:29:13 PDT
All reviewed patches have been landed. Closing bug.
Ryan Haddad
Comment 12 2016-08-24 14:36:38 PDT
Attempted Windows build fix in http://trac.webkit.org/projects/webkit/changeset/204933
Note You need to log in before you can comment on or make changes to this bug.