Bug 235919 - Use more AtomString and un-inline code for ScriptElementCachedScriptFetcher and its subclasses
Summary: Use more AtomString and un-inline code for ScriptElementCachedScriptFetcher a...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-01-31 13:55 PST by Ryosuke Niwa
Modified: 2022-02-01 14:13 PST (History)
8 users (show)

See Also:

Attachments
Patch (14.88 KB, patch)
2022-01-31 14:26 PST, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
Patch (14.99 KB, patch)
2022-01-31 14:29 PST, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2022-01-31 13:55:24 PST 
Cleanup suggested by Darin in https://bugs.webkit.org/show_bug.cgi?id=235876.
Comment 1 Ryosuke Niwa 2022-01-31 14:26:51 PST 
Created attachment 450463 [details]
Patch
Comment 2 Ryosuke Niwa 2022-01-31 14:29:05 PST 
Created attachment 450464 [details]
Patch
Comment 3 Darin Adler 2022-02-01 08:52:45 PST 
Comment on attachment 450464 [details]
Patch

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

> Source/WebCore/dom/LoadableClassicScript.cpp:44
> +LoadableClassicScript::LoadableClassicScript(const String& nonce, const AtomString& integrity, ReferrerPolicy policy, const AtomString& crossOriginMode, const String& charset, const AtomString& initiatorName, bool isInUserAgentShadowTree, bool isAsync)

Was nonce going to be AtomString?
Comment 4 Ryosuke Niwa 2022-02-01 11:10:34 PST 
(In reply to Darin Adler from comment #3)
> Comment on attachment 450464 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=450464&action=review
> 
> > Source/WebCore/dom/LoadableClassicScript.cpp:44
> > +LoadableClassicScript::LoadableClassicScript(const String& nonce, const AtomString& integrity, ReferrerPolicy policy, const AtomString& crossOriginMode, const String& charset, const AtomString& initiatorName, bool isInUserAgentShadowTree, bool isAsync)
> 
> Was nonce going to be AtomString?

Yup. In Element.h we see that:

...
    WEBCORE_EXPORT const AtomString& nonce() const;
...
Comment 5 Ryosuke Niwa 2022-02-01 11:12:56 PST 
Comment on attachment 450464 [details]
Patch

Clearing flags on attachment: 450464

Committed r288900 (?): <https://commits.webkit.org/r288900>
Comment 6 Ryosuke Niwa 2022-02-01 11:12:59 PST 
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2022-02-01 11:13:39 PST 
<rdar://problem/88338714>
Comment 8 Darin Adler 2022-02-01 12:24:58 PST 
Comment on attachment 450464 [details]
Patch

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

>>> Source/WebCore/dom/LoadableClassicScript.cpp:44
>>> +LoadableClassicScript::LoadableClassicScript(const String& nonce, const AtomString& integrity, ReferrerPolicy policy, const AtomString& crossOriginMode, const String& charset, const AtomString& initiatorName, bool isInUserAgentShadowTree, bool isAsync)
>> 
>> Was nonce going to be AtomString?
> 
> Yup. In Element.h we see that:
> 
> ...
>     WEBCORE_EXPORT const AtomString& nonce() const;
> ...

My point is that the code above says const String& nonce instead of const AtomString& nonce.
Comment 9 Ryosuke Niwa 2022-02-01 14:09:08 PST 
(In reply to Darin Adler from comment #8)
> Comment on attachment 450464 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=450464&action=review
> 
> >>> Source/WebCore/dom/LoadableClassicScript.cpp:44
> >>> +LoadableClassicScript::LoadableClassicScript(const String& nonce, const AtomString& integrity, ReferrerPolicy policy, const AtomString& crossOriginMode, const String& charset, const AtomString& initiatorName, bool isInUserAgentShadowTree, bool isAsync)
> >> 
> >> Was nonce going to be AtomString?
> > 
> > Yup. In Element.h we see that:
> > 
> > ...
> >     WEBCORE_EXPORT const AtomString& nonce() const;
> > ...
> 
> My point is that the code above says const String& nonce instead of const
> AtomString& nonce.

Oh! I missed that. Will fix.
Comment 10 Ryosuke Niwa 2022-02-01 14:13:09 PST 
Committed r288912 (246652@trunk): <https://commits.webkit.org/246652@trunk>