598401 - remove support for Quickdraw NPAPI

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Josh Aas]

https://mail.mozilla.org/pipermail/plugin-futures/2010-June/000120.html

The Quickdraw drawing model is considered deprecated and we are planning to remove support for it entirely after Firefox 4. We recommend that plugin vendors use the Core Graphics or Core Animation drawing models. We are working with plugin vendors to make sure they are ready for this change.

Also, see bug 598397 about removing the Carbon event model.

Comment 1

[Josh Aas]

Attachment: fix v1.0

Comment 2

[Josh Aas]

Attachment: fix v1.1

Comment 3

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 527565 [details] [diff] [review]
fix v1.1

This looks fine to me.  But I'm puzzled you didn't remove all
references to NP_NO_QUICKDRAW, even from the files you changed.

With the exception of npapi.h -- I *don't* think we should remove
NP_NO_QUICKDRAW from that file, because it's used by other vendors.

Here are the other places (in current trunk code) where you didn't
remove NP_NO_QUICKDRAW, by file name.  I list first the files that you
changed in this patch.

layout/generic/nsObjectFrame.cpp:
http://hg.mozilla.org/mozilla-central/annotate/863db843fde4/layout/generic/nsObjectFrame.cpp#l3223
http://hg.mozilla.org/mozilla-central/annotate/863db843fde4/layout/generic/nsObjectFrame.cpp#l4284

modules/plugin/base/src/nsNPAPIPlugin.cpp:
http://hg.mozilla.org/mozilla-central/annotate/863db843fde4/modules/plugin/base/src/nsNPAPIPlugin.cpp#l2197
(Here you did change the return value to PR_FALSE.)

widget/src/cocoa/nsChildView.mm:
http://hg.mozilla.org/mozilla-central/annotate/863db843fde4/widget/src/cocoa/nsChildView.mm#l2308

dom/plugins/ipc/PluginInstanceChild.cpp:
http://hg.mozilla.org/mozilla-central/annotate/863db843fde4/dom/plugins/ipc/PluginInstanceChild.cpp#l430

modules/plugin/base/src/nsNPAPIPluginInstance.cpp:
http://hg.mozilla.org/mozilla-central/annotate/863db843fde4/modules/plugin/base/src/nsNPAPIPluginInstance.cpp#l72

widget/src/xpwidgets/nsBaseWidget.cpp:
http://hg.mozilla.org/mozilla-central/annotate/863db843fde4/widget/src/xpwidgets/nsBaseWidget.cpp#l790

Comment 4

[Josh Aas]

Almost all of those cases (like the ones in nsObjectFrame) are i386 default Quickdraw values which are part of the API - they don't change when we don't support Quickdraw. We need to detect whether or not that has been negotiated by the plugin. Same goes for where we tell the plugin we don't support Quickdraw - in order to use the Quickdraw constants we need NP_NO_QUICKDRAW.

We probably should change the nsBaseWidget to a !i386 macro. I'll do that in a new patch. Thanks for the review.

Comment 5

[Josh Aas]

Attachment: fix v1.2

Comment 6

[Josh Aas]

Attachment: fix v1.3

Not tested extensively yet.

Comment 7

[Josh Aas]

I'm planning to do this for Firefox 17, when we will no longer support Mac OS X 10.5.

Comment 8

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 641099 [details] [diff] [review]
fix v1.3

While basically OK, this has some problems:

This patch drops support for printing plugins on OS X.  Will restoring
it be the subject of another bug?  We presumably shouldn't land this
patch until we've figured out how to print plugins without the few
QuickDraw calls we've been using.

+  if (!mView) {
     // [NSGraphicsContext currentContext] is supposed to "return the
     // current graphics context of the current thread."  But sometimes
     // (when called while mView isn't focused for drawing) it returns a
     // graphics context for the wrong window.  [window graphicsContext]
     // (which "provides the graphics context associated with the window
     // for the current thread") seems always to return the "right"
     // graphics context.  See bug 500130.
     mPluginCGContext.context = NULL;
     mPluginCGContext.window = NULL;
 #ifndef NP_NO_CARBON
+    NSWindow* cocoaWindow = [mView window];
+    WindowRef carbonWindow = cocoaWindow ? (WindowRef)[cocoaWindow windowRef] : NULL;
     if (carbonWindow) {
       mPluginCGContext.context = (CGContextRef)[[cocoaWindow graphicsContext] graphicsPort];
       mPluginCGContext.window = carbonWindow;
     }
 #endif
   }

The "if (!mView) {}" that surrounds this block should be dropped.

-// In QuickDraw mode the coordinate system used here should be that of the
-// browser window's content region (defined as everything but the 22-pixel
-// high titlebar).  But in CoreGraphics mode the coordinate system should be
-// that of the browser window as a whole (including its titlebar).  Both
-// coordinate systems have a top-left origin.  See bmo bug 474491.
+// In CoreGraphics mode the coordinate system should be that of the browser
+// window as a whole (including its titlebar).  This coordinate system has a
+// top-left origin.  See bmo bug 474491.

Without the description of the QuickDraw coordinate system, this whole
comment makes a lot less sense.

-  // In QuickDraw drawing mode, prevent reentrant handling of any plugin event
-  // (this emulates behavior on the 1.8 branch, where only QuickDraw mode is
-  // supported).  But in CoreGraphics drawing mode only do this if the current
+  // In CoreGraphics drawing mode only do this if the current

Same thing again.

But since we're no longer using QuickDraw, maybe this should be
rephrased as follows:

"In QuickDraw drawing mode we used to prevent reentrant handling of
any plugin event.  But in CoreGraphics drawing mode only do this if
the current ..."

-  // 10.6.2 and lower have a bug involving textures and pixel buffer objects
-  // that caused bug 629016, so we don't allow OpenGL-accelerated layers on
-  // those versions of the OS.
-  // This will still let full-screen video be accelerated on OpenGL, because
-  // that XUL widget opts in to acceleration, but that's probably OK.
-  SInt32 major, minor, bugfix;
-  OSErr err1 = ::Gestalt(gestaltSystemVersionMajor, &major);
-  OSErr err2 = ::Gestalt(gestaltSystemVersionMinor, &minor);
-  OSErr err3 = ::Gestalt(gestaltSystemVersionBugFix, &bugfix);
-  if (err1 == noErr && err2 == noErr && err3 == noErr) {
-    if (major == 10 && minor == 6) {
-      if (bugfix <= 2) {
-        accelerateByDefault = false;
-      }
-    }
-  }

This only makes sense if we're dropping support for OS X 10.5, which
should be the subject of another bug.

Comment 9

[Josh Aas]

Attachment: fix v1.4

Restored/fixed some comments, restored the printing code. We are planning to drop support for Mac OS X 10.5 in Firefox 17, this patch won't land until that is done.

Comment 10

[Josh Aas]

https://tbpl.mozilla.org/?tree=Try&rev=bd8caeb567c6

Comment 11

[Josh Aas]

Attachment: fix v1.5

Fixes a compile error and removes some dead code.

Comment 12

[Josh Aas]

https://tbpl.mozilla.org/?tree=Try&rev=dc37d03ced00

Comment 13

[Josh Aas]

Attachment: fix v1.6

We're all set to land a patch for this now. This update syncs to latest trunk.

Comment 14

[Josh Aas]

fix v1.6 try run:

https://tbpl.mozilla.org/?tree=Try&rev=d12b2a02432e

Comment 15

[Josh Aas]

Attachment: fix v1.7

Compile fix, and remove more dead code.

Comment 16

[Josh Aas]

https://tbpl.mozilla.org/?tree=Try&rev=7441e793aa77

Comment 17

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 656174 [details] [diff] [review]
fix v1.7

Looks fine to me.

Comment 18

[Josh Aas]

pushed to mozilla-inbound

http://hg.mozilla.org/integration/mozilla-inbound/rev/c38da7c88d55

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c38da7c88d55