Closed Bug 1139818 Opened 5 years ago Closed 5 years ago

Merge imgRequest public and private sections

Categories

(Core :: ImageLib, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: seth, Assigned: seth)

References

Details

Attachments

(1 file, 2 obsolete files)

imgRequest currently has public and private members mixed together, like this:

> class imgRequest {
> private:
>  ...
> public:
>  ...
> private:
>  ...
> public:
>  ...
> private:
>  ...
> };

That makes the code harder to read and maintain.

In this bug I'll merge things together so there is one explicit private section and one public section. Typedefs will go at the beginning of the class definition in the initial, implicitly-private section, just as we generally do in layout code.
Here's the patch.
Attachment #8573159 - Flags: review?(amarchesini)
Comment on attachment 8573159 [details] [diff] [review]
Merge imgRequest public and private sections

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

::: image/src/imgRequest.h
@@ +206,5 @@
>  
>    bool HasConsumers();
>  
>  private:
> +  friend class FinishPreparingForNewPartRunnable;

I would move this to line 51.

@@ +208,5 @@
>  
>  private:
> +  friend class FinishPreparingForNewPartRunnable;
> +
> +  virtual ~imgRequest();

no needs for virtual, this is MOZ_FINAL. right?
Attachment #8573159 - Flags: review?(amarchesini) → review+
Thanks for the review!

(In reply to Andrea Marchesini (:baku) from comment #2)
> no needs for virtual, this is MOZ_FINAL. right?

I see what you mean, but our style guide requires this, at least by my reading.
Updated patch and rebased against the new versions of the patches in bug 1139804.
Attachment #8573159 - Attachment is obsolete: true
Attachment #8575732 - Attachment is obsolete: true
Pushed to inbound:

29219668983e	Seth Fowler — Bug 1139818 - Merge imgRequest public and private sections. r=baku
https://hg.mozilla.org/mozilla-central/rev/29219668983e
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Depends on: 1150127
No longer depends on: 1150127
You need to log in before you can comment on or make changes to this bug.