Last Comment Bug 758432 - Fix SetScaleToSize call for plugins
: Fix SetScaleToSize call for plugins
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: unspecified
: x86_64 Windows 7
: -- normal (vote)
: mozilla15
Assigned To: Bas Schouten (:bas.schouten)
:
Mentors:
Depends on: 794836
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-24 16:21 PDT by Bas Schouten (:bas.schouten)
Modified: 2012-10-02 09:37 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix SetScaleToSize calling (1.91 KB, patch)
2012-05-24 16:21 PDT, Bas Schouten (:bas.schouten)
roc: review+
Details | Diff | Splinter Review

Description Bas Schouten (:bas.schouten) 2012-05-24 16:21:47 PDT
Created attachment 627006 [details] [diff] [review]
Fix SetScaleToSize calling

The current code that calls SetScaleToSize calls !UseAsyncRendering to not call this on OS X for some cases. However that is incorrect, UseAsyncRendering actually pretty much returns true everywhere at the moment and thus SetScaleToSize never gets called. And for NPAPI Async drawing we don't get the right scaling when asynchronous surface changes are made by the plugin.

I don't know this code very well and it's hard to make the right decision, we cannot just always call SetScaleToSize as it's not strictly correct for the CoreAnimation drawing model and it's unsupported for OpenGL layer managers.

The best thing to do for now (as we'll want to fix support for NPAPI async drawing), seems to me to just call SetScaleToSize on windows. And use the container size for determining the size only when using the CoreAnimation drawing model.

I will make sure a follow-up bug is filed to find a more proper (cross-platform) way to address the issue before we start using the Async bitmap model on other platforms.
Comment 1 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-05-24 17:37:19 PDT
Comment on attachment 627006 [details] [diff] [review]
Fix SetScaleToSize calling

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

::: layout/generic/nsObjectFrame.cpp
@@ +1522,4 @@
>      size = container->GetCurrentSize();
> +  } else
> +#endif
> +  size = gfxIntSize(window->width, window->height);

{} around this
Comment 2 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-05-24 17:38:33 PDT
Seems to me we should just add support for SetScaleToSize to all layer managers and call SetScaleToSize always. Why wouldn't that be OK for the CoreAnimation drawing model?
Comment 3 Bas Schouten (:bas.schouten) 2012-05-24 19:06:33 PDT
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #2)
> Seems to me we should just add support for SetScaleToSize to all layer
> managers and call SetScaleToSize always. Why wouldn't that be OK for the
> CoreAnimation drawing model?

From what I understand the scaling behavior of CoreAnimation isn't well defined. (i.e. it would at least change behavior from what it is now)
Comment 4 Bas Schouten (:bas.schouten) 2012-05-29 22:16:28 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/d4470d16d227
Comment 5 Ed Morley [:emorley] 2012-05-30 07:32:56 PDT
https://hg.mozilla.org/mozilla-central/rev/d4470d16d227
Comment 6 Benoit Girard (:BenWa) 2012-05-30 07:41:09 PDT
If by 'SetScaleToSize' you mean take the current frame and stretch it to fix the container I don't see anything wrong with that.

Note You need to log in before you can comment on or make changes to this bug.