314432 - Get rid of TVector glue in Default Plugin on Mac/Intel

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Mark Mentovai]

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]

Attachment: Patch

This is Mac-only code, so there's no problem getting rid of XP_MACOSX.

Comment 2

[Josh Aas]

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]

Fix checked in.  Leaving open for subsequent TARGET_CARBON cleanup.

Comment 4

[Mark Mentovai]

Attachment: Miscellaneous dead-code cleanup

Comment 5

[Josh Aas]

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)]

Comment on attachment 203446 [details] [diff] [review]
Miscellaneous dead-code cleanup

sr=pink

Comment 7

[Mark Mentovai]

Attachment: Miscellaneous dead-code cleanup, as checked in

Checked in on the trunk.

Comment 8

[Mark Mentovai]

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]

Comment on attachment 202676 [details] [diff] [review]
Patch

Add fixed1.8.1 and fixed1.8.0.1 when checked in.

Comment 10

[Mark Mentovai]

The first patch on this bug was checked in to the 1_8 and 1_8_0 branches.

Comment 11

[Mark Mentovai]

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]

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]]

Does a Firefox build include any of this code? It'll be easier to approve if the answer is "NPOTB"

Comment 14

[Mark Mentovai]

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)]

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]

Attachment 207005 [details] [diff] checked in on MOZILLA_1_8_BRANCH for 1.8.1.12.