Summary: | ASSERTION FAILED: RunLoop::isMain() in com.apple.WebKit: IPC::Connection::sendSyncMessage + 128 | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||||
Component: | WebKit2 | Assignee: | Chris Dumez <cdumez> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | ap, beidson, commit-queue, ggaren, ryanhaddad, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Chris Dumez
2017-06-05 16:51:10 PDT
Created attachment 312038 [details]
Patch
Comment on attachment 312038 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=312038&action=review Very nice! > LayoutTests/storage/domstorage/sessionstorage/set-item-synchronous-keydown.html:7 > +description("Tests updatinh sessionStorage in the keydown handler and makes sure the value is updated synchronously."); Typo: updatinh. Created attachment 312041 [details]
Patch
Comment on attachment 312041 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=312041&action=review > LayoutTests/storage/domstorage/sessionstorage/set-item-synchronous-keydown.html:12 > + sessionStorage.testValue = 1; Oh, won't this persist to other tests? Should have a unique name like setItemSynchronousKeydownTestValue (removing it at the end would be useful too). Comment on attachment 312041 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=312041&action=review >> LayoutTests/storage/domstorage/sessionstorage/set-item-synchronous-keydown.html:12 >> + sessionStorage.testValue = 1; > > Oh, won't this persist to other tests? Should have a unique name like setItemSynchronousKeydownTestValue (removing it at the end would be useful too). Hmm, it looks like other sessionStorage tests include <script src="resources/clearSessionStorage.js"></script> so they clear everything before running the test. I guess I should do the same. Created attachment 312042 [details]
Patch
Comment on attachment 312042 [details] Patch Clearing flags on attachment: 312042 Committed r217810: <http://trac.webkit.org/changeset/217810> All reviewed patches have been landed. Closing bug. +sessionStorage.removeItem("setItemSynchronousKeydownTestValue"); This should at least be in its own <script></script> block, to defend against exceptions in the main test body. Currently, run-webkit-tests restarts the UI process for crashes and freezes anyway, but in the general case, a crash or freeze could make the value leak. (In reply to Alexey Proskuryakov from comment #10) > +sessionStorage.removeItem("setItemSynchronousKeydownTestValue"); > > This should at least be in its own <script></script> block, to defend > against exceptions in the main test body. Currently, run-webkit-tests > restarts the UI process for crashes and freezes anyway, but in the general > case, a crash or freeze could make the value leak. Ok. Although other tests do not have any kind of completion clean up it seems. Reopen for follow-up. Created attachment 312043 [details]
Follow-up fix
Comment on attachment 312043 [details] Follow-up fix Clearing flags on attachment: 312043 Committed r217813: <http://trac.webkit.org/changeset/217813> All reviewed patches have been landed. Closing bug. (In reply to WebKit Commit Bot from comment #8) > Comment on attachment 312042 [details] > Patch > > Clearing flags on attachment: 312042 > > Committed r217810: <http://trac.webkit.org/changeset/217810> I skipped the test for this change on iOS with http://trac.webkit.org/projects/webkit/changeset/217820 since it relies upon eventSender.keyDown() |