Closed Bug 1368250 Opened 3 years ago Closed 3 years ago

Consider adding a destructor to EditorInitializer that calls Revoke()

Categories

(Core :: Layout: Form Controls, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: jfkthame, Assigned: jfkthame)

Details

Attachments

(1 file)

From bug 1365982 comment 14:
----------------------------
>layout/forms/nsTextControlFrame.cpp
>-  EditorInitializer* initializer = Properties().Get(TextControlInitializer());
>+  EditorInitializer* initializer = GetProperty(TextControlInitializer());
>   if (initializer) {
>     initializer->Revoke();
>-    Properties().Delete(TextControlInitializer());
>+    DeleteProperty(TextControlInitializer());

Stuff like this can also be improved if we had something like
FrameProperties::MaybeDelete that takes a callback that's invoked
if the property exists, like so:
MaybeDeleteProperty([] (EditorInitializer*& aValue) {
  aValue->Revoke();
  return true;  // means "delete it"
});

In this particular case though, we could add a ~EditorInitializer()
that Revokes and just use DeleteProperty(TextControlInitializer()) above.

>-    EditorInitializer* initializer = Properties().Get(TextControlInitializer());
>+    EditorInitializer* initializer = GetProperty(TextControlInitializer());
>     if (initializer) {
>       initializer->Revoke();
>     }
>     initializer = new EditorInitializer(this);
>-    Properties().Set(TextControlInitializer(),initializer);
>+    SetProperty(TextControlInitializer(),initializer);

This could also avoid the first Get if the dtor Revokes.
----------------------------
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Comment on attachment 8872086 [details] [diff] [review]
Give the TextControlInitializer property a dtor to automatically call Revoke when the property is deleted

Note that there is a path:
EditorInitializer::Run()
  mFrame->FinishedInitializer()
    Properties().Delete(TextControlInitializer())

I'm assuming that calling Revoke() from within Run() is harmless though.
Attachment #8872086 - Flags: review?(mats) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e61d7b8278567b029369b2dca13a26421c945cb
Bug 1368250 - Give the TextControlInitializer property a dtor to automatically call Revoke when the property is deleted. r=mats
https://hg.mozilla.org/mozilla-central/rev/2e61d7b82785
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.