RESOLVED FIXED 65265
[chromium] Make WebImage::operator=(CGImageRef) a WEBKIT_API 
https://bugs.webkit.org/show_bug.cgi?id=65265
Summary [chromium] Make WebImage::operator=(CGImageRef) a WEBKIT_API
Nico Weber
Reported 2011-07-27 09:45:01 PDT
[chromium] Make WebImage::operator=(CGImageRef) a WEBKIT_API
Attachments
Patch (1.98 KB, patch)
2011-07-27 09:46 PDT, Nico Weber 		no flags
DetailsFormatted DiffDiff
Patch (1.25 KB, patch)
2011-07-28 19:31 PDT, Nico Weber 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Nico Weber
Comment 1 2011-07-27 09:46:37 PDT
Created attachment 102152 [details] Patch
Nico Weber
Comment 2 2011-07-27 15:09:36 PDT
Darin: jamesr says this is for you
Darin Fisher (:fishd, Google)
Comment 3 2011-07-27 15:16:36 PDT
Comment on attachment 102152 [details] Patch This change is technically fine, but it would be nice to know why you want to make this change. Can you please explain the motivation for making this non-inline? Normally, the WebKit API implements operators inline as calls to exported functions.
Nico Weber
Comment 4 2011-07-27 15:20:18 PDT
The larger motivation is the components build for mac. As mentioned in the changelog, webkit/glue/webcursor_mac.mm ( http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/webkit/glue/webcursor_mac.mm&exact_package=chromium&q=webcursor_ma&type=cs ) calls operator=, so it needs to be exported. assign(CGImageRef) is currently private. If it's preferrable, I can make assign(CGImageRef) public, turn that into WEBKIT_API, keep operator=() inline, and update the call site (after waiting for the roll). Your call :-)
Darin Fisher (:fishd, Google)
Comment 5 2011-07-27 15:23:25 PDT
(In reply to comment #4) > The larger motivation is the components build for mac. > > As mentioned in the changelog, webkit/glue/webcursor_mac.mm ( http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/webkit/glue/webcursor_mac.mm&exact_package=chromium&q=webcursor_ma&type=cs ) calls operator=, so it needs to be exported. assign(CGImageRef) is currently private. If it's preferrable, I can make assign(CGImageRef) public, turn that into WEBKIT_API, keep operator=() inline, and update the call site (after waiting for the roll). Oh, I see. Yeah, the convention would be to tag assign(CGImageRef) with WEBKIT_API. We shouldn't need to make it public.
Darin Fisher (:fishd, Google)
Comment 6 2011-07-27 22:43:58 PDT
Comment on attachment 102152 [details] Patch As discussed, please tag the assign(CGImageRef) function with WEBKIT_API, and leave the rest as is.
Nico Weber
Comment 7 2011-07-28 19:31:09 PDT
Created attachment 102325 [details] Patch
James Robinson
Comment 8 2011-07-28 19:32:39 PDT
Comment on attachment 102325 [details] Patch R+ based on Darin's comment.
WebKit Review Bot
Comment 9 2011-07-28 20:25:53 PDT
Comment on attachment 102325 [details] Patch Clearing flags on attachment: 102325 Committed r91970: <http://trac.webkit.org/changeset/91970>
WebKit Review Bot
Comment 10 2011-07-28 20:25:57 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.