Apple's internal PLT test suite doesn't finish after r141136
Created attachment 185564 [details] Temporarily disable the feature
Comment on attachment 185564 [details] Temporarily disable the feature View in context: https://bugs.webkit.org/attachment.cgi?id=185564&action=review r=me We should make a reproducible case that's not internal. > Source/WebCore/loader/cache/CachedResourceLoader.cpp:482 > +#if PLATFORM(CHROMIUM) || PLATFORM(MAC) > // FIXME: Temporarily leave main resource caching disabled for chromium, see https://bugs.webkit.org/show_bug.cgi?id=107962 > // Ensure main resources aren't preloaded, and other main resource loads are removed from cache to prevent reuse. Please add a comment about Mac here.
Committed r141306: <http://trac.webkit.org/changeset/141306>
Comment on attachment 185564 [details] Temporarily disable the feature Clearing cq+ from a landed patch.
<rdar://problem/13119725>
The patch landed here fixed the bug so we need to investigate further and come up with a reduction.
It appears that WebFrameLoaderClient::dispatchDidFinishLoading or WebFrameLoaderClient::dispatchDidFailLoading is called with identifier of 0.
5 tests started failing on Mountain Lion WK1 Release tests in between 141305 and 141310: http://build.webkit.org/builders/Apple%20MountainLion%20Release%20WK1%20%28Tests%29/builds/6313 http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK1%20(Tests)/r141310%20(6314)/results.html http/tests/cache/cached-main-resource.html http/tests/inspector/resource-har-pages.html http/tests/loading/redirect-methods.html http/tests/misc/link-rel-icon-beforeload.html http/tests/misc/favicon-loads-with-images-disabled.html Especially for the first one, this change seems very suspect
(In reply to comment #8) > 5 tests started failing on Mountain Lion WK1 Release tests in between 141305 and 141310: > http://build.webkit.org/builders/Apple%20MountainLion%20Release%20WK1%20%28Tests%29/builds/6313 > http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK1%20(Tests)/r141310%20(6314)/results.html > > http/tests/cache/cached-main-resource.html > http/tests/inspector/resource-har-pages.html > http/tests/loading/redirect-methods.html > http/tests/misc/link-rel-icon-beforeload.html > http/tests/misc/favicon-loads-with-images-disabled.html > > Especially for the first one, this change seems very suspect All of those (except resource-har-pages.html) are expected to change, given that their results depend on the feature disabled in r141306. It's probably best to just expect them to fail until main resource caching is re-enabled.
(In reply to comment #9) > (In reply to comment #8) > > 5 tests started failing on Mountain Lion WK1 Release tests in between 141305 and 141310: > > http://build.webkit.org/builders/Apple%20MountainLion%20Release%20WK1%20%28Tests%29/builds/6313 > > http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK1%20(Tests)/r141310%20(6314)/results.html > > > > http/tests/cache/cached-main-resource.html > > http/tests/inspector/resource-har-pages.html > > http/tests/loading/redirect-methods.html > > http/tests/misc/link-rel-icon-beforeload.html > > http/tests/misc/favicon-loads-with-images-disabled.html > > > > Especially for the first one, this change seems very suspect > > All of those (except resource-har-pages.html) are expected to change, given that their results depend on the feature disabled in r141306. It's probably best to just expect them to fail until main resource caching is re-enabled. Marked them as expected failures in platform/mac’s TestExpectations: http://trac.webkit.org/changeset/141469
Actually, http/tests/inspector/resource-har-pages.html was changed in the checkin that enabled this feature: http://trac.webkit.org/changeset/141136 I am going to mark it as failing for now as well.
Has anyone had a chance to test whether this suite still fails since http://trac.webkit.org/changeset/141615 landed (with main resource caching re-enabled)?
(In reply to comment #12) > Has anyone had a chance to test whether this suite still fails since http://trac.webkit.org/changeset/141615 landed (with main resource caching re-enabled)? Yes, r141615 fixed the PLT when main resource caching is enabled. My plan is to post a new patch that reverts r141306. Sorry I got distracted by other work.
(In reply to comment #13) > (In reply to comment #12) > > Has anyone had a chance to test whether this suite still fails since http://trac.webkit.org/changeset/141615 landed (with main resource caching re-enabled)? > > Yes, r141615 fixed the PLT when main resource caching is enabled. My plan is to post a new patch that reverts r141306. Sorry I got distracted by other work. No problem, I just wanted to make sure there weren't other regressions blocking re-enable :)
Created attachment 186702 [details] Re-enable the reuse of main resource cache
I'm going to wait for EWS bots to catch up.
Comment on attachment 186702 [details] Re-enable the reuse of main resource cache Clearing flags on attachment: 186702 Committed r142024: <http://trac.webkit.org/changeset/142024>
All reviewed patches have been landed. Closing bug.