Closed
Bug 746132
Opened 12 years ago
Closed 12 years ago
Screenshot buffers won't be freed if tab not found
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 14
People
(Reporter: blassey, Assigned: blassey)
Details
Attachments
(1 file, 1 obsolete file)
1.46 KB,
patch
|
kats
:
review+
lsblakk
:
approval-mozilla-central+
|
Details | Diff | Splinter Review |
No description provided.
Attachment #615695 -
Flags: review?(bugmail.mozilla)
Comment 1•12 years ago
|
||
Comment on attachment 615695 [details] [diff] [review] patch Review of attachment 615695 [details] [diff] [review]: ----------------------------------------------------------------- Why do we call freeDirectBuffer from java rather than just doing it in the c++ code? Is is to reduce memory pressure before calling processThumbnail? Also, it might be worthwhile wrapping that entire function in a try/finally and doing the freeDirectBuffer in the finally. That would also make sure to free the buffer if say Bitmap.createBitmap threw an exception, or if future changes to the code added another early-exit path.
Attachment #615695 -
Flags: review?(bugmail.mozilla) → review+
Assignee | ||
Comment 2•12 years ago
|
||
(In reply to Kartikaya Gupta (:kats) from comment #1) > Comment on attachment 615695 [details] [diff] [review] > patch > > Review of attachment 615695 [details] [diff] [review]: > ----------------------------------------------------------------- > > Why do we call freeDirectBuffer from java rather than just doing it in the > c++ code? Is is to reduce memory pressure before calling processThumbnail? this is not freed from the C++ code because it gets processed asynchronously in a runnable (i.e. after the free would happen). > > Also, it might be worthwhile wrapping that entire function in a try/finally > and doing the freeDirectBuffer in the finally. That would also make sure to > free the buffer if say Bitmap.createBitmap threw an exception, or if future > changes to the code added another early-exit path. This is probably best
Assignee | ||
Comment 3•12 years ago
|
||
Assignee: nobody → blassey.bugs
Attachment #615695 -
Attachment is obsolete: true
Attachment #617095 -
Flags: review?(bugmail.mozilla)
Updated•12 years ago
|
Attachment #617095 -
Flags: review?(bugmail.mozilla) → review+
Assignee | ||
Comment 4•12 years ago
|
||
Comment on attachment 617095 [details] [diff] [review] patch [Approval Request Comment] Please see https://wiki.mozilla.org/Tree_Rules for information on mozilla-central landings that do not require approval. Possible risk to Fennec Native 14 (if any): This prevents leaks in fennec native. Should be zero risk
Attachment #617095 -
Flags: approval-mozilla-central?
Updated•12 years ago
|
Attachment #617095 -
Flags: approval-mozilla-central? → approval-mozilla-central+
Comment 5•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/344a48cc9998
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 14
Comment 6•12 years ago
|
||
Regression :( Robocop Checkerboarding Benchmark increase 21.8% on Android 2.2 (Native) Mozilla-Inbound ------------------------------------------------------------------------------------------------------ Previous: avg 0.813 stddev 0.006 of 30 runs up to revision d2b34e4338bb New : avg 0.990 stddev 0.000 of 5 runs since revision 344a48cc9998 Change : +0.177 (21.8% / z=27.800) Graph : http://mzl.la/I4Nh5G Changeset range: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=d2b34e4338bb&tochange=344a48cc9998 Changesets: * http://hg.mozilla.org/integration/mozilla-inbound/rev/190fc7cd65c6 : George Wright <gwright@mozilla.com> - Bug 747274 - Add a pref (default to true on Android) to forcibly use nearest pixel filtering for background drawing. r=jrmuizel,ajuma a=blassey : http://bugzilla.mozilla.org/show_bug.cgi?id=747274 * http://hg.mozilla.org/integration/mozilla-inbound/rev/9b2440c92909 : Brad Lassey <blassey@mozilla.com> - bug 746139 - local JNI frame won't be popped if init fails r=dougt a=lsblakk : http://bugzilla.mozilla.org/show_bug.cgi?id=746139 * http://hg.mozilla.org/integration/mozilla-inbound/rev/344a48cc9998 : Brad Lassey <blassey@mozilla.com> - bug 746132 - Screenshot buffers won't be freed if tab not found r=kats a=lsblakk : http://bugzilla.mozilla.org/show_bug.cgi?id=746132
Comment 7•12 years ago
|
||
(In reply to Ehsan Akhgari [:ehsan] from comment #6) > Regression :( Robocop Checkerboarding Benchmark increase 21.8% on Android > 2.2 (Native) Mozilla-Inbound > ----------------------------------------------------------------------------- > ------------------------- > Previous: avg 0.813 stddev 0.006 of 30 runs up to revision d2b34e4338bb > New : avg 0.990 stddev 0.000 of 5 runs since revision 344a48cc9998 > Change : +0.177 (21.8% / z=27.800) > Graph : http://mzl.la/I4Nh5G Talos might not know it, but "bigger is better" for the checkerboarding tests.
Comment 8•12 years ago
|
||
OK, filed bug 748174. Thanks!
Updated•10 years ago
|
tracking-fennec: ? → ---
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•