Bug 131318

Created attachment 228772 [details] the patch

Comment on attachment 228772 [details] the patch wrong patch, i hate you webkit-patch

(In reply to comment #2) > (From update of attachment 228772 [details]) > wrong patch, i hate you webkit-patch Reported: https://bugs.webkit.org/show_bug.cgi?id=131327

Created attachment 228774 [details] the patch

Created attachment 228775 [details] OK, this is actually it

Comment on attachment 228775 [details] OK, this is actually it View in context: https://bugs.webkit.org/attachment.cgi?id=228775&action=review > Source/WebCore/ChangeLog:15 > + Test: http/tests/inspector/replay/document-last-modified-fallback-value.html This test will not actually work until 130728 and 129391 are fixed. Until then, the resource could have cached values of Last-Modified and be flaky. I didn't mark anything skip because LayoutTests/inspector/ is completely skipped due to 129642.

BTW: this API is being addressed now, because it is used by the google analytics urchin. So, basically every page hits it once, then makes a network request with a URL based on it.

Comment on attachment 228775 [details] OK, this is actually it View in context: https://bugs.webkit.org/attachment.cgi?id=228775&action=review r=me > LayoutTests/http/tests/inspector/replay/document-last-modified-fallback-value.html:4 > +<script type="text/javascript" src="./replay-test.js"></script> Is the "./" really needed here?

Comment on attachment 228775 [details] OK, this is actually it Clearing flags on attachment: 228775 Committed r167261: <http://trac.webkit.org/changeset/167261>

All reviewed patches have been landed. Closing bug.

This test was landed without an expected result. But more importantly, it appears that this change broke a lot of tests: http/tests/loading/unfinished-main-resource-back-to-cached-page-callbacks.html http/tests/loading/unfinished-load-back-to-cached-page-callbacks.html http/tests/misc/isindex-with-no-form.html http/tests/navigation/javascriptlink-frames.html http/tests/navigation/document-location-click.html http/tests/navigation/javascriptlink-goback.html http/tests/navigation/javascriptlink-basic.html http/tests/navigation/document-location-click-timeout.html http/tests/navigation/document-location-mouseover.html Please see <http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK2%20(Tests)/r167261%20(17688)/results.html> for results. Will roll out. Unfortunately, bots were not being looked at today, and thus didn't work well, so I have to go by data from a single tester. Apologies in advance if this was not the culprit.

Re-opened since this is blocked by bug 131667

Comment on attachment 228775 [details] OK, this is actually it View in context: https://bugs.webkit.org/attachment.cgi?id=228775&action=review >> Source/WebCore/ChangeLog:15 >> + Test: http/tests/inspector/replay/document-last-modified-fallback-value.html > > This test will not actually work until 130728 and 129391 are fixed. Until then, the resource could have cached values of Last-Modified and be flaky. I didn't mark anything skip because LayoutTests/inspector/ is completely skipped due to 129642. Looks like this is not the case, because bots were running this test.

(In reply to comment #11) > This test was landed without an expected result. But more importantly, it appears that this change broke a lot of tests: > > http/tests/loading/unfinished-main-resource-back-to-cached-page-callbacks.html > http/tests/loading/unfinished-load-back-to-cached-page-callbacks.html > http/tests/misc/isindex-with-no-form.html > http/tests/navigation/javascriptlink-frames.html > http/tests/navigation/document-location-click.html > http/tests/navigation/javascriptlink-goback.html > http/tests/navigation/javascriptlink-basic.html > http/tests/navigation/document-location-click-timeout.html > http/tests/navigation/document-location-mouseover.html > > Please see <http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK2%20(Tests)/r167261%20(17688)/results.html> for results. > > Will roll out. Unfortunately, bots were not being looked at today, and thus didn't work well, so I have to go by data from a single tester. Apologies in advance if this was not the culprit. Ugh, my IRC client apparently stopped working after dinner or I would have looked into it. I'll look at this tomorrow on a ML debug build.

After some fiddling and test runs, my hypothesis at the moment is that the new test was running even though it shouldn't have. If it encountered an error and failed to clean up/restore non-replay settings (say, leaving page cache turned off), this may mess up later tests on the same process. (It's disturbing to me that I cannot confidently say that the test harness resets obvious settings like that.) When I explicitly skip the test in TestExpectations, I can no longer reproduce the problem on my WK2-ML-debug build. So, can this be re-landed with updated test expectations? I don't think it's a productive use of time to try and enable this test right now, since it clearly depends on other replay support that hasn't landed yet.

> So, can this be re-landed with updated test expectations? Seems worth trying.

Created attachment 229419 [details] patch with updated TestExpectations

Comment on attachment 229419 [details] patch with updated TestExpectations Clearing flags on attachment: 229419 Committed r167406: <http://trac.webkit.org/changeset/167406>

All reviewed patches have been landed. Closing bug.