507895 - Scrolling on pages with MPlayer plugin is slow since child widget removal

Status: RESOLVED FIXED

(Core Graveyard :: Widget: OS/2, defect)

Description

[Peter Weilbacher]

+++ This bug was initially created as a clone of Bug #501618 +++

See bug 501618 comment 17. I suspect that something about the Resize() call http://hg.mozilla.org/mozilla-central/file/86b666ca54d0/widget/src/os2/nsWindow.cpp#l2042 can still be optimized. (This is a minor regression over the state before widget removal.)

Comment 1

[Rich Walsh]

I've spent about 10 hours working on this, making every change I could think of to nsWindow, plus a few to nsPluginNativeWindowOS2.  My conclusion is that the browser code cannot fix this problem & that the plugin will have to be updated to accommodate changed circumstances.

In particular, the plugin should not respond to changes in the size of the mozilla plugin window.  Instead, it should use the size of the frame window it creates to set the video's dimensions.  The code in nsWindow::ConfigureChildren() will ensure that the frame is always at its maximum size and positioned correctly with the mozilla plugin window.

Also, if the plugin is updated, support for the 'object' tag should be added - 'embed' is obsolete.

Comment 2

[Peter Weilbacher]

I can add another 1/2 hour of trying out changes in nsWindow, always making matters worse.

But actually, it does support <object> here and there, because I definitely see it working on http://www.yolinux.com/MIME-EXAMPLES/WMV-VideoAppletTest.html (a page Steve pointed me to in another bug). I didn't find anything in the plugin docs that tells plugin developers that they have to do something special to support both object and embed tags.

komh, please let me know, if you need more info from the Mozilla side to fix your plugin for Firefox after version 3.5.

Comment 3

[KO Myung-Hun]

Hi/2.

Only one thing I know, is a flooding problem on DIVE mode.

And as before, sending WM_VRNDISABLE to the children of plugin window is the solution.

But I'm afraid I don't use Firefox 3.x later due to a DBCS font problem.

Nevertheless I willing to test the your testcases if you let me know where the binaries are.

Comment 4

[Rich Walsh]

(In reply to comment #3)
> Only one thing I know, is a flooding problem on DIVE mode.  And as
> before, sending WM_VRNDISABLE to the children of plugin window is the
> solution.

That problem has been solved.  The problem now is that sometimes the browser has to shrink its plugin window to keep it from covering other content.  When that happens, mplayer shrinks its video output - but it shouldn't.  It should not look at the size of the mozilla plugin window, it should use the size of its frame window.  As I said above, the browser will make sure the frame is the correct size.

> Nevertheless I willing to test the your testcases if you let me know
> where the binaries are.

http://e-vertise.com/warpzilla/firefox-36a1pre.zip  (I included a copy of gcc433.dll in case you don't have it.)

Use a page where the plugin scrolls off the bottom of the page.  If you scroll when the plugin is partly off the page, the image will shrink (because mplayer adjusted its size when the plugin window shrank), then the image will return to full size (because the browser restored mplayer's frame to the correct size).

Comment 5

[Rich Walsh]

After thinking about this for a while, I have to say that sending WM_VRNDISABLED msgs to the mplayer plugin's frame window is a really bad idea.  And, if the plugin itself is sending msgs to its windows, that's a really bad idea also.

Here's why:  the mplayer plugin's frame & output windows belong to another process.  When you send a msg between threads, the source thread is suspended until the target thread has completed processing it.  PM handles the msg as though it were the first msg posted to the target's msg queue.  If that thread isn't busy, it get processed immediately.  However, if the target is busy processing another msg, the sent msg has to wait.  And, if the target has gone into outer space & stopped servicing its queue, the source thread will be permanently blocked.

I've seen this repeatedly as I've tested using the mplayer plugin - it's a major source of instability in the browser.  The general solution is to use another form of inter-process communication between the plugin and mplayer.exe (a pipe might work but a DOS queue would probably be better).  For the WM_VRNDISABLED msgs, the browser should only send them to its (mozilla) plugin window.  The plugin code should intercept the msg and use its IPC mechanism to signal mplayer.exe that video output has to be suspended.

Comment 6

[KO Myung-Hun]

Hi/2.

(In reply to comment #4)
> (In reply to comment #3)
> > Only one thing I know, is a flooding problem on DIVE mode.  And as
> > before, sending WM_VRNDISABLE to the children of plugin window is the
> > solution.
> 
> That problem has been solved.  The problem now is that sometimes the browser
> has to shrink its plugin window to keep it from covering other content.  When
> that happens, mplayer shrinks its video output - but it shouldn't.  It should
> not look at the size of the mozilla plugin window, it should use the size of
> its frame window.  As I said above, the browser will make sure the frame is the
> correct size.

I don't understand why the plugin window should be shrunk to keep it from covering other content. Would you mind providing some examples that a plugin window is covering other content ?

And if some plugin use a plugin window directly without creating its own window, plugin should conserve its window size ?

I don't understand why Mozilla team determines to use this approach.

> 
> > Nevertheless I willing to test the your testcases if you let me know
> > where the binaries are.
> 
> http://e-vertise.com/warpzilla/firefox-36a1pre.zip  (I included a copy of
> gcc433.dll in case you don't have it.)
> 
> Use a page where the plugin scrolls off the bottom of the page.  If you 
scroll
> when the plugin is partly off the page, the image will shrink (because 
mplayer
> adjusted its size when the plugin window shrank), then the image will return 
to
> full size (because the browser restored mplayer's frame to the correct size).

Ok, I confirmed.

But if scrolling a page while playing movie, the movie size is restored. But 
only when restarting to play movie with a plugin window covered partly, the 
movie size is shrunk. Of course, scrolling the page restores the movie size.

In addition, if a plugin window is covered on a top boundary, the movie size 
is not resized.

And, 'Click here to play' msg is affected by a plugin window size. As I said 
above, in this case, the original window size is conserved not to be affected 
by resizing of a plugin window ?

Comment 7

[KO Myung-Hun]

Hi/2.

Peter also pointed out this before, too.

But, in that case, we concluded that we could ignore because a plugin itself did not work correctly, yet.

And the important thing is that the browser should not update a page until a plugin processs WM_VRNDISABLED msg.

In fact, WM_VRNDISABLED should be sent by a DIVE system, but it is not sent to a MPlayer window strangely. So we come to send it directly.

Anyway, would you mind providing some examples to reproduce the problematic situation of MPlayer plugin ?

If it can be fixed, it would be better.

As well as, it can be used as a testcase for a new method if we should use it.

(In reply to comment #5)
> After thinking about this for a while, I have to say that sending
> WM_VRNDISABLED msgs to the mplayer plugin's frame window is a really bad idea. 
> And, if the plugin itself is sending msgs to its windows, that's a really bad
> idea also.
> 
> Here's why:  the mplayer plugin's frame & output windows belong to another
> process.  When you send a msg between threads, the source thread is suspended
> until the target thread has completed processing it.  PM handles the msg as
> though it were the first msg posted to the target's msg queue.  If that thread
> isn't busy, it get processed immediately.  However, if the target is busy
> processing another msg, the sent msg has to wait.  And, if the target has gone
> into outer space & stopped servicing its queue, the source thread will be
> permanently blocked.
> 
> I've seen this repeatedly as I've tested using the mplayer plugin - it's a
> major source of instability in the browser.  The general solution is to use
> another form of inter-process communication between the plugin and mplayer.exe
> (a pipe might work but a DOS queue would probably be better).  For the
> WM_VRNDISABLED msgs, the browser should only send them to its (mozilla) plugin
> window.  The plugin code should intercept the msg and use its IPC mechanism to
> signal mplayer.exe that video output has to be suspended.

Comment 8

[Rich Walsh]

(In reply to comment #6)
> I don't understand why the plugin window should be shrunk to keep it
> from covering other content. Would you mind providing some examples
> that a plugin window is covering other content ?
> 
> I don't understand why Mozilla team determines to use this approach.

The way pages are composed has been changed.  A webpage used to have many child windows containing content.  Now, there is just one webpage window that contains all content & the only child windows belong to plugins.

For example, the vertical scrollbar used to be a separate window.  If you scrolled a plugin to the right, it's right side would be clipped when it hit the scrollbar because it had reached the edge of its parent window.  Now, the scrollbar is part of the parent window, so the plugin will cover it and won't be clipped until it reaches the browser's frame.

On other systems (e.g. Windows), you can assign a clipping region to a window and all painting will be restricted to that region.  On OS/2, you can only assign a clipping region to an HPS and I don't know of any way to force a window to use a particular HPS.

As a workaround, we use the mozilla plugin window as a form of clipping region.  By reducing the size of the plugin window and repositioning its child window, we can clip any side(s) we need to remove.  Of course, this won't work if we need to clip out the center of the plugin window.  Fortunately, there aren't too many pages that need that ability.

> if a plugin window is covered on a top boundary, the movie size
> is not resized.

This is a bug (actually, it's the result of an incomplete design).  When the plugin reaches the top or left side, I don't change its size.  Instead, I let PM clip the child when it goes outside its parent's boundaries.

However, yesterday I realized that if a page has content that doesn't move when scrolled (e.g. a header), it will be covered when the plugin is scrolled to the top or left.  I'll file a new bug when I have some code to fix this.

Comment 9

[KO Myung-Hun]

Hi/2.

(In reply to comment #8)
> (In reply to comment #6)
> > I don't understand why the plugin window should be shrunk to keep it
> > from covering other content. Would you mind providing some examples
> > that a plugin window is covering other content ?
> > 
> > I don't understand why Mozilla team determines to use this approach.
> 
> The way pages are composed has been changed.  A webpage used to have many child
> windows containing content.  Now, there is just one webpage window that
> contains all content & the only child windows belong to plugins.
> 
> For example, the vertical scrollbar used to be a separate window.  If you
> scrolled a plugin to the right, it's right side would be clipped when it hit
> the scrollbar because it had reached the edge of its parent window.  Now, the
> scrollbar is part of the parent window, so the plugin will cover it and won't
> be clipped until it reaches the browser's frame.
> 

I've tested this, but a plugin window does not overlap the scroll bar.

> On other systems (e.g. Windows), you can assign a clipping region to a window
> and all painting will be restricted to that region.  On OS/2, you can only
> assign a clipping region to an HPS and I don't know of any way to force a
> window to use a particular HPS.
> 
> As a workaround, we use the mozilla plugin window as a form of clipping region.
>  By reducing the size of the plugin window and repositioning its child window,
> we can clip any side(s) we need to remove.  Of course, this won't work if we
> need to clip out the center of the plugin window.  Fortunately, there aren't
> too many pages that need that ability.
> 

If you decide to use a window for clipping region, how about introducing one more window ?

That is, make two windows for a plugin, at least.

The one is for clipping, and the other is a plugin itself. Of course, the former is a parent of the latter. by this, you can clip any region of a plugin window, resizing the parent window. As well as, you can solve a backward compatibility with a old plugin.

In addition, by introducing another child window of a plugin window, you can clip the center region as well.

Comment 10

[Rich Walsh]

(In reply to comment #9)

> Now, the scrollbar is part of the parent window, so the plugin will
> cover it and won't be clipped until it reaches the browser's frame.
>
> I've tested this, but a plugin window does not overlap the scroll bar.

That's because I've already fixed this.

> If you decide to use a window for clipping region, how about
> introducing one more window ?
> That is, make two windows for a plugin, at least.

The other child windows were removed to simplify the browser's structure.  Adding a new child window is contrary to that goal.

If other plugins don't have a problem when the window is resized but mplayer does, then it's because mplayer is doing more than it should.
I would think this is easy to fix.

Comment 11

[Rich Walsh]

(In reply to comment #8)

> > if a plugin window is covered on a top boundary, the movie size
> > is not resized.
> 
> This is a bug
> I'll file a new bug when I have some code to fix this.

See Bug 508726

Comment 12

[KO Myung-Hun]

Hi/2.

(In reply to comment #10)
> (In reply to comment #9)
> 
> > Now, the scrollbar is part of the parent window, so the plugin will
> > cover it and won't be clipped until it reaches the browser's frame.
> >
> > I've tested this, but a plugin window does not overlap the scroll bar.
> 
> That's because I've already fixed this.
> 

Good.

> > If you decide to use a window for clipping region, how about
> > introducing one more window ?
> > That is, make two windows for a plugin, at least.
> 
> The other child windows were removed to simplify the browser's structure. 
> Adding a new child window is contrary to that goal.
> 
> If other plugins don't have a problem when the window is resized but mplayer
> does, then it's because mplayer is doing more than it should.

Hmmm... What is it a plugin should do ?

> I would think this is easy to fix.

I think it would be better that Mozilla makes sure the backward compatibility better than let a plugin overcome its design flaw.

Anyway, however, how does a child window of a plugin know its frame size ?

For examples, let's think about a plugin window showing partly at bottom. When a child window is created at that time, it cannot know the entire size of a plugin window. 

And if the child window does not respond to a plugin window, its size is fixed to a partial size of a plugin window.

Any idea ?

Comment 13

[Peter Weilbacher]

(In reply to comment #3)
> But I'm afraid I don't use Firefox 3.x later due to a DBCS font problem.

This is off-topic here, but can you please remind me what problem that is? I don't think we have a bug on file for anything with DBCS fonts, but if you have a reproducible testcase, please create a new one. It would be a shame if we lost Asian users because of this.

Comment 14

[KO Myung-Hun]

Hi/2.

(In reply to comment #13)
> (In reply to comment #3)
> > But I'm afraid I don't use Firefox 3.x later due to a DBCS font problem.
> 
> This is off-topic here, but can you please remind me what problem that is? I
> don't think we have a bug on file for anything with DBCS fonts, but if you have
> a reproducible testcase, please create a new one. It would be a shame if we
> lost Asian users because of this.

Thanks for your concern about a DBCS problem.

See https://bugzilla.mozilla.org/show_bug.cgi?id=509317

Comment 15

[Rich Walsh]

The issues with the mplayer plugin have been resolved by Bug 514408.