Two recent revision made two inavdertent changes around DOMApplicationCache. Those changes should be undone. r46609 made it a programming error to construct a DOMApplicationCache instance with a null frame ptr. r47018 made a change to return 'null' for window.applicationCache when the feature is disabled in the preferences.
Created attachment 34928 [details] simple cleanup
Comment on attachment 34928 [details] simple cleanup The DOMWindow change needs a test. It's possible to test preference changes in DumpRenderTree these days.
(In reply to comment #2) > It's possible to test preference changes in DumpRenderTree these days. What does that mean?
You can use things like: layoutTestController.overridePreference("WebKitDefaultFontSize", "24"); to override preferences for the duration of a single test. Thus you can test that toggling this preference no longer affects the visibility of window.applicaitonCache. Since we can test it, a test is required. :)
Created attachment 39035 [details] cleanup with layout test
Comment on attachment 39035 [details] cleanup with layout test r=me For the record, the rationale behind this behavior is: - it's slightly more consistent for a frameless window to be able to construct a DOMApplicaitonCache, because window.applicationCache would be non-null if constructed before detaching; - it's unlikely that any browser will have ApplicationCache compiled in, but disabled; run-time disabling is needed for clients like Dashboard. So, knowing that it's non-null can simplify code a little, and will do no harm. Neither is particularly important, just minor cleanup. + layoutTestController.overridePreference("WebKitOfflineWebApplicationCacheEnabled", false); Does this really work on all platforms? You may need to add this new test to Skipped list for some (or maybe the platforms that don't have it already skip all tests http/tests/in appcache?)
> layoutTestController.overridePreference("WebKitOfflineWebApplicationCacheEnabled", > false); > > Does this really work on all platforms? You may need to add this new test to > Skipped list for some (or maybe the platforms that don't have it already skip > all tests http/tests/in appcache?) I'm not able to run the tests on all other platforms. layoutTestController seems to be very platform specific. For example, this particular preference name was not being respected in chrome's test_shell for example. I have a seperate CL to add support for this key in chrome land. The others impls I looked at did seem to support this pref name in their overridePreference method. Worth pointing out that the test does not confirm if the pref is actually disabled by the overridePreference call, nor does passage depend on the pref actually being disabled. The test will PASS even if a feature is still enabled.
Exception: Unknown committer: michaeln@google.com Sigh. I need to fix the commit queue to not wedge itself when a non-committer sets commit-queue+. :(
Comment on attachment 39035 [details] cleanup with layout test Rejecting patch 39035 from commit-queue. This patch will require manual commit. ['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1'] failed with exit code 1
Comment on attachment 39035 [details] cleanup with layout test media/video-played-ranges-1.html -> failed Looks like yet another flakey media test. :(
Comment on attachment 39035 [details] cleanup with layout test Clearing flags on attachment: 39035 Committed r48108: <http://trac.webkit.org/changeset/48108>
All reviewed patches have been landed. Closing bug.
(In reply to comment #12) > All reviewed patches have been landed. Closing bug. Thank you Eric!