Closed
Bug 703601
Opened 13 years ago
Closed 12 years ago
Flash content continues to play regardless of set_screen_state 0
Categories
(Firefox for Android Graveyard :: General, defect, P2)
Tracking
(firefox13 verified, fennec11+)
VERIFIED
FIXED
Firefox 13
People
(Reporter: aaronmt, Assigned: snorp)
References
Details
(Whiteboard: [battery-killer])
Attachments
(1 file, 2 obsolete files)
31.54 KB,
patch
|
blassey
:
review+
|
Details | Diff | Splinter Review |
Phone was idle on my desk with the display off, yet heard a sound-clip faintly playing. Awoke my phone and saw that the CNN Video page I was testing earlier was continuing to loop and play videos from their stream. ER: Once my phone's display is off (i.e., set_screen_state 0) and phone is idle, pause or kill all Flash content (please do not drain device battery). Resume at will once set_screen_state is set to 1, and when Fennec is in the foreground. -- Samsung Nexus S (Android 2.3.6) Flash 11.1 20111118040220 http://hg.mozilla.org/projects/birch/rev/9999a423d8ab
Reporter | ||
Updated•13 years ago
|
Whiteboard: [battery-killer]
Updated•13 years ago
|
Assignee: nobody → snorp
Priority: -- → P4
Comment 1•13 years ago
|
||
bumping to a P2 on the assumption that this is a life cycle event we need to send
Priority: P4 → P2
Updated•13 years ago
|
tracking-fennec: --- → 11+
Assignee | ||
Comment 2•12 years ago
|
||
Attachment #597061 -
Flags: review?(blassey.bugs)
Comment 3•12 years ago
|
||
Comment on attachment 597061 [details] [diff] [review] Fix a bunch of lifecycle issues with Flash on Android Review of attachment 597061 [details] [diff] [review]: ----------------------------------------------------------------- r- for mapping pause and resume onto onPause() and onResume() ::: mobile/android/base/GeckoApp.java @@ +2731,5 @@ > + status = false; > + > + setScreenStatus(status); > + } > +} pretty much all of these changes to GeckoApp.java seem unnesesary if you just hook up to onPause() and onResume() for the application-foreground/background notifcations. ::: mobile/android/base/GeckoAppShell.java @@ +1988,5 @@ > return decodeBase64(s.getBytes(), flags); > } > + > + public static void notifyScreenChanged(boolean on) { > + sendEventToGecko(new GeckoEvent(on ? GeckoEvent.SCREEN_ON : GeckoEvent.SCREEN_OFF)); this is bit-rotted now, the new pattern is: GeckoEvent.createScreenStateEvent(on); ::: mobile/android/base/GeckoEvent.java @@ +87,5 @@ > public static final int VISITED = 21; > public static final int NETWORK_CHANGED = 22; > public static final int PROXIMITY_EVENT = 23; > + public static final int SCREEN_OFF = 24; > + public static final int SCREEN_ON = 25; please just have SCREEN_STATE and use the flags on the event to indicate on or off ::: widget/android/nsAppShell.cpp @@ +485,5 @@ > > + case AndroidGeckoEvent::SCREEN_OFF: { > + nsCOMPtr<nsIObserverService> obsServ = > + mozilla::services::GetObserverService(); > + obsServ->NotifyObservers(nsnull, "application-background", nsnull); pretty sure you want to be sending this from here: https://mxr.mozilla.org/mozilla-central/source/widget/android/nsAppShell.cpp#397 @@ +492,5 @@ > + > + case AndroidGeckoEvent::SCREEN_ON: { > + nsCOMPtr<nsIObserverService> obsServ = > + mozilla::services::GetObserverService(); > + obsServ->NotifyObservers(nsnull, "application-foreground", nsnull); and this from here (sorta): https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoApp.java#2076 You'll need to send an ACTIVITY_RESUMING event to gecko so its on the right thread
Attachment #597061 -
Flags: review?(blassey.bugs) → review-
Assignee | ||
Comment 4•12 years ago
|
||
(In reply to Brad Lassey [:blassey] from comment #3) > Comment on attachment 597061 [details] [diff] [review] > Fix a bunch of lifecycle issues with Flash on Android > > Review of attachment 597061 [details] [diff] [review]: > ----------------------------------------------------------------- > > r- for mapping pause and resume onto onPause() and onResume() > > ::: mobile/android/base/GeckoApp.java > @@ +2731,5 @@ > > + status = false; > > + > > + setScreenStatus(status); > > + } > > +} > > pretty much all of these changes to GeckoApp.java seem unnesesary if you > just hook up to onPause() and onResume() for the > application-foreground/background notifcations. That's what I tried at first, too, but it's problematic. When you open the tab tray, for instance, it pauses the main activity then resumes when you close it. > > ::: mobile/android/base/GeckoAppShell.java > @@ +1988,5 @@ > > return decodeBase64(s.getBytes(), flags); > > } > > + > > + public static void notifyScreenChanged(boolean on) { > > + sendEventToGecko(new GeckoEvent(on ? GeckoEvent.SCREEN_ON : GeckoEvent.SCREEN_OFF)); > > this is bit-rotted now, the new pattern is: > GeckoEvent.createScreenStateEvent(on); Fixed > > ::: mobile/android/base/GeckoEvent.java > @@ +87,5 @@ > > public static final int VISITED = 21; > > public static final int NETWORK_CHANGED = 22; > > public static final int PROXIMITY_EVENT = 23; > > + public static final int SCREEN_OFF = 24; > > + public static final int SCREEN_ON = 25; > > please just have SCREEN_STATE and use the flags on the event to indicate on > or off Fixed > > ::: widget/android/nsAppShell.cpp > @@ +485,5 @@ > > > > + case AndroidGeckoEvent::SCREEN_OFF: { > > + nsCOMPtr<nsIObserverService> obsServ = > > + mozilla::services::GetObserverService(); > > + obsServ->NotifyObservers(nsnull, "application-background", nsnull); > > pretty sure you want to be sending this from here: > https://mxr.mozilla.org/mozilla-central/source/widget/android/nsAppShell. > cpp#397 > > @@ +492,5 @@ > > + > > + case AndroidGeckoEvent::SCREEN_ON: { > > + nsCOMPtr<nsIObserverService> obsServ = > > + mozilla::services::GetObserverService(); > > + obsServ->NotifyObservers(nsnull, "application-foreground", nsnull); > > and this from here (sorta): > https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoApp. > java#2076 > > You'll need to send an ACTIVITY_RESUMING event to gecko so its on the right > thread
Assignee | ||
Comment 5•12 years ago
|
||
Attachment #597134 -
Flags: review?(blassey.bugs)
Assignee | ||
Updated•12 years ago
|
Attachment #597061 -
Attachment is obsolete: true
Comment 6•12 years ago
|
||
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #4) > (In reply to Brad Lassey [:blassey] from comment #3) > > Comment on attachment 597061 [details] [diff] [review] > > Fix a bunch of lifecycle issues with Flash on Android > > > > Review of attachment 597061 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > r- for mapping pause and resume onto onPause() and onResume() > > > > ::: mobile/android/base/GeckoApp.java > > @@ +2731,5 @@ > > > + status = false; > > > + > > > + setScreenStatus(status); > > > + } > > > +} > > > > pretty much all of these changes to GeckoApp.java seem unnesesary if you > > just hook up to onPause() and onResume() for the > > application-foreground/background notifcations. > > That's what I tried at first, too, but it's problematic. When you open the > tab tray, for instance, it pauses the main activity then resumes when you > close it. Isn't that what we want? If not that's something that can be solved with checking mOwnActivityDepth (or in this case including it in the PAUSING/RESUMING event). http://mxr.mozilla.org/mozilla-central/ident?i=mOwnActivityDepth
Comment 7•12 years ago
|
||
Comment on attachment 597134 [details] [diff] [review] Fix a bunch of lifecycle issues with Flash on Android Review of attachment 597134 [details] [diff] [review]: ----------------------------------------------------------------- as discussed on IRC, this should map on onPause/onResult
Attachment #597134 -
Flags: review?(blassey.bugs) → review-
Assignee | ||
Comment 8•12 years ago
|
||
Attachment #597398 -
Flags: review?(blassey.bugs)
Assignee | ||
Updated•12 years ago
|
Attachment #597134 -
Attachment is obsolete: true
Updated•12 years ago
|
Attachment #597398 -
Flags: review?(blassey.bugs) → review+
Assignee | ||
Comment 9•12 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/7c004a416b2a
Comment 10•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7c004a416b2a
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 13
Comment 11•12 years ago
|
||
Verified fixed on: Firefox 13.0a1 (2012-03-06) 20120306031101 http://hg.mozilla.org/mozilla-central/rev/7d0d1108a14e -- Device: HTC Desire OS: Android 2.2
Status: RESOLVED → VERIFIED
status-firefox13:
--- → verified
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
•