Closed Bug 883727 Opened 7 years ago Closed 7 years ago
Fix -Wsometimes-uninitialized warning in xpwidgets/Puppet
This is the only warning I see in widget/xpwidget code when building Firefox for OSX. This warning is a regression from bug 846881. widget/xpwidgets/PuppetWidget.cpp:126:9 [-Wsometimes-uninitialized] variable 'chromeSeqno' is used uninitialized whenever 'if' condition is false
Attachment #763390 - Flags: review?(bzbarsky)
Comment on attachment 763390 [details] [diff] [review] fix-xpwidget-warning.patch If we ever had !mTabChild here, this would leave the members uninitialized... How about we MOZ_ASSERT(mTabChild) up front in this method and remove the null-check instead?
Attachment #763390 - Flags: review?(bzbarsky) → review-
Patch v2 replaces `if (mTabChild)` with `MOZ_ASSERT(mTabChild)`. Try run did not hit an assertion failure. https://tbpl.mozilla.org/?tree=Try&rev=a8179a2f223d
Do we have debug tests on try that exercise this code?
(In reply to Boris Zbarsky [:bz] from comment #3) > Do we have debug tests on try that exercise this code? Good question. I don't know if we have tests for the puppet IME code. I added some MOZ_CRASH() checks to PuppetWidget::InitIMEState(). When running the try tests, mTabChild is always non-null (as expected), but mNeedIMEStateInit is never true. So my patch replaces `if (mTabChild)` with `MOZ_ASSERT(mTabChild)` without changing the control flow.
Comment on attachment 798153 [details] [diff] [review] fix-xpwidget-warning-v2.patch Ah, OK. MOZ_CRASH gives me the answer I wanted; I was worried about us not running any debug tests on try that exercise this code. r=me
Attachment #798153 - Flags: review?(bzbarsky) → review+
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.