Closed Bug 1218688 Opened 4 years ago Closed 4 years ago

Remove unused synchronous plugin drawing paths

Categories

(Core :: Plug-ins, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: dvander, Assigned: dvander)

References

Details

Attachments

(4 files, 2 obsolete files)

We have a synchronous drawing path for windowless plugins, possibly added during OOP development. It looks this involves creating a shared surface in SetWindow, then sending fake paint messages to the plugin during display list painting and waiting until each paint completes. On Windows, it paints twice for alpha extraction.

After talking to Jim and Bill, the synchronous code looks dead with OOP plugins, where we use async rendering instead. This involves a different SetWindow path - AsyncSetWindow - and a display item that creates an ImageLayer instead of painting on the main thread.

In theory we could ditch *all* of the synchronous rendering paths, since bug 1217665 removed in-process plugin support on Desktop. Except that we still use it for Flash when running specifically on Android 2.3.

Aside from that, we should be able to delete a whole bunch of code.
Attachment #8679291 - Attachment description: bug1218688-part1.patch → part 1, rm windows code
Historically (IIRC) the sync drawing code came first and async rendering came after. But yes, we don't need to support that on Desktop any more. We do need Android to keep working though.
Good to know - I'll leave the Android code alone then.
Benoit, context: I think this code is all related to synchronously painting plugins on Mac, and therefore dead. Seems green on Try.
Attachment #8679326 - Attachment is obsolete: true
Attachment #8679658 - Flags: review?(bgirard)
Assert that on non-Android platforms, we do not try to paint a windowless plugin synchronously.
Attachment #8679659 - Flags: review?(benjamin)
Comment on attachment 8679658 [details] [diff] [review]
part 3, rm mac code

Review of attachment 8679658 [details] [diff] [review]:
-----------------------------------------------------------------

I think this code could still, in theory, be used by someone running 32-bit in-process legacy plugins.

That population may very well be zero and I don't think we should maintain this code patch. r+
Attachment #8679658 - Flags: review?(bgirard) → review+
Comment on attachment 8679659 [details] [diff] [review]
part 4, assert no synchronous painting

FWIW, we stopped supporting in-process plugins on any desktop system (only Android).
Attachment #8679659 - Flags: review?(benjamin) → review+
Attachment #8679291 - Flags: review?(jmathies) → review+
Comment on attachment 8679324 [details] [diff] [review]
part 2, rm linux code

Review of attachment 8679324 [details] [diff] [review]:
-----------------------------------------------------------------

I can't believe all the nsPluginFrame painting code is finally going away. :) feels good to see it go.
Attachment #8679324 - Flags: review?(jmathies) → review+
Depends on: 1232888
You need to log in before you can comment on or make changes to this bug.