Bug 108380 - REGRESSION(r141136): Apple's internal PLT test suite doesn't finish
Summary: REGRESSION(r141136): Apple's internal PLT test suite doesn't finish
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on: 105667 108402 108435
Blocks:
  Show dependency treegraph
 
Reported: 2013-01-30 14:10 PST by Ryosuke Niwa
Modified: 2013-02-06 13:13 PST (History)
8 users (show)

See Also:


Attachments
Temporarily disable the feature (1.38 KB, patch)
2013-01-30 14:12 PST, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Re-enable the reuse of main resource cache (3.27 KB, patch)
2013-02-05 14:15 PST, Ryosuke Niwa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2013-01-30 14:10:54 PST
Apple's internal PLT test suite doesn't finish after r141136
Comment 1 Ryosuke Niwa 2013-01-30 14:12:09 PST
Created attachment 185564 [details]
Temporarily disable the feature
Comment 2 Alexey Proskuryakov 2013-01-30 14:13:35 PST
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.
Comment 3 Ryosuke Niwa 2013-01-30 14:18:54 PST
Committed r141306: <http://trac.webkit.org/changeset/141306>
Comment 4 Alexey Proskuryakov 2013-01-30 14:35:37 PST
Comment on attachment 185564 [details]
Temporarily disable the feature

Clearing cq+ from a landed patch.
Comment 5 Radar WebKit Bug Importer 2013-01-30 15:28:59 PST
<rdar://problem/13119725>
Comment 6 Ryosuke Niwa 2013-01-30 20:12:59 PST
The patch landed here fixed the bug so we need to investigate further and come up with a reduction.
Comment 7 Ryosuke Niwa 2013-01-30 22:02:08 PST
It appears that WebFrameLoaderClient::dispatchDidFinishLoading or  WebFrameLoaderClient::dispatchDidFailLoading is called with identifier of 0.
Comment 8 Jessie Berlin 2013-01-31 08:54:30 PST
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
Comment 9 Nate Chapin 2013-01-31 12:09:22 PST
(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.
Comment 10 Jessie Berlin 2013-01-31 12:57:58 PST
(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
Comment 11 Jessie Berlin 2013-01-31 17:07:09 PST
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.
Comment 12 Nate Chapin 2013-02-05 12:29:02 PST
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)?
Comment 13 Ryosuke Niwa 2013-02-05 12:31:04 PST
(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.
Comment 14 Nate Chapin 2013-02-05 12:33:07 PST
(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 :)
Comment 15 Ryosuke Niwa 2013-02-05 14:15:45 PST
Created attachment 186702 [details]
Re-enable the reuse of main resource cache
Comment 16 Ryosuke Niwa 2013-02-05 14:24:35 PST
I'm going to wait for EWS bots to catch up.
Comment 17 WebKit Review Bot 2013-02-06 13:13:11 PST
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>
Comment 18 WebKit Review Bot 2013-02-06 13:13:16 PST
All reviewed patches have been landed.  Closing bug.