Closed Bug 1053563 Opened 5 years ago Closed 5 years ago

Replace ::InitWith methods in TextureClient implemenations by static construction methods.

Categories

(Core :: Graphics: Layers, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: nical, Assigned: ethlin, Mentored)

References

Details

Attachments

(1 file, 1 obsolete file)

Following up on the stuff that went in bug 1027601
Blocks: 990924
Mentor: nical.bugzilla
Assignee: nobody → etlin
I search all InitWidth functions and move them to the constructor. Is that right?
Attachment #8599006 - Flags: review?(nical.bugzilla)
(In reply to Ethan Lin[:elin] from comment #1)
> Created attachment 8599006 [details] [diff] [review]
> Move all InitWidth to constructor
> 
> I search all InitWidth functions and move them to the constructor. Is that
> right?

Not constructors, be static methods instead. The idea is that there are many reasons for a texture allocation to fail so we want to be able to do:

RefPtr<TextureClient> texture = TextureClientFooBar::Create(size, format, ...);
if (!texture) {
  // handle failure case
}

Instead of:

RefPtr<TextureClient> texture = new TextureClientFooBar(...);
if (!texture->Allocate(...)) {
  texture = nullptr;
}

The goal is to avoid making it possible to hold on to a texture that we failed to allocate (and it makes the API slightly nicer).

For example, see https://bugzilla.mozilla.org/attachment.cgi?id=8444125&action=diff
Comment on attachment 8599006 [details] [diff] [review]
Move all InitWidth to constructor

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

Constructors don't let you handle failure cases gracefully. Please use a static method which internally calls the constructor, and only return a non-null texture if the allocation was successful.
Attachment #8599006 - Flags: review?(nical.bugzilla) → review-
(In reply to Nicolas Silva [:nical] from comment #3)
> Comment on attachment 8599006 [details] [diff] [review]
> Move all InitWidth to constructor
> 
> Review of attachment 8599006 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Constructors don't let you handle failure cases gracefully. Please use a
> static method which internally calls the constructor, and only return a
> non-null texture if the allocation was successful.

I see. I will upload another patch to do this.
I create a static function for the TextureClient which has InitWith function. There is no caller of GrallocTextureClient's InitWith, so I remove it.
Attachment #8599006 - Attachment is obsolete: true
Attachment #8599656 - Flags: review?(nical.bugzilla)
Attachment #8599656 - Flags: review?(nical.bugzilla) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/0241a6b50e7b
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.