Drawing page with drawWindow duplicates flash content

VERIFIED FIXED in mozilla1.9.1b2

Status

()

Core
Plug-ins
--
major
VERIFIED FIXED
9 years ago
9 years ago

People

(Reporter: mossop, Assigned: smichaud)

Tracking

({regression, verified1.9.1})

Trunk
mozilla1.9.1b2
All
Mac OS X
regression, verified1.9.1
Points:
---
Bug Flags:
blocking1.9.1 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(8 attachments, 3 obsolete attachments)

(Reporter)

Description

9 years ago
+++ This bug was initially created as a clone of Bug #408898 +++

I'm still seeing the same issue as bug 408898, quite frequently now. I'm not on flash 9.0.124. It doesn't seem to be every drawWindow call that duplicates the flash display, but I can reasonably reliably see it after using pandora for a short time with my add-on Tab Sidebar installed and enabled for a short time (http://www.oxymoronical.com/files/extensions/TabSidebar/archive/TabSidebar-2.5a1.rev395.xpi).

Given that we are starting to use drawWindow in the product with ctrl-tab and other planned ideas I think we should consider blocking on this as it is pretty ugly when it happens.
Flags: blocking1.9.1?
(Reporter)

Comment 1

9 years ago
Created attachment 342749 [details]
screenshot
(Assignee)

Comment 2

9 years ago
We can't really block on this until we have steps to reproduce.

Does it also happen on Windows and/or Linux?
(Reporter)

Comment 3

9 years ago
STR:

1. Install the add-on in comment 0.
2. Restart Firefox and go to view - sidebar - Tab Sidebar.
3. Go to www.pandora.com.
4. If the problem isn't immediately evident click play/pause a few times or resize the sidebar.
(Assignee)

Comment 4

9 years ago
(In reply to comment #3)

I can't reproduce your STR.  I'm using Flash 9.0 r124 (which is the
current version from Apple).  I tested on OS X 10.5.5.

> I'm not on flash 9.0.124.

What version are you using.
(Assignee)

Comment 5

9 years ago
Should have mentioned that I tested with today's Minefield nightly.
(Reporter)

Comment 6

9 years ago
(In reply to comment #4)
> (In reply to comment #3)
> 
> I can't reproduce your STR.  I'm using Flash 9.0 r124 (which is the
> current version from Apple).  I tested on OS X 10.5.5.
> 
> > I'm not on flash 9.0.124.
> 
> What version are you using.

Should have read "I'm now on flash 9.0.124"
(Reporter)

Comment 7

9 years ago
Try it with extensions.tabsidebar.selecteddelay at -1 and extensions.tabsidebar.position as 3
(Assignee)

Comment 8

9 years ago
Ok, with these settings (which I assume are integer settings) I saw the upside down display once (in the Tab Sidebar).  But that doesn't a good STR make (and even I wasn't able to figure out exactly what I'd done to trigger the problem).

Please give us an STR that works at least 50% of the time.  A reduced testcase would also be really nice :-)
Matthew Gregan probably has the best clue here.
Created attachment 343156 [details]
flash media for testcase
Created attachment 343159 [details]
testcase

This testcase reproduces the issue reliably for me.  Open the testcase, then hold the cursor down key to move the fixed position div containing the flash and canvas.  Fairly regularly, the canvas will paint at the very bottom left of the browser window.  When this happens, it does not paint in expected location, so you will see the canvas blink each time the invalid paint in the bottom left of the window occurs.

I don't have any insight into the problem yet, but this testcase should help someone work on this.  I'm probably busy all this week, then away next week.  If nobody else has picked this up once I've got some time, I'll take a look at it.
Note that it's a standalone test case using drawWindow, so you don't need Tab Sidebar, but you will need to grant UniversalXPConnect privileges.
(Reporter)

Comment 13

9 years ago
This website appears to do something that triggers it for what most be most of the drawWindow calls, and it doesn't look like it it changing the DOM I think either:

http://blip.tv/file/1229589
(Reporter)

Comment 14

9 years ago
Forgot to add, that website makes it reproducible without my extension. Just have it and a couple of other tabs open then press ctrl-tab.
(Reporter)

Comment 15

9 years ago
Something maybe related. At the site http://thinkprogress.org/2008/10/18/real-virginia/ The flash that is getting duplicated is in the middle left. It is getting duplicated upside down at the bottom left (same as every example). Tab Sidebar is receiving MozAfterPaint events indicating that the top left of the page has been painted though.
(Reporter)

Comment 16

9 years ago
Ok the website in comment 13 is so bad not because of the site but because I was looking since bug 459244 landed. Looks like that has made this problem even worse. With a local backout that site doesn't exhibit the duplication at all.
Blocks: 459244
(In reply to comment #16)

Tha patch for bug 459244 probably just uncovered another bug.  Or to
put this another way, fixing the incorrect behavior described in bug
459244 probably just uncovered (or further uncovered) the true
cause(s) of this bug (bug 459530).

By the way, does adding in the patch for bug 459319 (on top of current
trunk code) make any difference?
(Reporter)

Comment 18

9 years ago
(In reply to comment #17)
> By the way, does adding in the patch for bug 459319 (on top of current
> trunk code) make any difference?

No obvious difference
OK, the patch for bug 459244 has also made this bug very easy for me
to reproduce.  Now all I need do is install TabSidebar (using the URL
from comment #0), choose View : Sidebar : Tab Sidebar, and load any
Flash plugin into a tab (only a single tab need be open).  A mirror
image of the plugin (upside down and backwards) appears in the lower
left.  If the Flash "movie" is animated, the mirror image also
flickers annoyingly.

Here are the Flash "movies" I tested with (in addition to the one from
comment #13).  The first is static.  The second two are animated.

http://www.playercore.com/bugfiles/146162/AddReturnHTML.html
http://www.rogerdean.com
http://www.youtube.com/watch?v=TZ860P4iTaM&NR=1

This is fortunate -- it should make it easier to find the true cause
of this bug.

Sadly, though, I still can't reproduce the problem using kinetik's
testcase from comment #11.
i can repro this easily on a clean profile (no extensions), on the mac.   Simply have one tab running a flash banner, and open a second tab and start typing.   Try http://www.nfl.com/scores another testcase.

Updated

9 years ago
Duplicate of this bug: 460675
> Simply have one tab running a flash banner, and open a second tab
> and start typing.  Try http://www.nfl.com/scores another testcase.

This doesn't work 100% of the time for me.

But I've found that I can do the following:

1) Open http://www.nfl.com/scores.

2) Cmd-t to open another (blank) tab.

3) Wait.  Within 5 minutes (probably less) you'll see the problem.

   So you don't need to type anything.

Someone please provide an example of a Flash banner that updates
continuously.
(In reply to comment #22)
> Someone please provide an example of a Flash banner that updates
> continuously.

Would a Flash-based video count? If so, perhaps this 90-minute video from the last presidential debate would work? http://www.yagoo.ro/play.php?vid=783
> Would a Flash-based video count?

Apparently not.  Neither your example nor the YouTube video from
comment #19 (http://www.youtube.com/watch?v=TZ860P4iTaM&NR=1) "work".
Nor do the other examples from comment #19.

Interestingly, I've found a non-banner ad that *does* "work" very
well.  Go to http://www.vg.no/ and scroll down until the "Stopp
lekkasjen!" ad is displayed.  Then open another	(blank) tab and wait
less than a minute.

Unfortunately I can't find where this ad is loaded (it's not loaded in
the "View Source" page).

Even more interesting:  If I right click on the ad and choose
Settings, then disable (uncheck) hardware acceleration, the problem
stops happening.
The "Stopp lekkansjen!" ad is a dripping faucet.
Created attachment 343939 [details]
Dripping faucet Flash ad

The dripping faucet Flash ad is at
http://flash.vg.no/annonser/2008/varmeogbad/2010/vb_lekasje_vg.swf.
Here's a copy of it.

I still haven't managed to figure out how http://www.vg.no/ is loading
it (what kind of tag it uses).

The problem I describe in comment #22 doesn't happen if you load this
ad as an *.swf file.

Comment 27

9 years ago
FWIW, I can't repro with the latest Minefield nightly and Flash 10 on PPC (with harware acceleration enabled, as it is by default). Maybe this is Flash 9- or Intel-only.
(Reporter)

Comment 28

9 years ago
No I updated to flash today and still see it. Though I am on Intel.
Created attachment 343955 [details]
Testcase with dripping faucet ad

Mstange found out how http://www.vg.no/ loads the dripping faucet
Flash ad.  Here's a semi-reduced testcase that, for me, reliably
reproduces this problem in recent mozilla-central nightlies (those
containing my patch for bug 459244).

1) Start Minefield and load the testcase.

2) Cmd-tab to open a new, blank, tab.

3) Wait 5 seconds.

   If nothing happens, cmd-w to close the blank tab and then cmd-tab
   again to open another blank tab.

   But often something *will* happen -- you'll see a mirrored image of
   the plugin (upside down and backwards) in the lower left of the
   browser page.

Whoever's listening here, please tell me if you can reproduce my STR.
I'm particularly interested in hearing from mossop, tchung and cl.
(Reporter)

Comment 30

9 years ago
If you mean cmd-t to open a new tab then yes I can reproduce. I suspect that whatever code gets the thumbnails for the ctrl-tab display is probably doing so in response to a timer or something and that is what is causing the 5 second delay.
(Reporter)

Comment 31

9 years ago
And boy air mozilla is really irritating because of it :(

Updated

9 years ago
Flags: blocking1.9.1? → blocking1.9.1+

Updated

9 years ago
Hardware: PC → Macintosh
Yes, cmd-tab should be cmd-t.  Sorry :-(

Here's my STR from comment #29 over again:

1) Start Minefield and load the testcase from comment #29 (attachment
   343955).

2) Cmd-t to open a new (blank) tab.

3) Wait 5 seconds.

   If nothing happens, cmd-w to close the blank tab and then cmd-t
   again to open another blank tab.

   But often something *will* happen -- you'll see a mirrored image of
   the plugin (upside down and backwards) in the lower left of the
   browser page.
Flags: blocking1.9.1+ → blocking1.9.1?
Hardware: Macintosh → PC
(Reporter)

Comment 33

9 years ago
putting back blocking+
Flags: blocking1.9.1? → blocking1.9.1+

Comment 34

9 years ago
(In reply to comment #32)
> Yes, cmd-tab should be cmd-t.  Sorry :-(

If this is a Macintosh bug, why did you change the platform back to PC with OS Mac OS X? Is this a bug that only affects hackintoshes or something? I notice it on my Apple Mac, thus it's a Macintosh platform bug.
Hardware: PC → Macintosh
(Reporter)

Comment 35

9 years ago
Although it isn't all that important, the hardware is used to differentiate between intel and ppc for Mac's. In this case it is seen on both so All is more relevant really.
Hardware: Macintosh → All

Comment 36

9 years ago
(In reply to comment #35)
> In this case it is seen on both so All is more
> relevant really.

Dave, has someone actually seen this bug on PPC? I certainly can't reproduce it on PPC, and it seems everyone who's commenting here is on Intel.
(Reporter)

Comment 37

9 years ago
(In reply to comment #36)
> (In reply to comment #35)
> > In this case it is seen on both so All is more
> > relevant really.
> 
> Dave, has someone actually seen this bug on PPC? I certainly can't reproduce it
> on PPC, and it seems everyone who's commenting here is on Intel.

Sorry I misread comment 27 to mean that you had seen it on ppc with Flash 9.
Hardware: All → PC
(In reply to comment #29)
> Created an attachment (id=343955) [details]
> Testcase with dripping faucet ad
> 
> Mstange found out how http://www.vg.no/ loads the dripping faucet
> Flash ad.  Here's a semi-reduced testcase that, for me, reliably
> reproduces this problem in recent mozilla-central nightlies (those
> containing my patch for bug 459244).
> 
> 1) Start Minefield and load the testcase.
> 
> 2) Cmd-tab to open a new, blank, tab.
> 
> 3) Wait 5 seconds.
> 
>    If nothing happens, cmd-w to close the blank tab and then cmd-tab
>    again to open another blank tab.
> 
>    But often something *will* happen -- you'll see a mirrored image of
>    the plugin (upside down and backwards) in the lower left of the
>    browser page.
> 
> Whoever's listening here, please tell me if you can reproduce my STR.
> I'm particularly interested in hearing from mossop, tchung and cl.

I cant' repro the drawWindow issue with your testcase.  But i can repro it pretty easily with the nfl.com/scores one.  

My system:  Intel-based Mac OSX 10.5.2, running a new profile, on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081020 Minefield/3.1b2pre.

I have a PPC mac here in the lab.  i'll try that now.
Hardware: PC → All
okay i take that back.  i *can* repro your upside-down dripping faucet testcase on an Intel Mac after closing/opening a new tab.

However, i *couldn't* repro this on a PPC mac osx 10.4.  Hope that helps you narrow things down.

Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.4; en-US; rv:1.9.1b2pre) Gecko/20081020 Minefield/3.1b2pre
Using my (corrected) STR from comment #32 (which seems to be 100%
effective if followed exactly):

This bug doesn't happen on PPC 10.4.11.

It does happen on Intel 10.5.5 and 10.4.11.

It doesn't happen using a special build in which support for the
CoreGraphics drawing model in plugins has been disabled.

It happens whether or not hardware acceleration (a Flash setting) is
disabled (so what I said at the end of comment #24 is incorrect).

All my tests have (so far) been done with Apple's current Flash
version (9.0 r124).
(Assignee)

Updated

9 years ago
Hardware: All → PC
(In reply to comment #40)
> All my tests have (so far) been done with Apple's current Flash
> version (9.0 r124).

Is not 10.0 r12 the current Flash version? 
http://www.adobe.com/products/flashplayer/
> Is not 10.0 r12 the current Flash version?

It's not *Apple's* current Flash version (the one Apple distributes via Software Update).
Assignee: nobody → joshmoz
Component: Layout: Canvas → Widget: Cocoa
QA Contact: layout.canvas → cocoa
(Assignee)

Updated

9 years ago
Assignee: joshmoz → smichaud
Actually, this is really a plugins bug (though it's OS-X-specific).
Assignee: smichaud → nobody
Component: Widget: Cocoa → Plug-ins
QA Contact: cocoa → plugins
(Assignee)

Updated

9 years ago
Assignee: nobody → smichaud

Comment 44

9 years ago
I can reproduce it quite often on PPC (10.5, Flash 10), most of the test cases posted here reproduce it.
In addition to duplicate flash content, it's also being drawn all over the wrong place, with flash controls misplaced everywhere.

See http://video.yahoo.com
(In reply to comment #45)

Sigh, yes.  That video draws out of place (higher than it should), and
overlays the ticker (also a Flash object?).

That problem also predates my patch for bug 459244.

This is one complex bug (or rather, it's several bugs disguised as
one).  I'm slowly making progress (in fact I've got the STR I reported
in comment #32 fixed).  But over the next couple of days I'm going to
try to fix all the Flash problems at one go.
Tony, could you try to find the regression range for the problems with
http://video.yahoo.com/?  They don't happen with Firefox 3.0.3.
Regression falls between builds:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081011 Minefield/3.1b2pre and 
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081012 Minefield/3.1b2pre

Hg: http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2008-10-11+02%3A00%3A00&enddate=2008-10-12+03%3A00%3A00
Thanks!

The only patch in that range that seems possible (as a trigger for the problem with http://video.yahoo.com/) is bug 458111 ("Remove Mac-specific tabbrowser-tab binding").
dao, if this is really a regression from your patch of bug 458111, please set the product/component field to what it should be.  Thanks.
> dao, if this is really a regression from your patch of bug 458111 ...

"This" is just the problem described in comment #46, not the rest of the bug (which is definitely a plugins bug).

We should eventually spin off the comment #46 problem into a new bug.

But before we do that, it'd be nice to know if I'm right (and it was your patch for bug 458111 that triggered the comment #46 problem).
(In reply to comment #51)
> But before we do that, it'd be nice to know if I'm right (and it was your patch
> for bug 458111 that triggered the comment #46 problem).

I don't think it could. In what way does it seem possible to you?
> In what way does it seem possible to you?

Your patch is the only Mac-specific one in the regression range from comment #48.

Updated

9 years ago
Duplicate of this bug: 460986
(In reply to comment #53)
> > In what way does it seem possible to you?
> 
> Your patch is the only Mac-specific one in the regression range from comment
> #48.

video.yahoo.com looks just as broken in the latest builds on Windows and Linux, so it's certainly not a Mac-specific issue.
> video.yahoo.com looks just as broken in the latest builds on Windows and Linux,
> so it's certainly not a Mac-specific issue.

Broken in the same way, with the same regression range?  (See comment #48.)
(In reply to comment #56)
> > video.yahoo.com looks just as broken in the latest builds on Windows and Linux,
> > so it's certainly not a Mac-specific issue.
> 
> Broken in the same way, with the same regression range?  (See comment #48.)

Yup.
> Broken in the same way, with the same regression range?

I also say yes to both questions ... at least on Linux.
Created attachment 344388 [details] [diff] [review]
Fix

Here's a patch that fixes this bug as originally reported (and my STRs
from comment #19 and comment #32).  It turns out to be caused by a
Flash bug (CoreGraphics-specific), and my patch works around that.  A
tryserver build will follow in an hour or two.

This patch *doesn't* fix the problem discussed in comment #45 through
comment #53 and comment #55 through comment #58.  That's a completely
different/unrelated problem, which effects OS X, Windows and Linux.
In the next hour or so I'll open a new bug on it.
Attachment #344388 - Flags: review?(kinetik)
(Assignee)

Updated

9 years ago
Attachment #344388 - Flags: review?(joshmoz)
> This patch *doesn't* fix the problem discussed in comment #45
> through comment #53 and comment #55 through comment #58.  That's a
> completely different/unrelated problem, which effects OS X, Windows
> and Linux.  In the next hour or so I'll open a new bug on it.

I've spun this off into bug 461266.
Here's a tryserver build made with my patch:

https://build.mozilla.org/tryserver-builds/2008-10-22_15:31-smichaud@pobox.com-bugzilla459530/smichaud@pobox.com-bugzilla459530-firefox-try-mac.dmg

I've tested it (fairly lightly) on OS X 10.5.5 (with Flash 10.0 r12)
and 10.4.11 (with Flash 9.0r124):  First I confirmed that it fixes my
STR from comment #32.  Then I loaded the dripping faucet testcase
(attachment 343955 [details]) and all the URLs from comment #19 into different
tabs, turned on View : Sidebar : TabSidebar, and played around for a
while.

I didn't see this bug even once.
Created attachment 344414 [details] [diff] [review]
Fix rev1 (clean up)

I fixed some potential leaks in
nsPluginInstanceOwner::StartPluginPaint().

I discovered that the Flash plugin can sometimes also paint (and paint
outside its frame) from an idle event.

This last caused the following STR to "work" even with my original
patch:

1) Start Minefield and load the dripping faucet "movie" from comment
   #26 (attachment 343939 [details]).

2) Cmd-t to open a new (blank) tab.

3) Click on the first tab to switch back to it.

4) Wait 2-3 seconds.

   Often the dripping faucet will be (partially) redrawn just below
   its proper location (so that it overwrites the status bar).

This STR no longer "works" with my rev1 patch.

A new tryserver build will follow soon.
Attachment #344388 - Attachment is obsolete: true
Attachment #344414 - Flags: review?(kinetik)
Attachment #344388 - Flags: review?(kinetik)
Attachment #344388 - Flags: review?(joshmoz)
(Assignee)

Updated

9 years ago
Attachment #344414 - Flags: review?(joshmoz)
Comment on attachment 344414 [details] [diff] [review]
Fix rev1 (clean up)

This patch causes SetWindow to be called *much* too often.

I'll need to revise it.
Attachment #344414 - Attachment is obsolete: true
Attachment #344414 - Flags: review?(kinetik)
Attachment #344414 - Flags: review?(joshmoz)

Comment 64

9 years ago
"It turns out to be caused by a Flash bug (CoreGraphics-specific)"

If this is true, we should attempt to get Adobe to fix the problem before working around it in our own code. I don't think the current situation is so pressing that we couldn't wait for a fix from them. We have to live with workarounds in our code for a *long* time and we should be very careful about taking on such burdens.
(In reply to comment #64)
> "It turns out to be caused by a Flash bug (CoreGraphics-specific)"
> 
> If this is true, we should attempt to get Adobe to fix the problem before
> working around it in our own code. I don't think the current situation is so

On a potentially related note, Steven, is this CoreGraphics-specific bug present in both Flash 9 and Flash 10? 

(Supposing for a moment that it's only present in Flash 9, would it be worth targeting our workaround just for that version and subsequently not engaging the workaround if the user is running a Flash version >= 10 ?)
(In reply to comment #65)

> On a potentially related note, Steven, is this CoreGraphics-specific
> bug present in both Flash 9 and Flash 10?

It's present in both.

I think we should fix this (i.e. work around the bug).  It's a pretty
bad problem, and (now) quite easy to reproduce.  There are lots of
other product-specific workarounds in our plugin code, without which
the user experience would be considerably worse.  And a workaround
needs to stay around for a while to be useful -- even if the vendor
fixes the bug right away (very unusual), it takes a while for the fix
to appear in a release, and older releases can stay in use for years
(particularly if, as with the Flash plugin, the plugin is supplied by
someone else than the plugin vendor (in this case by Apple)).

As for my patch for bug 459244, that just unmasked the true bug.
Backing it out would just be a clumsier and stupider workaround.

I'm still working on this, and hope to have a new patch in a couple of
hours.
Created attachment 344532 [details] [diff] [review]
Fix rev2 (new design)

Here's a new patch, with a substantially different design.  I've
tested it against the STRs from comment #19, comment #32 and comment
#62, on OS X 10.5.5 and 10.4.11.  I didn't see any problems.

Here's a tryserver build made with my rev2 patch:

https://build.mozilla.org/tryserver-builds/2008-10-23_11:21-smichaud@pobox.com-bugzilla459530-rev2/smichaud@pobox.com-bugzilla459530-rev2-firefox-try-mac.dmg

Please try it out and let us know your results.  I'm particularly
interested in hearing from mossop.  I won't ask for review until we
get some results from testing (besides those I've already done
myself).
(In reply to comment #67)
> 
> Please try it out and let us know your results.  I'm particularly
> interested in hearing from mossop.  I won't ask for review until we
> get some results from testing (besides those I've already done
> myself).

At first glance, its looking good.  I can't seem to repro the issue anymore on Flash 10, mac osx 10.5.2.  I'll comment more if i see issues.
(Reporter)

Comment 69

9 years ago
With the new build I am still able to reproduce using the video in comment 13 with Tab Sidebar. I can however no longer reproduce in a couple of other places. Unfortunately it looks like flash is no longer getting included with the window content painted with drawWindow calls.
(In reply to comment #69)

> Unfortunately it looks like flash is no longer getting included with
> the window content painted with drawWindow calls.

I saw that, but didn't know what to make of it.

But now I see that it works with the 2008-10-10-02 Minefield nightly
(using a static Flash plugin like
http://www.playercore.com/bugfiles/146162/AddReturnHTML.html from
comment #19).
Not sure if this adds any additional information, but I can still reproduce the problem consistently with a current trunk build and the most recent patch applied using my attached testcase (saved to a local file so it can be given the appropriate privs).

Updated

9 years ago
Duplicate of this bug: 461466
Created attachment 344627 [details]
Patch for logging changes to nsPluginWindow->window->cgPort.context (not a fix)
(Assignee)

Updated

9 years ago
Attachment #344627 - Attachment description: Patch for logging Flash plugin → Patch for logging Flash plugin's changes to nsPluginWindow->window->cgPort.context
(Assignee)

Updated

9 years ago
Attachment #344627 - Attachment description: Patch for logging Flash plugin's changes to nsPluginWindow->window->cgPort.context → Patch for logging Flash plugin's changes to nsPluginWindow->window->cgPort.context (not a fix)
The plot thickens.

I've discovered that the Flash plugin (when using the CoreGraphics
drawing model) actually changes the value of a member of the
nsPluginWindow structure that gets passed to it in calls to
NPP_SetWindow() -- the window->cgPort.context member, which is a
CGContextRef.

These changes tend to happen at the same time as the problems
described in my STRs from comment #32 and comment #62 ... on Intel
boxes.  But they don't happen at all on my PPC G4 running OS X 10.4.11
(where I never see the problems that I've been able to reproduce on my
Intel boxes).

To see the changes, run the tryserver build corresponding to this
patch, and observe what gets logged to the system console.

https://build.mozilla.org/tryserver-builds/2008-10-23_21:51-smichaud@pobox.com-bugzilla459530-testing-rev1/

Michelle and cliss, can you tell me what's going on?

And do this bug's problems go away if you stop the Flash plugin from
changing nsPluginWindow->window->cgPort.context?
(In reply to comment #71, comment #11 and comment #12)

I didn't realize I needed to download your testcase and run it locally
-- so I wasn't able to reproduce any problems with it.

But when I do run it locally (and permanently grant it
UniversalXPConnect privileges at the prompt), I now *can* see the
problem -- though only on Intel boxes.  I also see that my rev2 patch
doesn't fix it.  And my testing build (from comment #74) shows a
constant stream of changes (by the Flash plugin) to
nsPluginWindow->window->cgPort.context while it's running.
(In reply to comment #69)

> With the new build I am still able to reproduce using the video in
> comment 13 with Tab Sidebar.

I can confirm that my rev2 patch doesn't fix this problem (when I load
that video into a tab and turn on Tab Sidebar).  And (while it's
running in Tab Sidebar) I also (using my testing build from comment
#74) see a constant stream of changes to
nsPluginWindow->window->cgPort.context.

But this is only on Intel boxes.  I don't see any problems on my PPC
G4 running OS X 10.4.11.
(Following up comment #74)

I was wrong :-(

The Flash plugin *isn't* changing the value of
nsPluginWindow->window->cgPort.context.  Instead it's calls to
childView->GetNativeData(NS_NATIVE_PLUGIN_PORT_CG) that do this --
they always return a pointer to the same internal (private) object,
but may change the values in that object.  This object (the pointer to
it) is the "window" in nsPluginWindow->window->cgPort.context.

So this *isn't* a Flash bug, and *is* a Firefox box.

Sorry for the confusion.  I hope to have a new patch before too long.
I'm still puzzled why this problem doesn't happen on my PPC box.  It may be some kind of obscure endian issue.
Created attachment 344695 [details] [diff] [review]
Fix rev3 (another new design)

This is it, finally ... at least I hope so :-)

This patch fixes my STRs from comment #19, comment #32 and comment
#62.  It also fixes the problems running the video from comment #13 in
Tab Sidebar, and those seen running kinetik's testcase locally
(attachment 343159 [details], as described in comment #75).

The only remaining problem is probably insoluble:

When you load multiple dynamic Flash plugins into multiple tabs and
view them using Tab Sidebar, those in pages that aren't currently
"active" don't update.  I believe this is because only plugins in
foreground tabs get sent idle events (aka null events) -- which (if
I'm right) is as it should be.

A tryserver build will follow shortly.
Attachment #344532 - Attachment is obsolete: true
Attachment #344695 - Flags: review?(kinetik)
(Assignee)

Updated

9 years ago
Attachment #344695 - Flags: review?(joshmoz)
Here's a tryserver build made with my rev3 patch:

https://build.mozilla.org/tryserver-builds/2008-10-24_15:52-smichaud@pobox.com-bugzilla459530-rev3/smichaud@pobox.com-bugzilla459530-rev3-firefox-try-mac.dmg
+        window->window->cgPort.context = cgContext;
+        mInstanceOwner->SetPluginPortChanged(PR_TRUE);
+      }
+      *pluginPortCopy = *window->window;

+#ifndef NP_NO_QUICKDRAW
+  if (drawingModel == NPDrawingModelQuickDraw) {
+    if (mPluginWindow->window->qdPort.port != mPluginPortCopy.qdPort.port)
+      mPluginPortChanged = PR_TRUE;
+  } else if (drawingModel == NPDrawingModelCoreGraphics)
+#endif
+  {
+    if ((mPluginWindow->window->cgPort.context !=
+         mPluginPortCopy.cgPort.context) ||
+        (mPluginWindow->window->cgPort.window !=
+         mPluginPortCopy.cgPort.window))
+      mPluginPortChanged = PR_TRUE;
+  }
+  mPluginPortCopy = *mPluginWindow->window;

To avoid ambiguities in how the compiler might interpret lines that
copy an entire nsPluginPort struct (which is a union), I'll replace
these chunks in my next revision with the following:

+        window->window->cgPort.context = cgContext;
+        pluginPortCopy->cgPort.context = cgContext;
+        mInstanceOwner->SetPluginPortChanged(PR_TRUE);
+      }

+#ifndef NP_NO_QUICKDRAW
+  if (drawingModel == NPDrawingModelQuickDraw) {
+    if (mPluginWindow->window->qdPort.port != mPluginPortCopy.qdPort.port) {
+      mPluginPortCopy.qdPort.port = mPluginWindow->window->qdPort.port;
+      mPluginPortChanged = PR_TRUE;
+    }
+  } else if (drawingModel == NPDrawingModelCoreGraphics)
+#endif
+  {
+    if ((mPluginWindow->window->cgPort.context !=
+         mPluginPortCopy.cgPort.context) ||
+        (mPluginWindow->window->cgPort.window !=
+         mPluginPortCopy.cgPort.window)) {
+      mPluginPortCopy.cgPort.context = mPluginWindow->window->cgPort.context;
+      mPluginPortCopy.cgPort.window = mPluginWindow->window->cgPort.window;
+      mPluginPortChanged = PR_TRUE;
+    }
+  }
(Reporter)

Comment 82

9 years ago
(In reply to comment #80)
> Here's a tryserver build made with my rev3 patch:
> 
> https://build.mozilla.org/tryserver-builds/2008-10-24_15:52-smichaud@pobox.com-bugzilla459530-rev3/smichaud@pobox.com-bugzilla459530-rev3-firefox-try-mac.dmg

Just tested this and it looks perfect. Couldn't reproduce the issue any longer in any of my tests.
(In reply to comment #82)
> 
> Just tested this and it looks perfect. Couldn't reproduce the issue any longer
> in any of my tests.

I concur... the tryserver build seems to have fixed these drawWindow problems i saw earlier also.  

As expected, bug 461266 still exists with video.yahoo.com, but we'll track that issue over in that bug.

Nice work, smichaud.

Updated

9 years ago
Duplicate of this bug: 461705
Duplicate of this bug: 461791
Comment on attachment 344695 [details] [diff] [review]
Fix rev3 (another new design)

Looks great, thanks Steven!

With respect to comment #81, as far as I can tell from the standard, union-to-union assignments are legal and will do the right thing, but it may be true that the intention of the code is clearer with the proposed change.
Attachment #344695 - Flags: review?(kinetik) → review+
(Assignee)

Updated

9 years ago
Attachment #344627 - Attachment description: Patch for logging Flash plugin's changes to nsPluginWindow->window->cgPort.context (not a fix) → Patch for logging changes to nsPluginWindow->window->cgPort.context (not a fix)
Duplicate of this bug: 462075

Comment 88

9 years ago
I have been running the build from comment #80 for a day now, it appears to fix the flipped flash ghost content.

Thanks Steven!

Updated

9 years ago
Attachment #344695 - Flags: review?(joshmoz) → review+
(Assignee)

Updated

9 years ago
Attachment #344695 - Flags: superreview?(roc)
Duplicate of this bug: 462226
Comment on attachment 344695 [details] [diff] [review]
Fix rev3 (another new design)

+  void SetPluginPortChanged(PRBool aState) { mPluginPortChanged = aState; }
+  nsPluginPort* GetPluginPortCopy() { return &mPluginPortCopy; }
+  nsPluginPort* SetPluginPortAndDetectChange();
+  void BeginCGPaint();
+  void EndCGPaint();

Please add comments to document these.
Attachment #344695 - Flags: superreview?(roc) → superreview+

Updated

9 years ago
Duplicate of this bug: 462158
Landed on mozilla-central (changeset 9b4084a61cd9).
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Created attachment 345521 [details] [diff] [review]
What I landed

This is	what I actually landed.

I incorporated the changes from comment #81, added the comments roc
requested in comment #90, and got rid of the "doesn't happen in
current code" comments on Josh's recommendation.
Depends on: 462397
Hardware: PC → All
Target Milestone: --- → mozilla1.9.1b2
No longer depends on: 462397
verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081031 Minefield/3.1b2pre.
Status: RESOLVED → VERIFIED
(Reporter)

Updated

9 years ago
Duplicate of this bug: 464224

Updated

9 years ago
Duplicate of this bug: 465441
Keywords: fixed1.9.1
Keywords: fixed1.9.1 → verified1.9.1
You need to log in before you can comment on or make changes to this bug.