| Summary: | Pointer lock causes abandons documents | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||||||
| Component: | DOM | Assignee: | Jeremy Jones <jeremyj-wk> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | ahmad.saleem792, ap, cdumez, commit-queue, ews-watchlist, jeremyj-wk, rniwa, simon.fraser, webkit-bug-importer | ||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||
| Version: | WebKit Nightly Build | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Simon Fraser (smfr)
2018-08-18 16:59:56 PDT
These tests are all leaky: pointer-lock/lock-lost-on-esc-in-fullscreen.html [ Leak ] pointer-lock/locked-element-removed-from-dom.html [ Leak Pass ] pointer-lock/mouse-event-delivery.html [ Leak Pass ] Created attachment 351123 [details]
Patch
Comment on attachment 351123 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=351123&action=review You can run tests with --world-leaks to test this. > Source/WebCore/ChangeLog:8 > + Reviewed by NOBODY (OOPS!). > + > + No new tests (OOPS!). More words please. > Source/WebCore/page/PointerLockController.h:72 > + WeakPtr<Document> m_documentOfRemovedElementWhileWaitingForUnlock; > + WeakPtr<Document> m_documentAllowedToRelockWithoutUserGesture; The names of these suggests that they should get cleared later, so what caused the leaks? Attachment 351123 [details] did not pass style-queue:
ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5]
Total errors found: 1 in 3 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 351125 [details]
Patch
Comment on attachment 351125 [details] Patch Attachment 351125 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/9388242 New failing tests: media/range-extract-contents-crash.html Created attachment 351151 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 351123 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=351123&action=review >> Source/WebCore/page/PointerLockController.h:72 >> + WeakPtr<Document> m_documentAllowedToRelockWithoutUserGesture; > > The names of these suggests that they should get cleared later, so what caused the leaks? The behavior of PointerLockController::documentDetached should be changed to clear m_documentAllowedToRelockWithoutUserGesture, since there is no point in relocating an element in a document that has been removed. Maybe we should keep around m_documentOfRemovedElementWhileWaitingForUnlock after documentDetached so we know how to handle responses coming from the page. If this is weak, it may still not fulfill that purpose. My guess is that it may not be necessary in that case, I need to confirm with a test. Did this land? Created attachment 357842 [details]
Patch for landing.
Comment on attachment 357842 [details] Patch for landing. Clearing flags on attachment: 357842 Committed r239469: <https://trac.webkit.org/changeset/239469> This landed and didn't backed out (while using BugID to search on Webkit GitHub): Commit - https://github.com/WebKit/WebKit/commit/afef071f0dfd605f6f97238cebe6fadfdfbfed57 Marking this as "RESOLVED FIXED". Thanks! |