Last Comment Bug 314432 - Get rid of TVector glue in Default Plugin on Mac/Intel
: Get rid of TVector glue in Default Plugin on Mac/Intel
Status: RESOLVED FIXED
: fixed1.8.0.1, fixed1.8.1, fixed1.8.1.12
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: 1.8 Branch
: PowerPC Mac OS X
: -- normal (vote)
: mozilla1.8.1
Assigned To: Mark Mentovai
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-10-30 12:13 PST by Mark Mentovai
Modified: 2007-12-18 13:32 PST (History)
6 users (show)
benjamin: blocking1.8.1+
benjamin: blocking1.8.0.1+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (1.25 KB, patch)
2005-11-11 08:41 PST, Mark Mentovai
jaas: review+
benjamin: approval1.8.0.1+
benjamin: approval1.8.1+
Details | Diff | Splinter Review
Miscellaneous dead-code cleanup (16.12 KB, patch)
2005-11-17 14:03 PST, Mark Mentovai
jaas: review+
mikepinkerton: superreview+
Details | Diff | Splinter Review
Miscellaneous dead-code cleanup, as checked in (17.45 KB, patch)
2005-12-28 10:24 PST, Mark Mentovai
mark: review+
mark: superreview+
samuel.sidler+old: approval1.8.1.12+
Details | Diff | Splinter Review

Description Mark Mentovai 2005-10-30 12:13:47 PST
The Default Plugin has ugly TVector glue for the CFM interface.  This is appropriate on ppc, but it's now wrong on x86.  On x86, this needs to change to use raw native Mach-O the whole way.

http://lxr.mozilla.org/mozilla/source/modules/plugin/samples/default/mac/npmac.cpp#92

Note: Default Plugin is now disabled on Firefox.  (Why is it still packaged and shipped?)
Comment 1 Mark Mentovai 2005-11-11 08:41:16 PST
Created attachment 202676 [details] [diff] [review]
Patch

This is Mac-only code, so there's no problem getting rid of XP_MACOSX.
Comment 2 Josh Aas 2005-11-14 10:11:57 PST
On checkin, please expand up the following comment to explain the intel/ppc function pointer situation:

// glue for mapping outgoing Macho function pointers to TVectors

Lots of people use that plugin as a reference.
Comment 3 Mark Mentovai 2005-11-17 13:38:42 PST
Fix checked in.  Leaving open for subsequent TARGET_CARBON cleanup.
Comment 4 Mark Mentovai 2005-11-17 14:03:23 PST
Created attachment 203446 [details] [diff] [review]
Miscellaneous dead-code cleanup
Comment 5 Josh Aas 2005-11-17 17:44:03 PST
Comment on attachment 203446 [details] [diff] [review]
Miscellaneous dead-code cleanup

Yeah, I think we can probably get rid of the 68k mac code now. I didn't compile this myself, I trust you on this one. Looks good.
Comment 6 Mike Pinkerton (not reading bugmail) 2005-12-02 13:57:54 PST
Comment on attachment 203446 [details] [diff] [review]
Miscellaneous dead-code cleanup

sr=pink
Comment 7 Mark Mentovai 2005-12-28 10:24:43 PST
Created attachment 207005 [details] [diff] [review]
Miscellaneous dead-code cleanup, as checked in

Checked in on the trunk.
Comment 8 Mark Mentovai 2005-12-28 10:25:44 PST
Comment on attachment 202676 [details] [diff] [review]
Patch

Need this patch for DefaultPlugin to be compatible with x86 Macs.  DefaultPlugin is disabled in the standard Firefox configuration, but is shipped and can be enabled.  DefaultPlugin IS used by other products.

Only requesting approval for the first patch here, the clean-up patch isn't necessary.
Comment 9 Benjamin Smedberg [:bsmedberg] 2006-01-04 12:05:27 PST
Comment on attachment 202676 [details] [diff] [review]
Patch

Add fixed1.8.1 and fixed1.8.0.1 when checked in.
Comment 10 Mark Mentovai 2006-01-04 12:50:28 PST
The first patch on this bug was checked in to the 1_8 and 1_8_0 branches.
Comment 11 Mark Mentovai 2007-10-30 19:11:48 PDT
Comment on attachment 207005 [details] [diff] [review]
Miscellaneous dead-code cleanup, as checked in

This patch was r=josh sr=pink, and was checked into the trunk on 2005-12-28.  Since it's just cleanup, it was never checked in on the branch, although the code is dead on the branch too.  I'd like to take this patch on the branch now because the #include changes fix a problem that occurs when building the 1.8 branch using the native SDK on Mac OS X 10.5 ("Leopard").  I'd like to take this change on that branch because active development still occurs on that branch for Camino: Camino 1.6, just now approaching alpha, will be based on MOZILLA_1_8_BRANCH.
Comment 12 Mark Mentovai 2007-11-08 11:14:01 PST
Comment on attachment 207005 [details] [diff] [review]
Miscellaneous dead-code cleanup, as checked in

(carrying forward reviews from previous version - this is the version that was checked in)
Comment 13 Daniel Veditz [:dveditz] 2007-12-17 15:20:10 PST
Does a Firefox build include any of this code? It'll be easier to approve if the answer is "NPOTB"
Comment 14 Mark Mentovai 2007-12-17 21:09:34 PST
Comment on attachment 207005 [details] [diff] [review]
Miscellaneous dead-code cleanup, as checked in

Everything that's removed is #if 0 on Firefox.  The compiled (preprocessed, actually) version of this source is the same before and after this patch.
Comment 15 Samuel Sidler (old account; do not CC) 2007-12-18 10:55:12 PST
Comment on attachment 207005 [details] [diff] [review]
Miscellaneous dead-code cleanup, as checked in

Thanks for the comment Mark.

Approved for 1.8.1.12; a=ss for release-drivers.
Comment 16 Mark Mentovai 2007-12-18 13:32:55 PST
Attachment 207005 [details] [diff] checked in on MOZILLA_1_8_BRANCH for 1.8.1.12.

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