Bug 208218 - Make sure a client cannot cause a whole DOM tree to get leaked by simply holding on to a WKBundleNodeHandle
Summary: Make sure a client cannot cause a whole DOM tree to get leaked by simply hold...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-02-25 14:33 PST by Chris Dumez
Modified: 2020-02-26 12:57 PST (History)
7 users (show)

See Also:


Attachments
Patch (15.90 KB, patch)
2020-02-25 14:36 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (16.84 KB, patch)
2020-02-26 11:58 PST, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2020-02-25 14:33:00 PST
Make sure a client cannot cause a whole DOM tree to get leaked by simply holding on to a WKBundleNodeHandle. WKBundleNodeHandle currently refs its node, which keeps the whole HTML document alive.
Comment 1 Chris Dumez 2020-02-25 14:36:50 PST
Created attachment 391688 [details]
Patch
Comment 2 WebKit Commit Bot 2020-02-25 16:20:26 PST
Comment on attachment 391688 [details]
Patch

Clearing flags on attachment: 391688

Committed r257389: <https://trac.webkit.org/changeset/257389>
Comment 3 WebKit Commit Bot 2020-02-25 16:20:28 PST
All reviewed patches have been landed.  Closing bug.
Comment 4 Radar WebKit Bug Importer 2020-02-25 16:21:17 PST
<rdar://problem/59785892>
Comment 6 Chris Dumez 2020-02-26 07:53:25 PST
(In reply to Jacob Uphoff from comment #5)
> Looks like the patch broke api tests in wk1&wk2
> https://build.webkit.org/builders/Apple-Catalina-Debug-WK2-Tests/builds/2518/
> steps/run-api-tests/logs/stdio and also caused 50+ debug tests to crash on
> wk2
> https://build.webkit.org/results/Apple-Catalina-Debug-WK2-Tests/
> r257467%20(2536)/results.html

Oh, easy fix. Will take care of it now.
Comment 7 Jacob Uphoff 2020-02-26 07:55:25 PST
Didn't see your comment till I had already rolled out out..
Comment 8 Chris Dumez 2020-02-26 08:04:28 PST
(In reply to Chris Dumez from comment #6)
> (In reply to Jacob Uphoff from comment #5)
> > Looks like the patch broke api tests in wk1&wk2
> > https://build.webkit.org/builders/Apple-Catalina-Debug-WK2-Tests/builds/2518/
> > steps/run-api-tests/logs/stdio and also caused 50+ debug tests to crash on
> > wk2
> > https://build.webkit.org/results/Apple-Catalina-Debug-WK2-Tests/
> > r257467%20(2536)/results.html
> 
> Oh, easy fix. Will take care of it now.

Fix landed in https://trac.webkit.org/changeset/257471/webkit.
Comment 9 Chris Dumez 2020-02-26 10:58:36 PST
Reverted r257389 for reason:

257471

Committed r257482: <https://trac.webkit.org/changeset/257482>
Comment 10 Chris Dumez 2020-02-26 11:58:43 PST
Created attachment 391764 [details]
Patch
Comment 11 WebKit Commit Bot 2020-02-26 12:57:18 PST
The commit-queue encountered the following flaky tests while processing attachment 391764 [details]:

editing/spelling/spellcheck-async-remove-frame.html bug 158401 (authors: morrita@google.com, rniwa@webkit.org, and tony@chromium.org)
The commit-queue is continuing to process your patch.
Comment 12 WebKit Commit Bot 2020-02-26 12:57:54 PST
Comment on attachment 391764 [details]
Patch

Clearing flags on attachment: 391764

Committed r257503: <https://trac.webkit.org/changeset/257503>
Comment 13 WebKit Commit Bot 2020-02-26 12:57:56 PST
All reviewed patches have been landed.  Closing bug.