Bug 209925

Summary:

HTMLFormElement should use WeakPtr to keep track of its FormNamedItem

Product:

WebKit

Reporter:

Ryosuke Niwa <rniwa>

Component:

Forms

Assignee:

Ryosuke Niwa <rniwa>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, cdumez, ddkilzer, esprehn+autocc, ews-watchlist, ggaren, gyuyoung.kim, koivisto, mifenton, webkit-bug-importer, wenson_hsieh, zalan

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Deploys WeakPtr	wenson_hsieh: review+

Description Ryosuke Niwa 2020-04-02 13:39:36 PDT

Right now HTMLFormElement uses a HashMap of AtomString to a raw pointer of FormNamedItem.
We should replace the latter with WeakPtr to the equivalent HTMLElement.

Comment 1 Ryosuke Niwa 2020-04-02 13:58:39 PDT

Created attachment 395298 [details]
Deploys WeakPtr

Comment 2 Wenson Hsieh 2020-04-02 16:38:49 PDT

Comment on attachment 395298 [details]
Deploys WeakPtr

The windows EWS test failures seem unrelated.

Comment 3 Ryosuke Niwa 2020-04-03 13:53:44 PDT

(In reply to Wenson Hsieh from comment #2)
> Comment on attachment 395298 [details]
> Deploys WeakPtr
> 
> The windows EWS test failures seem unrelated.

Yup, indeed it's been cleared now.

Comment 4 Ryosuke Niwa 2020-04-03 14:05:21 PDT

Committed r259513: <https://trac.webkit.org/changeset/259513>

Comment 5 Radar WebKit Bug Importer 2020-04-03 14:06:14 PDT

<rdar://problem/61274486>

Comment 6 David Kilzer (:ddkilzer) 2020-04-03 22:06:58 PDT

(In reply to Ryosuke Niwa from comment #4)
> Committed r259513: <https://trac.webkit.org/changeset/259513>

Build fix:

Committed r259527: <https://trac.webkit.org/changeset/259527>

Comment 7 Ryosuke Niwa 2020-04-03 22:11:31 PDT

(In reply to David Kilzer (:ddkilzer) from comment #6)
> (In reply to Ryosuke Niwa from comment #4)
> > Committed r259513: <https://trac.webkit.org/changeset/259513>
> 
> Build fix:
> 
> Committed r259527: <https://trac.webkit.org/changeset/259527>

Oh oops, thanks for the build fix but that's wrong :( We shouldn't be enabling this assertions even when security assertions are enabled.

Comment 8 David Kilzer (:ddkilzer) 2020-04-04 21:16:32 PDT

(In reply to Ryosuke Niwa from comment #7)
> (In reply to David Kilzer (:ddkilzer) from comment #6)
> > (In reply to Ryosuke Niwa from comment #4)
> > > Committed r259513: <https://trac.webkit.org/changeset/259513>
> > 
> > Build fix:
> > 
> > Committed r259527: <https://trac.webkit.org/changeset/259527>
> 
> Oh oops, thanks for the build fix but that's wrong :( We shouldn't be
> enabling this assertions even when security assertions are enabled.

Committed r259543: <https://trac.webkit.org/changeset/259543>

Comment 9 Ryosuke Niwa 2020-04-06 12:32:28 PDT

(In reply to David Kilzer (:ddkilzer) from comment #8)
> (In reply to Ryosuke Niwa from comment #7)
> > (In reply to David Kilzer (:ddkilzer) from comment #6)
> > > (In reply to Ryosuke Niwa from comment #4)
> > > > Committed r259513: <https://trac.webkit.org/changeset/259513>
> > > 
> > > Build fix:
> > > 
> > > Committed r259527: <https://trac.webkit.org/changeset/259527>
> > 
> > Oh oops, thanks for the build fix but that's wrong :( We shouldn't be
> > enabling this assertions even when security assertions are enabled.
> 
> Committed r259543: <https://trac.webkit.org/changeset/259543>

Thanks!