Regression test for bug fixed in http://codereview.chromium.org/171039. Regexps created in one frame were not callable outside that frame.
Created attachment 34966 [details] Initial version
Comment on attachment 34966 [details] Initial version AFAIK, CPH is not a committer, adding cq+
Comment on attachment 34966 [details] Initial version Clearing flags on attachment: 34966 Committed r47399: <http://trac.webkit.org/changeset/47399>
All reviewed patches have been landed. Closing bug.
Sorry, I accidentally made this patch relative to LayoutTests rather than WebKit so it has landed one level too far out, creating trunk/fast/... It will have to be rolled back.
Rolled out in <https://trac.webkit.org/changeset/47411>.
Created attachment 35015 [details] Relative to WebKit
Comment on attachment 35015 [details] Relative to WebKit WebKit indent is 4 spaces. If you want this reviewed, you'll need to mark it r=? Also, official webkit style has { on a new line for function declarations. Also, LayoutTests/fast/regex/regexp-cross-frame-callable.html should just be a standard JS test, which uses make-script-test-wrappers to generate the wrapper .html file from the TEMPLATE.html You just write fast/regexp/resources/regexp-cross-frame-callable.js and let make-script-test-wrappers do its thing for you. If this was marked r=? I woudl mark it r-. Since it's not, I'm assuming this is just a work-in progress. :)
> You just write fast/regexp/resources/regexp-cross-frame-callable.js and let > make-script-test-wrappers do its thing for you. I need at least two html pages to exercise the cross-frame aspect. How would I do that?
You can easily do: var iframe = document.createElement('iframe'); iframe.src = "my_amazing_test_file_with_lazer_eyes.html"; document.body.appendChild(iframe); Your current test uses body.onload, which I think won't even wait for the iframe load. I'm not certain. You mgiht have to use iframe.onload = doTest() anyway.
Created attachment 35021 [details] Let's try that again
Comment on attachment 35021 [details] Let's try that again So this looks fine. It could be one step better though. Notice how the expected results are out of order: +PASS successfullyParsed is true + +TEST COMPLETE +PASS re('a') is ['a'] This is because the script in the subframe is executing after the main frame finishes parsing. Sadly, one ilmitation of the script-test testing framework is that we don't have a nice clean way to handle tests which need to delay completion. I expect you could get around this pretty easily this way: var iframe = document.createElement('iframe'); document.body.appendChild(iframe); iframe.document.body.innerHTML = "<script>top.doTest(/a/)</script>"; I would expect the script to execute synchronously from the innerHTML. If not, certainly this should: iframe.document.write("<script>top.doTest(/a/)</script>"); I'm OK committing this as-is, but I think we could improve it one step further if you're willing. Either of the above should remove the need for a second file altogether.
Created attachment 35023 [details] Results ordering
> I would expect the script to execute synchronously from the innerHTML. > > If not, certainly this should: > iframe.document.write("<script>top.doTest(/a/)</script>"); That's also a lot better since it removes the need for a separate file for the child frame. I've fixed it.
Comment on attachment 35023 [details] Results ordering Glorious!
Comment on attachment 35023 [details] Results ordering Rejecting patch 35023 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
Testing 11088 test cases. fast/regex/cross-frame-callable.html -> failed Exiting early after 1 failures. 7420 tests run. 131.57s total testing time I did not look at the diffs.
Where does this failure happen? As far as I can tell it passes on safari and chrome with webkit latest and v8 latest.
It happened when the commit-queue applied your patch, built (release) and ran the tests. I didn't look at the diffs, but if you're not seeing this locally I can dig them up.
(In reply to comment #19) > It happened when the commit-queue applied your patch, built (release) and ran > the tests. I didn't look at the diffs, but if you're not seeing this locally I > can dig them up. Is there a way for me to run the tests in the same setup as the commit-queue? I ran the test in safari on mac and there was no problem so if the diff is available somewhere that would be really helpful.
The commit queue just runs "run-webkit-tests" which uses DumpRenderTree to run your test.
Created attachment 35119 [details] Added trailing newline The problem is that there has to be an empty line at the end of the expectation file. In my workspace the line is there but between creating and applying the patch it disappears. This patch includes an extra newline but I'm not sure it works -- when I tried applying the patch the newline still disappeared.
Created attachment 38363 [details] Fixed trailing newline issue Removing the iframe doesn't make any difference, there's still a trailing newline. Only workaround I could come up with was to write DONE after the test was over, that seems to work. If it turns out not to work for some reason I'm ready to just drop this CL altogether.
Comment on attachment 38363 [details] Fixed trailing newline issue I'm not sure I know what's going on here, why "done" would be needed. But I also think this is fine.
Comment on attachment 38363 [details] Fixed trailing newline issue Clearing flags on attachment: 38363 Committed r47647: <http://trac.webkit.org/changeset/47647>