1276020 - [e10s] Flash content does not appear

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[blinky]

Steps to reproduce:

Open this flash in the same tab or by copy and past the link into the address bar https://dl.dropboxusercontent.com/u/95157096/85f61cf7/3hl97q6u59.swf.

I am using the latest Flash Player beta 22.0.0.168.


Actual results:

It displays a blank page instead of Flash content.

See https://dl.dropboxusercontent.com/u/95157096/85f61cf7/uxqgr9mb9e.mp4

Comment 1

[blinky]

Regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=02c69f4f896255189ce2f9f4e0d875e383bcfbd7&tochange=a1bd47d76f71162534090485acc57866dcd55eef

Regressed by: bug 1229961

Comment 2

[Sotaro Ikeda [:sotaro]]

Attachment: patch - InvalidateRect in PluginInstanceParent::SetCurrentImage()

Comment 3

[Sotaro Ikeda [:sotaro]]

attachment 8757248 [details] [diff] [review] addressed the problem for me.

Comment 4

[Sotaro Ikeda [:sotaro]]

To reproduce the problem, I used latest "Flash Player beta 22.0.0.168" as in Bug 1275680 Comment 8.

Comment 5

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #4)
> To reproduce the problem, I used latest "Flash Player beta 22.0.0.168" as in
> Bug 1275680 Comment 8.

And also direct plugin drawing needs to be enabled.

Comment 6

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 8757248 [details] [diff] [review]
patch - InvalidateRect in PluginInstanceParent::SetCurrentImage()

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

Sotaro, could you explain why this change works? This was designed to not need to invalidate anything in layout: SetCurrentImage bumps the frame ID which should tell the compositor it needs to composite the layer again, and SetCurrentImages should perform an async transaction.

Comment 7

[Sotaro Ikeda [:sotaro]]

When ImageContainer is in ImageContainer::ASYNCHRONOUS, ImageClientSingle is created on ImageBridge. ImageHost is also created on ImageBridge. At this point, ImageHost is not connected to ImageLayerComposite. Even when ImageHost receives an image to render, ScheduleComposition() is not called in ReceiveCompositableUpdate()
  https://dxr.mozilla.org/mozilla-central/source/gfx/layers/ipc/CompositableTransactionParent.cpp#196

To connect ImageHost to ImageLayerComposite, ImageClientBridge::UpdateImage() needs to be called. ImageClientBridge is on CompositorBridge. To call the UpdateImage(), LayerTransaction needs to be triggered. To trigger the layer transaction, the patch called Invalidate.

  https://github.com/sotaroikeda/firefox-diagrams/blob/master/gfx/gfx_ImageBridge_FirefoxOS_2_1.pdf

Comment 8

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Call invalidate to trigger ImageClientBridge::UpdateImage()

This patch also worked.

Comment 9

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8758090 [details] [diff] [review]
patch - Call invalidate to trigger ImageClientBridge::UpdateImage()

:dvander, which patch is better? Or should be address the problem another way?

Comment 10

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #9)
> Comment on attachment 8758090 [details] [diff] [review]
> patch - Call invalidate to trigger ImageClientBridge::UpdateImage()

It might be fragile in a case that layer is recreated compared to attachment 8757248 [details] [diff] [review].

Comment 11

[David Anderson [:dvander] - inactive, e-mail if emergency]

(In reply to Sotaro Ikeda [:sotaro] from comment #10)
> (In reply to Sotaro Ikeda [:sotaro] from comment #9)
> > Comment on attachment 8758090 [details] [diff] [review]
> > patch - Call invalidate to trigger ImageClientBridge::UpdateImage()
> 
> It might be fragile in a case that layer is recreated compared to attachment
> 8757248 [details] [diff] [review].

How does video handle this scenario? Does it delete the ImageContainer? (I'm fine with taking the first patch if we think the new approach is fragile, but I want to understand what's happening first)

Comment 12

[Sotaro Ikeda [:sotaro]]

During video rendering there is a case that ImageLayer does not exist. VideoFrameContainer::InvalidateWithFlags() handles it. frame->InvalidateFrame() or frame->InvalidateLayer() is called for each video rendering. From it, I just thought the first patch is safer.
  https://dxr.mozilla.org/mozilla-central/source/dom/media/VideoFrameContainer.cpp#169

Comment 13

[David Anderson [:dvander] - inactive, e-mail if emergency]

(In reply to Sotaro Ikeda [:sotaro] from comment #12)
> During video rendering there is a case that ImageLayer does not exist.
> VideoFrameContainer::InvalidateWithFlags() handles it.
> frame->InvalidateFrame() or frame->InvalidateLayer() is called for each
> video rendering. From it, I just thought the first patch is safer.
>  
> https://dxr.mozilla.org/mozilla-central/source/dom/media/VideoFrameContainer.
> cpp#169

Okay, thanks for the explanation. The first patch seems reasonable then.

Comment 14

[Sotaro Ikeda [:sotaro]]

Attachment: patch - InvalidateRect in PluginInstanceParent::SetCurrentImage()

Fixed warining.

Comment 15

[Sotaro Ikeda [:sotaro]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c3126d1f84f

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/189923b75373
InvalidateRect in PluginInstanceParent::SetCurrentImage() r=dvander

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/189923b75373