Last Comment Bug 773462 - OMTC: Fix calls to ImageContainer::SetCurrentImage in layer transactions
: OMTC: Fix calls to ImageContainer::SetCurrentImage in layer transactions
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics: Layers (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla17
Assigned To: Nicolas Silva [:nical]
:
: Milan Sreckovic [:milan]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-12 14:58 PDT by Nicolas Silva [:nical]
Modified: 2012-07-28 18:35 PDT (History)
4 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Replaces calls to ImageContainer's SetCurrentImage with calls to SetCurrentImageInTransaction used within a layers transaction. (7.06 KB, patch)
2012-07-26 11:15 PDT, Nicolas Silva [:nical]
roc: review+
Details | Diff | Splinter Review

Description Nicolas Silva [:nical] 2012-07-12 14:58:26 PDT
With bug 598868, ImageContainer got a new and shiny method SetCurrentImageInTransaction better suited to cases where the current image is set on the main thread within a layer transaction.

We need to replace the concerned calls to SetCurrentImage by calls to SetCurrentImageInTransaction.
Comment 1 Nicolas Silva [:nical] 2012-07-25 13:51:44 PDT
Here is a list of the potentail caller of SetCurrentImage that might be in layer transactions:

layout/base/FrameLayerBuilder.cpp
2952:    container->SetCurrentImage(image);

image/src/RasterImage.cpp
931:  mImageContainer->SetCurrentImage(image);

dom/plugins/base/nsPluginInstanceOwner.cpp
3703:      container->SetCurrentImage(nsnull);

dom/plugins/ipc/PluginInstanceParent.cpp
758:        container->SetCurrentImage(ioImage);
773:    container->SetCurrentImage(pluginImage);

I am confident the first two happen at least some times during transactions.

roc, you know these parts of the code way better than me, do you know if some of these should be changed for SetCurrentImageInTransaction?
Comment 2 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-07-25 16:10:41 PDT
(In reply to Nicolas Silva [:nical] from comment #1)
> layout/base/FrameLayerBuilder.cpp
> 2952:    container->SetCurrentImage(image);
> 
> image/src/RasterImage.cpp
> 931:  mImageContainer->SetCurrentImage(image);
> 
> dom/plugins/base/nsPluginInstanceOwner.cpp
> 3703:      container->SetCurrentImage(nsnull);

There are actually several SetCurrentImage calls in this file.

> roc, you know these parts of the code way better than me, do you know if
> some of these should be changed for SetCurrentImageInTransaction?

I think they all need to be SetCurrentImageInTransaction.
Comment 3 Nicolas Silva [:nical] 2012-07-26 11:15:13 PDT
Created attachment 646220 [details] [diff] [review]
Replaces calls to ImageContainer's SetCurrentImage with calls to SetCurrentImageInTransaction used within a layers transaction.
Comment 4 Ryan VanderMeulen [:RyanVM] 2012-07-27 14:55:00 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/2fd48f64273e
Comment 5 Ryan VanderMeulen [:RyanVM] 2012-07-28 18:35:53 PDT
https://hg.mozilla.org/mozilla-central/rev/2fd48f64273e

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