Last Comment Bug 775848 - BasicShadowLayerManager::mRepeatTransaction is uninitialized
: BasicShadowLayerManager::mRepeatTransaction is uninitialized
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: All All
-- normal (vote)
: mozilla17
Assigned To: Karl Tomlinson (:karlt)
: Milan Sreckovic [:milan]
: 778071 (view as bug list)
Depends on:
Blocks: 771219
  Show dependency treegraph
Reported: 2012-07-19 22:18 PDT by Daniel Holbert [:dholbert]
Modified: 2012-08-09 18:27 PDT (History)
4 users (show)
karlt: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Initialize (984 bytes, patch)
2012-07-29 18:13 PDT, Karl Tomlinson (:karlt)
b56girard: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description User image Daniel Holbert [:dholbert] 2012-07-19 22:18:58 PDT
This changeset for Bug 771219...
added a boolean member-var BasicShadowLayerManager::mRepeatTransaction, which isn't necessarily ever set, though it is read, during "BasicShadowLayerManager::EndTransaction().

We should probably be initializing it, to false I imagine.

I ran across this due to the following valgrind warning, near startup:
==19324== Conditional jump or move depends on uninitialised value(s)
==19324==    at 0x99B01F3: mozilla::layers::BasicShadowLayerManager::EndTransaction(void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegi
on const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) (BasicLayerManager.cpp:1001)
==19324==    by 0x8291D2A: nsDisplayList::PaintForFrame(nsDisplayListBuilder*, nsRenderingContext*, nsIFrame*, unsigned int) const (nsDisplayList.cpp:651)
==19324==    by 0x82915D2: nsDisplayList::PaintRoot(nsDisplayListBuilder*, nsRenderingContext*, unsigned int) const (nsDisplayList.cpp:549)
==19324==    by 0x82C85B7: nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion const&, unsigned int, unsigned int) (nsLayoutUtils.cpp:1786)
==19324==    by 0x82F4FB3: PresShell::Paint(nsIView*, nsIWidget*, nsRegion const&, nsIntRegion const&, bool) (nsPresShell.cpp:5278)
==19324==    by 0x8A692C6: nsViewManager::Refresh(nsView*, nsIWidget*, nsIntRegion const&, bool) (nsViewManager.cpp:339)
==19324==    by 0x8A6A77E: nsViewManager::DispatchEvent(nsGUIEvent*, nsIView*, nsEventStatus*) (nsViewManager.cpp:768)
==19324==    by 0x8A64DBC: HandleEvent(nsGUIEvent*) (nsView.cpp:127)
==19324==    by 0x956F934: nsWindow::DispatchEvent(nsGUIEvent*, nsEventStatus&) (nsWindow.cpp:474)
==19324==    by 0x95739A8: nsWindow::OnExposeEvent(_GdkEventExpose*) (nsWindow.cpp:2213)
==19324==    by 0x957B622: expose_event_cb(_GtkWidget*, _GdkEventExpose*) (nsWindow.cpp:5091)
==19324==    by 0xE1F3DD7: _gtk_marshal_BOOLEAN__BOXED (gtkmarshalers.c:86)
Comment 1 User image Karl Tomlinson (:karlt) 2012-07-29 18:13:20 PDT
Created attachment 647032 [details] [diff] [review]
Comment 2 User image Karl Tomlinson (:karlt) 2012-07-29 18:17:47 PDT
Not sure how much repeating the transaction unnecessarily could affect performance.
It should happen only once per layer manager.
But this should be a trivial fix for Aurora.
Comment 3 User image Benoit Girard (:BenWa) 2012-07-29 19:35:39 PDT
Comment on attachment 647032 [details] [diff] [review]

Wow sorry about that. I think the repeat transaction wont do anything expensive (draw/upload) since it will have completed before. That said I rather just patch it and not take the risk. Let's uplift this to aurora.
Comment 4 User image Ali Juma [:ajuma] 2012-07-30 06:26:46 PDT
*** Bug 778071 has been marked as a duplicate of this bug. ***
Comment 6 User image Ed Morley [:emorley] 2012-08-01 02:53:17 PDT
Comment 7 User image Karl Tomlinson (:karlt) 2012-08-01 17:20:01 PDT
Comment on attachment 647032 [details] [diff] [review]

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 771219
User impact if declined: possible performance impact in rare situations, valgrind warnings.
Testing completed (on m-c, etc.): on m-c.
Risk to taking this patch (and alternatives if risky): none.
String or UUID changes made by this patch: none.
Comment 8 User image Lukas Blakk [:lsblakk] use ?needinfo 2012-08-03 12:06:54 PDT
Comment on attachment 647032 [details] [diff] [review]

low risk, approving.
Comment 9 User image Karl Tomlinson (:karlt) 2012-08-09 18:27:41 PDT

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