Last Comment Bug 683891 - Stop exporting symbols that are THEBES_API from libxul
: Stop exporting symbols that are THEBES_API from libxul
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla12
Assigned To: Kyle Huey [:khuey] (khuey@mozilla.com)
:
Mentors:
Depends on: 718985
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-01 07:17 PDT by Kyle Huey [:khuey] (khuey@mozilla.com)
Modified: 2012-01-18 05:04 PST (History)
3 users (show)
khuey: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (1.71 KB, patch)
2011-09-01 07:17 PDT, Kyle Huey [:khuey] (khuey@mozilla.com)
mh+mozilla: review+
Details | Diff | Review

Description Kyle Huey [:khuey] (khuey@mozilla.com) 2011-09-01 07:17:02 PDT
Created attachment 557487 [details] [diff] [review]
Patch

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.
Comment 1 Mike Hommey [:glandium] 2011-09-01 08:45:29 PDT
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.
Comment 2 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-09-01 08:47:25 PDT
(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 3 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-09-26 08:31:27 PDT
Review ping?
Comment 4 Mike Hommey [:glandium] 2011-09-26 10:57:41 PDT
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.
Comment 5 Kyle Huey [:khuey] (khuey@mozilla.com) 2012-01-06 06:48:20 PST
https://hg.mozilla.org/mozilla-central/rev/7564c427cd98

Note You need to log in before you can comment on or make changes to this bug.