Bug 235876 - Delete runtime flag for subresource integrity
Summary: Delete runtime flag for subresource integrity
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-01-30 20:22 PST by Ryosuke Niwa
Modified: 2022-01-31 13:55 PST (History)
12 users (show)

See Also:


Attachments
Patch (9.63 KB, patch)
2022-01-30 20:39 PST, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Patch (9.60 KB, patch)
2022-01-30 21:49 PST, Ryosuke Niwa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2022-01-30 20:22:30 PST
It's always enabled.
Comment 1 Ryosuke Niwa 2022-01-30 20:39:19 PST
Created attachment 450375 [details]
Patch
Comment 2 Ryosuke Niwa 2022-01-30 21:49:40 PST
Created attachment 450377 [details]
Patch
Comment 3 Darin Adler 2022-01-30 21:54:45 PST
Comment on attachment 450375 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=450375&action=review

> Source/WebCore/dom/ScriptElement.cpp:293
> +            m_element.attributeWithoutSynchronization(HTMLNames::integrityAttr).string(),

Might want to check and see if we should change this argument to const AtomString& since it’s an attribute value. Not sure why it uses String, but it might have to do with the conditional, although nullAtom would have let use do that before.

> Source/WebCore/dom/ScriptElement.cpp:350
> +            m_element.attributeWithoutSynchronization(HTMLNames::integrityAttr).string(),

Ditto.
Comment 4 Ryosuke Niwa 2022-01-30 22:15:08 PST
(In reply to Darin Adler from comment #3)
> Comment on attachment 450375 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=450375&action=review
> 
> > Source/WebCore/dom/ScriptElement.cpp:293
> > +            m_element.attributeWithoutSynchronization(HTMLNames::integrityAttr).string(),
> 
> Might want to check and see if we should change this argument to const
> AtomString& since it’s an attribute value. Not sure why it uses String, but
> it might have to do with the conditional, although nullAtom would have let
> use do that before.
> 
> > Source/WebCore/dom/ScriptElement.cpp:350
> > +            m_element.attributeWithoutSynchronization(HTMLNames::integrityAttr).string(),
> 
> Ditto.

That's a good point. Probably want to tackle that in a separate patch though.
Comment 5 Ryosuke Niwa 2022-01-31 13:37:18 PST
Comment on attachment 450377 [details]
Patch

Clearing flags on attachment: 450377

Committed r288840 (246604@trunk): <https://commits.webkit.org/246604@trunk>
Comment 6 Ryosuke Niwa 2022-01-31 13:37:22 PST
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2022-01-31 13:38:20 PST
<rdar://problem/88291720>
Comment 8 Ryosuke Niwa 2022-01-31 13:55:45 PST
(In reply to Ryosuke Niwa from comment #4)
> (In reply to Darin Adler from comment #3)
> > Comment on attachment 450375 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=450375&action=review
> > 
> > > Source/WebCore/dom/ScriptElement.cpp:293
> > > +            m_element.attributeWithoutSynchronization(HTMLNames::integrityAttr).string(),
> > 
> > Might want to check and see if we should change this argument to const
> > AtomString& since it’s an attribute value. Not sure why it uses String, but
> > it might have to do with the conditional, although nullAtom would have let
> > use do that before.
> > 
> > > Source/WebCore/dom/ScriptElement.cpp:350
> > > +            m_element.attributeWithoutSynchronization(HTMLNames::integrityAttr).string(),
> > 
> > Ditto.
> 
> That's a good point. Probably want to tackle that in a separate patch though.

Doing this followup in https://bugs.webkit.org/show_bug.cgi?id=235919.