Closed Bug 1407937 Opened 4 years ago Closed 4 years ago

Move mDefinition from CustomElementReacion to CustomElementUpgradeReaction

Categories

(Core :: DOM: Core & HTML, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: edgar, Assigned: edgar)

Details

Attachments

(1 file, 2 obsolete files)

mDefinition [1] is only used in CustomElementUpgradeReaction, so we could consider move it from CustomElementReacion to CustomElementUpgradeReaction.

[1] http://searchfox.org/mozilla-central/rev/31606bbabc50b08895d843b9f5f3da938ccdfbbf/dom/base/CustomElementRegistry.h#229
Attached patch Patch, v2 (obsolete) — Splinter Review
Attachment #8917742 - Attachment is obsolete: true
Attachment #8917839 - Flags: feedback?(jdai)
Comment on attachment 8917839 [details] [diff] [review]
Patch, v2

Review of attachment 8917839 [details] [diff] [review]:
-----------------------------------------------------------------

Overall it looks good. Thank you.

::: dom/base/CustomElementRegistry.h
@@ +213,5 @@
>  
>  class CustomElementReaction
>  {
>  public:
> +  CustomElementReaction()

Nit: Please remove this line. We don't need to write a default CTOR in here.

@@ +228,5 @@
>  class CustomElementUpgradeReaction final : public CustomElementReaction
>  {
>  public:
>    explicit CustomElementUpgradeReaction(CustomElementDefinition* aDefinition)
> +    : CustomElementReaction()

Nit: Please remove this line.

@@ +243,5 @@
>  class CustomElementCallbackReaction final : public CustomElementReaction
>  {
>    public:
> +    explicit CustomElementCallbackReaction(UniquePtr<CustomElementCallback> aCustomElementCallback)
> +      : CustomElementReaction()

Nit: Please remove this line.
Attachment #8917839 - Flags: feedback?(jdai) → feedback+
Address comment #3
Attachment #8917839 - Attachment is obsolete: true
Attachment #8918169 - Flags: review?(bugs)
Attachment #8918169 - Flags: review?(bugs) → review+
Pushed by echen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7f3731e3864d
Move mDefinition from CustomElementReacion to CustomElementUpgradeReaction; f=jdai; r=smaug
https://hg.mozilla.org/mozilla-central/rev/7f3731e3864d
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Assignee: nobody → echen
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.