windowless plugins in object elements with data (e.g. certain Flash pages) are not painted

RESOLVED FIXED

Status

()

--
major
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: playwatch, Assigned: romaxa)

Tracking

({regression})

Trunk
All
Linux
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(blocking2.0 beta7+)

Details

(Whiteboard: [flash], URL)

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

8 years ago
User-Agent:       Mozilla/5.0 (X11; Linux i686; rv:2.0b7pre) Gecko/20100924 Firefox/4.0b7pre
Build Identifier: Mozilla/5.0 (X11; Linux i686; rv:2.0b7pre) Gecko/20100924 Firefox/4.0b7pre

On some pages flash plugin stopped working

Reproducible: Always

Steps to Reproduce:
1.Go to http://www.youtube.com/user/tippexperience with a fresh profile
2.wait until site is loaded
3.
Actual Results:  
Instead of the video there is only a white space

Expected Results:  
The video should start

Another page with this issue is http://www.divineproject.altervista.org/.
I'll try to find the latest working build.
(Reporter)

Updated

8 years ago
Version: unspecified → Trunk
(Reporter)

Comment 1

8 years ago
Latest Working hourly is 1285098901-20100921125501-81c0aef6b272-firefox-4.0b7pre.en-US.linux-i686.
First Broken hourly is 1285106168-20100921145608-8b252736a650-firefox-4.0b7pre.en-US.linux-i686.tar.bz2.
(Reporter)

Updated

8 years ago
blocking2.0: --- → ?
(Reporter)

Comment 2

8 years ago
So this bug it's probably caused by https://bugzilla.mozilla.org/show_bug.cgi?id=598112 patches...

Updated

8 years ago
Keywords: qawanted

Comment 3

8 years ago
anyone else seeing this?
I see it on Vimeo and YTMND
http://vimeo.com/14980662


I can conform the regression range insofar that the Linux x86_64 2010-09-21 nightly works and the 2010-09-22 nightly does not.

Comment 5

8 years ago
I see this with Mozilla/5.0 (X11; Linux x86_64; rv:2.0b7pre) Gecko/20100101 Firefox/4.0b7pre (self-built) on Fedora 13 with both Flash 10.1.85.3 (using nspluginwrapper) and 10.2.161.22, the 64-bit preview.

For me, this issue affects the the Moogalover player used by Vimeo (http://vimeo.com). YouTube videos work OK.

If I set dom.ipc.plugins.enabled to false, the Flash videos appear, though on some sites, i.e. http://www.cartoonbrew.com/, embedded Vimeo videos show as black with a play button on it. On these sites, playing the video gives audio and the player's controls, but the video itself remains black.

Comment 6

8 years ago
I see the same on a windows build that has patches from bug 596451.

Backing out

-  if (RUNNING != mRunning)
-    return NS_OK;
-

from bug 598112 fixes the issue.
(Assignee)

Comment 7

8 years ago
Yep, I see it is reproducible, 

> -  if (RUNNING != mRunning)
> -    return NS_OK;
> -
> 
removing this will disable plugin layers implementation....


(In reply to comment #6)
> I see the same on a windows build that has patches from bug 596451.

interesting why this is visible windows build, because plugin layers should not be available on windows
Assignee: nobody → romaxa
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(Assignee)

Updated

8 years ago
Depends on: 599704
(Reporter)

Comment 8

8 years ago
(In reply to comment #5)
> I see this with Mozilla/5.0 (X11; Linux x86_64; rv:2.0b7pre) Gecko/20100101
> Firefox/4.0b7pre (self-built) on Fedora 13 with both Flash 10.1.85.3 (using
> nspluginwrapper) and 10.2.161.22, the 64-bit preview.
> 
> For me, this issue affects the the Moogalover player used by Vimeo
> (http://vimeo.com). YouTube videos work OK.
> 
> If I set dom.ipc.plugins.enabled to false, the Flash videos appear, though on
> some sites, i.e. http://www.cartoonbrew.com/, embedded Vimeo videos show as
> black with a play button on it. On these sites, playing the video gives audio
> and the player's controls, but the video itself remains black.

Vimeo black playback happens even in the latest beta, so it's probably another regression...
(Assignee)

Comment 9

8 years ago
Ok, I found where is the problem,
ObjectFrame and PluginInstance created and then PluginInstance immediately destroyed.
After that new PluginInstance created with the same ObjectFrame but CallSetWindow not called and surface not initialized.

I'll fix it soon
(Assignee)

Comment 10

8 years ago
Easy way to get that working is press CTRL + and CTRL+0 (force reflow and setWindow call)
(Reporter)

Comment 11

8 years ago
I cam make it work by changing the zoom, but animations freeze by right-clicking them.
(Reporter)

Comment 12

8 years ago
(In reply to comment #8)
> (In reply to comment #5)
> > I see this with Mozilla/5.0 (X11; Linux x86_64; rv:2.0b7pre) Gecko/20100101
> > Firefox/4.0b7pre (self-built) on Fedora 13 with both Flash 10.1.85.3 (using
> > nspluginwrapper) and 10.2.161.22, the 64-bit preview.
> > 
> > For me, this issue affects the the Moogalover player used by Vimeo
> > (http://vimeo.com). YouTube videos work OK.
> > 
> > If I set dom.ipc.plugins.enabled to false, the Flash videos appear, though on
> > some sites, i.e. http://www.cartoonbrew.com/, embedded Vimeo videos show as
> > black with a play button on it. On these sites, playing the video gives audio
> > and the player's controls, but the video itself remains black.
> 
> Vimeo black playback happens even in the latest beta, so it's probably another
> regression...

https://bugzilla.mozilla.org/show_bug.cgi?id=596454
I see this quite a bit on newgrounds.com/movies and www.youtube.com on friday's nightly build of Fennec on maemo. I'm getting either a blank space or the plugin disabled message.
Keywords: qawanted
Hardware: x86 → All
tracking-fennec: --- → ?
(Assignee)

Comment 14

8 years ago
Created attachment 478673 [details] [diff] [review]
Track call set window  after SetInstance call

Variant 1, track destroy calls, and call setWindow in BuildLayer if new Instance was installed
Attachment #478673 - Flags: feedback?(roc)
(Assignee)

Comment 15

8 years ago
Created attachment 478675 [details] [diff] [review]
another version, check the  same what DrawWithXlib cheking

in non-layers plugin mode, we have SetWindow call coming from DrawWithXlib call because of offset not equals to mPluginWindow->x/y.

This patch is trying to do the same thing
Attachment #478675 - Flags: feedback?(roc)
(Assignee)

Comment 16

8 years ago
Comment on attachment 478673 [details] [diff] [review]
Track call set window  after SetInstance call

roc is on vacation
Attachment #478673 - Flags: feedback?(roc) → feedback?(benjamin)
(Assignee)

Comment 17

8 years ago
Comment on attachment 478675 [details] [diff] [review]
another version, check the  same what DrawWithXlib cheking

roc is on vacation, benjamin I need your feedback
Attachment #478675 - Flags: feedback?(roc) → feedback?(benjamin)
Whiteboard: [flash]

Updated

8 years ago
blocking2.0: ? → beta7+

Comment 18

8 years ago
Comment on attachment 478673 [details] [diff] [review]
Track call set window  after SetInstance call

This one seems clearer to me. We should really get some sort of peer to review this: karlt/bz/josh?
Attachment #478673 - Flags: feedback?(benjamin) → feedback+
(Assignee)

Updated

8 years ago
Attachment #478673 - Flags: review?(karlt)

Comment 19

8 years ago
There are also issues with Google Maps, as reported in bug 599492 and bug 599493.

Also http://www.norc.at/street-view/ is having Flash related issues.

May they be related to this bug?
Also seems to affect http://www.cnn.com/video/
Keywords: regression
Summary: Flash does not work on certain pages → Flash content is not painted on certain pages
Comment on attachment 478675 [details] [diff] [review]
another version, check the  same what DrawWithXlib cheking

With non-layer X11 plugins, the first SetWindow always arrives before the first
paint, even with 0 offset, because the visual (and colormap) are 0 until
the first paint.

With non-layer GDI plugins, the first SetWindow always arrives because the HDC
is null until the first paint.
Attachment #478675 - Attachment is obsolete: true
Attachment #478675 - Flags: feedback?(benjamin)
It should be possible to write a simple reftest based on
data:text/html,<object width="200" height="200" data="data:application/x-test,0" drawmode="solid" color="ff00ff00"></object>
Flags: in-testsuite?
Comment on attachment 478673 [details] [diff] [review]
Track call set window  after SetInstance call

Because the plugin is painting asynchronously, I think it would be better to
tell the plugin its size and tell the host we are using layers as soon as possible, rather than at the point where we try to use what it would paint.

This looks like a good place for that,
http://hg.mozilla.org/mozilla-central/annotate/5a2012482a63/modules/plugin/base/src/nsPluginStreamListenerPeer.cpp#l598
which would be more consistent with the currently working
nsObjectFrame::Instantiate(const char* aMimeType, nsIURI* aURI) behavior (as well as more consistent with windowed plugin behavior).
tracking-fennec: ? → ---
Comment on attachment 478673 [details] [diff] [review]
Track call set window  after SetInstance call

This issue here is not that SetInstance is replacing an existing PluginInstance with a new one (which would fire an assertion), but that these object elements use nsObjectFrame::Instantiate(nsIChannel* aChannel, nsIStreamListener** aStreamListener), which, unlike nsObjectFrame::Instantiate(const char* aMimeType, nsIURI* aURI), does not call AsyncSetWindow.

The appropriate fix is to call AsyncSetWindow as indicated in comment 23.
Attachment #478673 - Flags: review?(karlt) → review-
Summary: Flash content is not painted on certain pages → windowless plugins in object elements with data (e.g. certain Flash pages) are not painted
(Assignee)

Comment 25

8 years ago
Created attachment 480136 [details] [diff] [review]
part 2: Fixd according karl comment
Attachment #478673 - Attachment is obsolete: true
Attachment #480136 - Flags: review?(karlt)
Blocks: 598112

Comment 26

8 years ago
We really need an automated test for this. Can we do it with a reftest?

Updated

8 years ago
Duplicate of this bug: 601212
(In reply to comment #26)
> We really need an automated test for this. Can we do it with a reftest?

Yes.  Compare a solid green div with

<object width="200" height="100"
data="data:application/x-test,0" drawmode="solid" color="ff00ff00"></object>
Comment on attachment 480136 [details] [diff] [review]
part 2: Fixd according karl comment

This part is good, but we can't use it until UseAsyncPainting knows about mozilla.plugins.use_layers.  I assume testing that pref can be moved to nsNPAPIPluginInstance.
Attachment #480136 - Flags: review?(karlt)
(Assignee)

Comment 30

8 years ago
Created attachment 480364 [details] [diff] [review]
part 1: prefs part
Attachment #480364 - Flags: review?(karlt)
(Assignee)

Comment 31

8 years ago
Comment on attachment 480136 [details] [diff] [review]
part 2: Fixd according karl comment

I think these two patches make sense now.
Attachment #480136 - Flags: review?(karlt)
(Assignee)

Comment 32

8 years ago
Created attachment 480367 [details] [diff] [review]
reftest
Attachment #480367 - Flags: review?(karlt)
Comment on attachment 480364 [details] [diff] [review]
part 1: prefs part

>   PRBool UseLayers()
>   {
>-    return (mUsePluginLayers &&
>+    PRBool useAsyncRendering = PR_FALSE;

No need to initialize useAsyncRendering as there is an NS_SUCCEEDED check.

>+    return (NS_SUCCEEDED(mInstance->UseAsyncPainting(&useAsyncRendering)) &&
>+            useAsyncRendering &&
>             (!mPluginWindow ||
>              mPluginWindow->type == NPWindowTypeDrawable));

Add a null check for mInstance.

>+  nsCOMPtr<nsIPrefBranch> prefs(do_GetService(NS_PREFSERVICE_CONTRACTID));
>+  if (prefs) {
>+    PRBool useLayersPref = mUsePluginLayersPref;

No need to initialize useLayersPref.
It is only an outparam for GetBoolPref().
Attachment #480364 - Attachment description: prefs part → part 1: prefs part
Attachment #480364 - Flags: review?(karlt) → review+
Attachment #480136 - Flags: review?(karlt) → review+
Comment on attachment 480367 [details] [diff] [review]
reftest

Thanks!
Attachment #480367 - Flags: review?(karlt) → review+
Attachment #480136 - Attachment description: Fixd according karl comment → part 2: Fixd according karl comment
(Assignee)

Comment 35

8 years ago
http://hg.mozilla.org/mozilla-central/rev/bb18e81ea0f8
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.