Closed Bug 683891 Opened 10 years ago Closed 9 years ago

Stop exporting symbols that are THEBES_API from libxul

Categories

(Core :: Graphics, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla12

People

(Reporter: khuey, Assigned: khuey)

References

Details

Attachments

(1 file)

Attached patch PatchSplinter Review
The attached patch reduces the number of symbols we export from libxul from over 2000 (!) to about 300.

Code in browser/components expects to be able to Release a gfxASurface though, so I had to make virtual versions of the refcounting methods for external consumers.
Attachment #557487 - Flags: review?(mh+mozilla)
Comment on attachment 557487 [details] [diff] [review]
Patch

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

::: gfx/thebes/gfxASurface.h
@@ +72,5 @@
> +    }
> +#else
> +    virtual nsresult AddRef(void);
> +    virtual nsresult Release(void);
> +#endif

The only place I can find that uses a gfxASurface is browser/components/shell/src/nsWindowsShellService.cpp, and it looks like we could entirely get rid of the use of the nsRefPtr there.
(In reply to Mike Hommey [:glandium] from comment #1)
> The only place I can find that uses a gfxASurface is
> browser/components/shell/src/nsWindowsShellService.cpp, and it looks like we
> could entirely get rid of the use of the nsRefPtr there.

Right.  How would getting rid of the ref ptr help?  We need to call Release on it somehow ...
Comment on attachment 557487 [details] [diff] [review]
Patch

I really don't like that we need to add a vtable for one use of that API in browsercomps. I think the background saving stuff should be rewritten instead (like, do we really need to have it write a bmp file in 2011?), but it's probably not worth holding up this change longer. I however think you should make that gfxASurface workaround windows-only. I'd even be tempted to say to put that workaround on gfxImageSurface only.
Attachment #557487 - Flags: review?(mh+mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/7564c427cd98
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
Depends on: 718985
You need to log in before you can comment on or make changes to this bug.