Last Comment Bug 409066 - sNativeRegionPool visibility is wrong in libgkgfx
: sNativeRegionPool visibility is wrong in libgkgfx
: verified1.8.1.12
Product: Core Graveyard
Classification: Graveyard
Component: GFX: Mac (show other bugs)
: 1.8 Branch
: All Mac OS X
: -- normal (vote)
: ---
Assigned To: Mark Mentovai
Depends on:
Blocks: 408959
  Show dependency treegraph
Reported: 2007-12-19 12:14 PST by Mark Mentovai
Modified: 2009-01-22 10:17 PST (History)
4 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---

Decorate with visibility at declaration (830 bytes, patch)
2007-12-19 12:25 PST, Mark Mentovai
ted: review+
dveditz: approval1.8.1.12+
mbeltzner: approval1.9+
Details | Diff | Splinter Review

Description Mark Mentovai 2007-12-19 12:14:19 PST
We've got a global variable, sNativeRegionPool, that's part of libgkgfx on the Mac only.  The variable is instantiated as NS_EXPORT which sets the visibility properly, but when building with compiler visibility support activated, a shared library build fails when linking libgfx_mac, which depends on libgkgfx and the sNativeRegionPool variable, because the symbol winds up not exported.  The build log reveals:

(while compiling mozilla/gfx/src/mac/nsRegionPool.cpp)
.../mozilla/gfx/src/mac/nsRegionPool.cpp:41: warning: ‘sNativeRegionPool’: visibility attribute ignored because it
.../mozilla/gfx/src/mac/nsRegionPool.h:66: warning: conflicts with previ ous declaration here

The export attribute needs to be applied to the declaration (nsRegionPool.h:66) in order for it to be effective.
Comment 1 Mark Mentovai 2007-12-19 12:25:14 PST
Created attachment 293909 [details] [diff] [review]
Decorate with visibility at declaration

This adds NS_EXPORT to the declaration, which allows sNativeRegionPool to be exported from libgkgfx, so that libgfx_mac and any other users can find it.

This code is not built on the trunk, which has moved on to Cairo gfx.

This change is needed on the branch to support the hidden-visibility Camino build for Camino 1.6.  It's completely safe on the branch, as NS_EXPORT always sets default visibility on all current 1.8-branch builds on the Mac.  This change is a no-op as far as all existing 1.8-branch builds are concerned: we do not use hidden visibility at all on the 1.8 branch on the Mac.
Comment 2 Mark Mentovai 2007-12-19 13:58:14 PST
Comment on attachment 293909 [details] [diff] [review]
Decorate with visibility at declaration

Requesting approval1.9 for the trunk.  Even though this is no longer built on the trunk since the switch to cairo gfx, I'll keep it synchronized until the old gfx source is removed from cvs.

Requesting approval1.8.1.12 for the MOZILLA_1_8_BRANCH.  This change has no impact on any existing 1.8 branch build, because symbols are always exported on the Mac on all existing 1.8 branch builds.
Comment 3 Mike Beltzner [:beltzner, not reading bugmail] 2007-12-20 14:42:14 PST
Comment on attachment 293909 [details] [diff] [review]
Decorate with visibility at declaration

a=beltzner for 1.9
Comment 4 Mark Mentovai 2007-12-20 14:52:56 PST
Checked in on the trunk.
Comment 5 Daniel Veditz [:dveditz] 2007-12-21 11:53:26 PST
Comment on attachment 293909 [details] [diff] [review]
Decorate with visibility at declaration

approved for, a=dveditz for release-drivers
Comment 6 Mark Mentovai 2007-12-21 12:49:45 PST
Checked in on MOZILLA_1_8_BRANCH.
Comment 7 Al Billings [:abillings] 2008-01-30 16:34:53 PST

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