Closed
Bug 1107139
Opened 9 years ago
Closed 9 years ago
[Window Management] Active app window appears blue/grey in card view
Categories
(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)
Tracking
(blocking-b2g:2.2+, b2g-v2.1 unaffected, b2g-v2.2 verified, b2g-master verified)
Tracking | Status | |
---|---|---|
b2g-v2.1 | --- | unaffected |
b2g-v2.2 | --- | verified |
b2g-master | --- | verified |
People
(Reporter: KTucker, Assigned: sfoster)
References
Details
(Keywords: polish, regression, Whiteboard: [2.2-flame-reduced-run] [systemsfe])
Attachments
(13 files, 7 obsolete files)
12.54 KB,
image/png
|
Details | |
275.21 KB,
text/plain
|
Details | |
41.09 KB,
text/x-log
|
Details | |
7.85 MB,
text/plain
|
Details | |
4.51 KB,
text/plain
|
Details | |
62.26 KB,
text/plain
|
Details | |
7.37 MB,
video/3gpp
|
Details | |
1.81 MB,
text/plain
|
Details | |
46 bytes,
text/x-github-pull-request
|
kgrandon
:
review+
djabber
:
ui-review+
|
Details | Review |
46 bytes,
text/x-github-pull-request
|
bajaj
:
approval-gaia-v2.2+
|
Details | Review |
123.69 KB,
image/png
|
Details | |
28.25 KB,
image/png
|
Details | |
116.66 KB,
image/png
|
Details |
Description: Camera and Gallery appear gray and they missing icons in card view. Repro Steps: 1) Updated Flame to Build ID: 20141203040207 2) Open the camera or gallery app. 3) Press and hold the home button to enter card view. 4) Observe the card. Actual: The camera and gallery apps are gray and they are missing icons in card view. Expected: The camera and gallery apps appear correctly in card view. Environmental variables: Device: Flame 2.2 Master (319mb)(Kitkat Base)(Full Flash) BuildID: 20141203040207 Gaia: 725685831f5336cf007e36d9a812aad689604695 Gecko: 2c9781c3e9b5 Gonk: 48835395daa6a49b281db62c50805bd6ca24077e Version: 37.0a1 (2.2 Master) Firmware: V188-1 User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0 Notes: Repro frequency: 100% See attached: screenshot, logcat
Reporter | ||
Comment 1•9 years ago
|
||
Reporter | ||
Updated•9 years ago
|
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Reporter | ||
Comment 2•9 years ago
|
||
Comment 3•9 years ago
|
||
NI to Windows Management Owner to make a blocking call - Notes: This is a regression, but you can still tell what the cards are suppose to represent
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell) → needinfo?(gchang)
Reporter | ||
Comment 4•9 years ago
|
||
This is also occurring in other apps such as Dialer and Calendar but it didn't seem to happen as often as camera and gallery.
Comment 5•9 years ago
|
||
NI window management owner for this issue.
Flags: needinfo?(gchang) → needinfo?(hcheng)
Comment 6•9 years ago
|
||
I can reproduce this problem when I entered card view from homescreen and the card with wrong display is the most recent app. My steps: (the error occurs at step 3) 1. open camera (or gallery or dialer or calendar) 2. press home button and then back to homescreen 3. long press home button frome homescreen to enter card view and would get the camera card with wrong display. 4. If I did not enter camera app after step 3 and back to homescreen, the display would be correct after I enter card view again.
Flags: needinfo?(hcheng) → needinfo?(alive)
Updated•9 years ago
|
QA Contact: hcheng
Comment 7•9 years ago
|
||
Transfer NI
Flags: needinfo?(sfoster)
Flags: needinfo?(aus)
Flags: needinfo?(alive)
Assignee | ||
Comment 8•9 years ago
|
||
yeah something definitely regressed here, its taking much longer for the screenshots to show up. I suspect this is down in platform, but I'll do some investigation.
Assignee: nobody → sfoster
Flags: needinfo?(sfoster)
Flags: needinfo?(aus)
Assignee | ||
Comment 9•9 years ago
|
||
So I can't reproduce the icons part of this bug, they show up near instantly every time for me. However the screenshot itself is slow to show up - sometimes 5 or more seconds after the cards view displays. With some logging I can see that the request for the screenshot *and* the request success and assignment of the blob: url to the backgroundImage for each card is all taking place with the expected timing. However, we don't *see* that screenshot until many seconds later.
Comment 10•9 years ago
|
||
When I reproduce this problem, I got below message in logcat and only got it at 1st time. *message in logcat: W/GeckoConsole( 203): [JavaScript Warning: "Error in parsing value for 'visibility'. Declaration dropped." {file: "app://system.gaiamobile.org/index.html" line: 0 column: 12 source: "visibility: undefined"}] *step: 1. restart phone (flame-kk, build id: 20141211160202) 2. start gallery app 3. tap home button to homescreen 4. long press home button to card view and got gallery card with gray display
Flags: needinfo?(sfoster)
Assignee | ||
Comment 11•9 years ago
|
||
I've filed bug 1110932 for the visibility warning, I believe it is unrelated and due to having no initial value for favoriteButtonVisibility when the card template is rendered. I'll get that fixed to eliminate it as a suspect
Flags: needinfo?(sfoster)
Assignee | ||
Comment 12•9 years ago
|
||
I've not had success yet profiling this issue - my local build wouldnt start up and nothing useful I could spot in logcat. Probably I missed something or messed it up - will try again first thing next week (Please steal it though if you figure it out before me.) Marking this as a 2.2 blocker in the meantime.
blocking-b2g: --- → 2.2?
Assignee | ||
Comment 13•9 years ago
|
||
Aus and I dug into this a bit and here's what I see happening: STR: 1. Launch gallery app (arbitrary, any app should do). 2. Tap home to return to homescreen 3. Long tap home to bring up task manager At step 2 we actually take and store the screenshot in a _screenshotBlob property on the AppWindow instance. You can get this object from the console by e.g. appWindowManager.getApp('app://gallery.gaiamobile.org') The screenshot blob is stored in a _screenshotBlob property on the instance. The instance also an instanceID property which you can use to find its rendered representation in the DOM. If you inspect the DOM of the AppWindow representing your app, (it will match #windows .appWindow) you'll see a .appWindow > browser-container -> screenshot-overlay element. At step 2 it has visibility:hidden and no backgroundImage property. The task manager uses -moz-element to show us the .appWindow div. In step 3, the cardviewbeforeshow event handler calls _showScreenshotOverlay on the AppWindow instance, which toggles the visibility of this screenshot-overlay, and puts the screenshot blob in the style.backgroundImage. If you watch the DOM at this point, you can see this property show up on the element long before the screenshot is painted. Sometimes the lag is seconds, sometimes it seems to never get painted. Benoit: AFAICT all the system app/js and CSS is doing the right thing here. For some reason there is a variable, long period between assigning the backgroundImage on the appwindow and it getting painted in the -moz-element'd task manager div. Can you help or direct this to someone that can?
Flags: needinfo?(bgirard)
Summary: [Window Management] Camera and Gallery appear gray and they are missing icons in card view → [Window Management] Inactive app windows extremely slow to paint in card view
Comment 14•9 years ago
|
||
Why not start with getting a profile of the whole process? With a custom build use: $ ./profile.sh start -p b2g -t GeckoMain,Compositor -f stackwalk,js $ ./profile.sh start -p <Camera PID> -f stackwalk,js Type but don't submit $ ./profile.sh capture Run the animation and *immediately* after the slow execution period press return. If you submit the capture command with more than 1 second delay reboot the phone and retry the steps. Once collected attach profile_captured.sym. You can have a look yourself if you'd like: https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Profiling_with_the_Built-in_Profiler
Flags: needinfo?(bgirard)
Assignee | ||
Comment 15•9 years ago
|
||
(In reply to Benoit Girard (:BenWa) from comment #14) > Why not start with getting a profile of the whole process? Thanks for the details. I'm getting an error when I ./profile.sh capture: http://pastebin.mozilla.org/8054175 which I've had no luck troubleshooting. Not sure if this is a python env problem on my machine, or some other peculiarity, but the profile_captured.sym ends up empty.
Flags: needinfo?(bgirard)
Comment 16•9 years ago
|
||
That's bug 1111830. Make sure you're have that patch in your tree, it's on central now.
Flags: needinfo?(bgirard) → needinfo?(sfoster)
Assignee | ||
Comment 17•9 years ago
|
||
This capture should include the 2-3 seconds after tapping hold-home when the Gallery screenshot *should* have been displayed in the card view. A few seconds after the capture it finally showed up, I hope this is useful.
Flags: needinfo?(sfoster)
Assignee | ||
Comment 18•9 years ago
|
||
I don't really know how to interpret these profiles, but it looks to me like not much of anything is going on while we wait for that screenshot. Is it more likely that we're just not invalidating that -moz-element area for re-paint?
Flags: needinfo?(bgirard)
Updated•9 years ago
|
Whiteboard: [2.2-flame-reduced-run] → [2.2-flame-reduced-run] [systemsfe]
Comment 19•9 years ago
|
||
Let's try hard to fix this, but we won't block the release on it.
blocking-b2g: 2.2? → ---
Keywords: polish
Comment 20•9 years ago
|
||
The profile here is no good. It doesn't record any interesting activity so there isn't anything to interpret. Either the activity is happening in a different process/thread or it didn't happen over the last 4 seconds after you type ./profile.sh capture. If it's happening in a different process try profiling an additional process or two: ./profile.sh start -p b2g && sleep 1 && ./profile.sh start -p Gallery <interact> ./profile.sh capture (immediately after)
Flags: needinfo?(bgirard)
Assignee | ||
Comment 21•9 years ago
|
||
Ok this time I used ./profile.sh start -p b2g && sleep 1 && ./profile.sh start -p Gallery. I did the holdhome to show cardview from the homescreen, with just Gallery running in the background. I waited approx 30s before the card showed the gallery app, and hit capture immediately after. Looks like there are some spikes at the end of the profule that correspond to what I saw, but again not much of anything going on before then to explain the long 30s wait. Still, I hope this is useful. Let me know if I can do anything more to help.
Attachment #8538777 -
Attachment is obsolete: true
Flags: needinfo?(bgirard)
Comment 22•9 years ago
|
||
Do you have a file called profile_captured.sym? This should be the merged output of all processes. If you got the timing right you're executing is happening in another process/thread, possibly the b2g (system) process' main thread.
Flags: needinfo?(bgirard)
Assignee | ||
Comment 23•9 years ago
|
||
Scratch that last one. This one also includes the Homescreen
Attachment #8545417 -
Attachment is obsolete: true
Comment 24•9 years ago
|
||
We talked on IRC. All the profiles indicate that the CPU is idle. I don't think this is a CPU performance problem. I think we need to debug what requests are being made and if any threads are being blocked.
Assignee | ||
Comment 25•9 years ago
|
||
Heres the output from running top -t -m 10 -n 5 while waiting for that gallery card preview area to show up. I tried a few other scenarios and noticed that while all in-active apps are a little slow to show up (1-2s) only the fullscreen apps have this extreme problem. I'll keep digging.
Comment 26•9 years ago
|
||
Could it be we're hitting a -moz-element invalidation bug? I've noticed on the status bar that there used to be situations where when it was showing via -moz-element, it would stop animating (e.g. when it's connecting to a network), but certain operations would force it to update. Have you tried logging when the screenshots are actually returned/loaded to see that they're in line with when they get displayed?
Assignee | ||
Comment 27•9 years ago
|
||
(In reply to Chris Lord [:cwiiis] from comment #26) > Could it be we're hitting a -moz-element invalidation bug? That was my thought also, but I think BenWa thought not? > Have you tried logging when the screenshots are actually returned/loaded to > see that they're in line with when they get displayed? Yes I tried this - at least on the Gaia side. The browser_mixin's getScreenshot callback is called when you would expect, but it never paints, or paints many seconds later.
Assignee | ||
Comment 28•9 years ago
|
||
Does anyone else reproduce this with the severity I'm seeing? For me, when switching to the task manager from (e.g.) the homescreen, the screenshots for the other apps take *many* seconds to display. Sometimes 30s or more. This bug was last triaged as not-blocking, but unless my experience is unique, it seems like a blocker to me?
blocking-b2g: --- → 2.2?
Comment 31•9 years ago
|
||
Regression range is being investigated in bug 1086594
Comment 32•9 years ago
|
||
According to the comments in bug 1086594, this bug has existed in 2.0 and 2.1 as well, and because it is intermittent, getting regression range is difficult. Any actions that QA can take to speed things up?
Assignee | ||
Comment 33•9 years ago
|
||
The expexted sequence should be: 1. Open full screen app (e.g. Gallery) 2. Tap home to return to homescreen 3. Long-tap home to launch task-manager 4. Task manager shows "cards" with title/name above, screenshot and icon below. At step 4 the effect I was seeing was quite severe - the card rectangle with title and icon would be visible almost immediately, but the screenshot area was black for many seconds. Sometimes the screenshot would take as much as a minute to show up - if at all. Today I cant reproduce though - so maybe this was fixed? If QA cant reproduce the severe effect we can resolve as worksforme.
Flags: needinfo?(npark)
Comment 34•9 years ago
|
||
I can reproduce that with the today's nightly build. But problem with this bug is that it is quite intermittent, that sometimes it takes a number of effort to reproduce it once. On other days, it happens on the first attempt.
Flags: needinfo?(npark)
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(nhirata.bugzilla)
Comment 35•9 years ago
|
||
FYI, I run a image comparison test in jenkins for every nightly build, and in most cases this comes up as a failure: http://jenkins1.qa.scl3.mozilla.com/view/Graphics/job/flame-kk-319.mozilla-central.tinderbox.ui.graphics.smoke/ WARNING: 28328 pixels mismatched between <http://jenkins1.qa.scl3.mozilla.com/job/flame-kk-319.mozilla-central.tinderbox.ui.graphics.smoke/148/artifact/screenshots/test_cards_view_with_two_apps_flame_3+2015-02-12-09-02-07.png> and <http://jenkins1.qa.scl3.mozilla.com/job/flame-kk-319.mozilla-central.tinderbox.ui.graphics.smoke/148/artifact/reference_images/test_cards_view_with_two_apps_flame_3.png> WARNING: 71474 pixels mismatched between <http://jenkins1.qa.scl3.mozilla.com/job/flame-kk-319.mozilla-central.tinderbox.ui.graphics.smoke/148/artifact/screenshots/test_cards_view_with_two_apps_flame_4+2015-02-12-09-02-17.png> and <http://jenkins1.qa.scl3.mozilla.com/job/flame-kk-319.mozilla-central.tinderbox.ui.graphics.smoke/148/artifact/reference_images/test_cards_view_with_two_apps_flame_4.png> the picture in reference_images folder is being compared against the picture in screenshots folder every time when the test runs (Above link is accessible if you're inside the mozilla network)
http://people.mozilla.org/~bgirard/cleopatra/#report=c179d5ae492495dd435cc1702e48573a7b536b93&selection=0,1,6,7,50,51,56,87,88,92,94,95,96,51,56,99,100,100,101,92,102,95,104,105,96,110 Sam's profiling in cleopatra.
Flags: needinfo?(nhirata.bugzilla)
I think it might have something to do with the time it takes a screenshot and what's in memory. I get a larger percent hit of seeing gray when I go from homescreen to camera and long tap while the camera app is loading versus camera already launched and the long tap. Would it be worth capturing a profile for the camera app in that condition? homescreen -> camera => long tap?
I found that there's actually logging for screen shots in the debug. I/GeckoDump( 4586): [system] [AppWindow][Video][AppWindow_6][-5019.321] getScreenshot timeout! I/GeckoDump( 4586): [system] [AppWindow][Settings][AppWindow_5][-5019.290] close with immediate Also to note, I think there's a greater chance that this might happen more on freshly flashed devices.
Assignee | ||
Comment 39•9 years ago
|
||
(In reply to Sam Foster [:sfoster] from comment #33) > 1. Open full screen app (e.g. Gallery) > 2. Tap home to return to homescreen > 3. Long-tap home to launch task-manager > 4. Task manager shows "cards" with title/name above, screenshot and icon > below. > > At step 4 the effect I was seeing was quite severe - the card rectangle with > title and icon would be visible almost immediately, but the screenshot area > was black for many seconds. Sometimes the screenshot would take as much as a > minute to show up - if at all. I'm still unable to reproduce this, with (relatively) freshly flashed Flame, moz-central/master with 319MB. But the original STR do produce a blank screenshot area for the camera app if you holdhome quickly while the Camera app is still loading/initializing I
Assignee | ||
Updated•9 years ago
|
Summary: [Window Management] Inactive app windows extremely slow to paint in card view → [Window Management] App windows appear blue/grey in card view
Assignee | ||
Updated•9 years ago
|
Summary: [Window Management] App windows appear blue/grey in card view → [Window Management] Active app window appears blue/grey in card view
Assignee | ||
Comment 40•9 years ago
|
||
I've updated the title of this bug. Sorry about the confusion. We're focusing on the original STR in the description. You can repro quite easily by launching camera app and immediately long-tap the home button to show cards view. The blue/grey is a background-color added in https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_window.js#L1035,L1039, its a symptom of there being no screenshot available yet to display. We've run into this before. There's no event fired when a new screenshot does become available, so if requestScreenshotUrl returns null (_screenshotBlob is not yet populated) we dont recover - we just see that background color via the -moz-element which sits behind the screenshot "layer". In the debugger, at least some of the time you can see that a screenshot later becomes available; in the console, do: appWindowManager.getActiveApp().requestScreenshotURL() .. so it might help to publish an event in AppWindow which the card can listen for if app.isActive(), but app.requestScreenshotURL() returns falsey.
Comment 41•9 years ago
|
||
Assignee | ||
Comment 42•9 years ago
|
||
Comment on attachment 8568683 [details] [review] [gaia] sfoster:task-manager-screenshotupdate > mozilla-b2g:master Aus, you've run into this same issue with screenshots in task-manager. Can I get your feedback on this before I go further?
Flags: needinfo?(aus)
Assignee | ||
Comment 43•9 years ago
|
||
Ive left logging in there which shows we actually get 2 callbacks and 2 screenshots. This is a bug? I'm using a needsScreenshot flag on the Card to just update on the first one.
Comment 44•9 years ago
|
||
Hi Sam, We had seen this before for sure. The screenshot was actually taken and available but was simply not getting repainted until the card was somehow dragged which causes the surface to force repaint. Simply setting the new screenshot as the background doesn't seem to always cause the invalidation of the drawing surface. I have no idea why. It's a bug, it's kind of nasty. :(
Flags: needinfo?(aus)
Assignee | ||
Comment 45•9 years ago
|
||
> We had seen this before for sure. The screenshot was actually taken and
> available but was simply not getting repainted until the card was somehow
> dragged which causes the surface to force repaint. Simply setting the new
> screenshot as the background doesn't seem to always cause the invalidation
> of the drawing surface. I have no idea why. It's a bug, it's kind of nasty.
> :(
Ok, well I'm asserting this patch fixes at least some manifestations of this bug. I hope this additional asynchrony of the appscreenshotupdate listener doesn't complicate too much. I'm going to look at some tests and put it in for review
Assignee | ||
Comment 46•9 years ago
|
||
status: currently going around in circles on this one. The problem is that if you holdhome while an app is still initializing, the iframe.getScreenshot is slow to respond and we timeout before it returns. The best outcome I've had so far turns out to be simply bumping up the timeout at https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/task_manager.js#L532,L534 .. but that swings us back to a situation where the UI appears slow to respond to the long home button tap. The screenshotupdate approach does succeed make a screenshot available in the task manager, but by the time we see it, we've also watched the transition to closed via the -moz-element and it ends up looking very flickery and disjointed - particularly when the screenshot doesn't look anything like the last state we saw in the window. FWIW I didn't get a integration test to fail (I tried skipping the waitUntilScreenshotable and doing taskManager.show()) to test the patch against. So I'm 0/2 on this right now.
Assignee | ||
Comment 47•9 years ago
|
||
(thinking out loud) At https://mxr.mozilla.org/mozilla-central/source/dom/browser-element/BrowserElementChildPreload.js#904 We see: // Try to wait for the event loop to go idle before we take the screenshot, // but once we've waited maxDelayMS milliseconds, go ahead and take it // anyway. maxScreenshotDlayMS defaults to 2000ms, so this accounts for the timeout we often see when trying to get a screenshot of an app that is busy initializing. I don't think we want to wait much more than the 400ms we currently do before giving up and showing the task manager anyway. The question is, what to do then? Another screenshot is created when the app closes, we could use that but we'll get a flicker as we redraw the background. Currently requestScreenshotURL can give us an old screenshotBlob in the case where the getScreenshot call times out, which may look completely different to what the user last saw of the app before they long-tapped home. Maybe we could leave up the identificationOverlay if we've nothing to show in the screenshotOverlay?
Flags: needinfo?(etienne)
Comment 48•9 years ago
|
||
(In reply to Sam Foster [:sfoster] from comment #47) > (thinking out loud) > > At > https://mxr.mozilla.org/mozilla-central/source/dom/browser-element/ > BrowserElementChildPreload.js#904 > We see: > > // Try to wait for the event loop to go idle before we take the screenshot, > // but once we've waited maxDelayMS milliseconds, go ahead and take it > // anyway. > Ha! This explains a lot, good find! > Currently requestScreenshotURL can give us an old screenshotBlob in the case > where the getScreenshot call times out, which may look completely different > to what the user last saw of the app before they long-tapped home. Maybe we > could leave up the identificationOverlay if we've nothing to show in the > screenshotOverlay? We should check with UX but I think this is the best alternative.
Flags: needinfo?(etienne)
Assignee | ||
Comment 49•9 years ago
|
||
(In reply to Etienne Segonzac (:etienne) from comment #48) > > Currently requestScreenshotURL can give us an old screenshotBlob in the case > > where the getScreenshot call times out, which may look completely different > > to what the user last saw of the app before they long-tapped home. Maybe we > > could leave up the identificationOverlay if we've nothing to show in the > > screenshotOverlay? > > We should check with UX but I think this is the best alternative. What do you think, Francis?
Flags: needinfo?(fdjabri)
Comment 50•9 years ago
|
||
(In reply to Sam Foster [:sfoster] from comment #49) > (In reply to Etienne Segonzac (:etienne) from comment #48) > > > Currently requestScreenshotURL can give us an old screenshotBlob in the case > > > where the getScreenshot call times out, which may look completely different > > > to what the user last saw of the app before they long-tapped home. Maybe we > > > could leave up the identificationOverlay if we've nothing to show in the > > > screenshotOverlay? > > > > We should check with UX but I think this is the best alternative. > > What do you think, Francis? Hi Sam, as we discussed, this sounds good to me.
Flags: needinfo?(fdjabri)
Assignee | ||
Updated•9 years ago
|
Attachment #8568683 -
Attachment is obsolete: true
Comment 51•9 years ago
|
||
Assignee | ||
Comment 52•9 years ago
|
||
Comment on attachment 8576945 [details] [review] [gaia] sfoster:task-manager-no-screenshot-bug-1107139 > mozilla-b2g:master Status update - this is fiddly stuff. The patch expresses what I *want to happen* but its not quite there. At cardviewbeforeshow, if the app doesn't get have a screenshotBlob, we want to show the notification overlay and watch that scale via the closeAppTowardsCardView anim. Right now it swaps in after that animation is done. Does it jump out at you Etienne? When we select a card in the task manager and re-open, right now the identification overlay is hidden immediately. That ends up looking ok to me.
Attachment #8576945 -
Flags: feedback?(etienne)
Comment 53•9 years ago
|
||
Comment on attachment 8576945 [details] [review] [gaia] sfoster:task-manager-no-screenshot-bug-1107139 > mozilla-b2g:master (hopefully helpful) comments on github! And f+ for the logic/structure of the patch (minus all the debugging logs ;))
Attachment #8576945 -
Flags: feedback?(etienne) → feedback+
Assignee | ||
Comment 54•9 years ago
|
||
Comment on attachment 8576945 [details] [review] [gaia] sfoster:task-manager-no-screenshot-bug-1107139 > mozilla-b2g:master This is performing pretty well for me. You'll often see the identification-overlay - when going to task manager from an active app the first time. But if the screenshot comes before the timeout we display it, and using cardviewshown can remove the .no-screenshot class so you get the expected transition when hiding the task-manager. I nudged down the timeout for the screenshot in the task manager because of this - I think we can trade up for a bit more responsiveness with this arrangement. Left one new this.debug statement in, happy to remove though if you prefer
Attachment #8576945 -
Flags: review?(etienne)
Comment 55•9 years ago
|
||
Comment on attachment 8576945 [details] [review] [gaia] sfoster:task-manager-no-screenshot-bug-1107139 > mozilla-b2g:master Looking good, r=me with quick unit tests for the AppWindow/AppWindowManager changes.
Attachment #8576945 -
Flags: review?(etienne) → review+
Comment 56•9 years ago
|
||
I just noticed that in master branch, card view is shown in portrait mode only, and I can't seem to repro this bug anymore. was this orientation fix required to fix this bug? I welcome the change, just that I couldn't find the discussion specific to the orientation change.
Assignee | ||
Comment 57•9 years ago
|
||
(In reply to No-Jun Park [:njpark] from comment #56) > I just noticed that in master branch, card view is shown in portrait mode > only, and I can't seem to repro this bug anymore. was this orientation fix > required to fix this bug? > > I welcome the change, just that I couldn't find the discussion specific to > the orientation change. We locked to portrait (or default) in bug 1118390 - which restored the behavior we had in earlier versions of the card view. Eventually we want to support landscape and portrait card view but its quite a large effort and out of scope for 2.2
Assignee | ||
Comment 58•9 years ago
|
||
Figured out why my unit tests were failing :/ I've updated the PR with what I hope to get to pass once bug 1143886 lands.
Depends on: 1143886
Updated•9 years ago
|
Keywords: checkin-needed
Comment 60•9 years ago
|
||
Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/da4b493f94a4d06d2be49f7f07e2fabca7392781
Updated•9 years ago
|
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment 61•9 years ago
|
||
sorry had to back this out since this caused in mozilla-inbound when landing with the merges test failures like https://treeherder.mozilla.org/img/logviewerIcon.svg
Status: RESOLVED → REOPENED
Flags: needinfo?(sfoster)
Resolution: FIXED → ---
Comment 62•9 years ago
|
||
(In reply to Carsten Book [:Tomcat] from comment #61) > sorry had to back this out since this caused in mozilla-inbound when landing > with the merges test failures like > https://treeherder.mozilla.org/img/logviewerIcon.svg err wrong link :) https://treeherder.mozilla.org/logviewer.html#?job_id=7746599&repo=mozilla-inbound is better :)
Comment 63•9 years ago
|
||
Assignee | ||
Comment 64•9 years ago
|
||
Comment on attachment 8579462 [details] [review] [gaia] sfoster:task-manager-no-screenshot-v2-bug-1107139 > mozilla-b2g:master I've updated the test in question. I'll attempt to re-land once the Gaia-Try results go green for the new PR. Carrying Etienne's r+.
Flags: needinfo?(sfoster)
Attachment #8579462 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Attachment #8576945 -
Attachment is obsolete: true
Assignee | ||
Comment 65•9 years ago
|
||
Comment on attachment 8579462 [details] [review] [gaia] sfoster:task-manager-no-screenshot-v2-bug-1107139 > mozilla-b2g:master Former version of this patch had r+ from :etienne. I've fixed the test that caused the backout. Looking for a review stamp to get this landed again.
Attachment #8579462 -
Flags: review+ → review?(kgrandon)
Comment 66•9 years ago
|
||
Comment on attachment 8579462 [details] [review] [gaia] sfoster:task-manager-no-screenshot-v2-bug-1107139 > mozilla-b2g:master Leaving a review stamp in case you want to leverage autolander. Sorry about the workflow pain.
Attachment #8579462 -
Flags: review?(kgrandon) → review+
Assignee | ||
Comment 67•9 years ago
|
||
I'm told autolander will run Gij before attempting to merge, so lets try it.
Keywords: checkin-needed
Updated•9 years ago
|
Keywords: checkin-needed
Comment 68•9 years ago
|
||
Comment 69•9 years ago
|
||
Comment on attachment 8579462 [details] [review] [gaia] sfoster:task-manager-no-screenshot-v2-bug-1107139 > mozilla-b2g:master I want to see if autolander will process a new attachment.
Attachment #8579462 -
Attachment is obsolete: true
Updated•9 years ago
|
Attachment #8580385 -
Flags: review+
Updated•9 years ago
|
Keywords: checkin-needed
Comment 71•9 years ago
|
||
Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/9c6c6d38ab7c9864459e77e44648494b7c209a0b
Updated•9 years ago
|
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 73•9 years ago
|
||
(In reply to Hermes Cheng[:hermescheng] from comment #72) > Please uplift to v2.2. Thanks! This got backed out earlier this week. I would like to let it bake on master for at least a day before uplifting. I'll request uplift before the end of the week
We're seeing problems in tests with: JavascriptException: TypeError: this.element is null in app://system.gaiamobile.org/gaia_build_defer_index.js line: 2337 I'll link in the bug I'm filing presently with more info, but we think it's a pretty good chance that's coming from https://github.com/mozilla-b2g/gaia/commit/413a4dea46b52afcba7cb110542edb4c4c1f188d since it just started this morning.
Assignee | ||
Comment 76•9 years ago
|
||
(In reply to Geo Mealer [:geo] from comment #75) > We're seeing problems in tests with: > > JavascriptException: TypeError: this.element is null in > app://system.gaiamobile.org/gaia_build_defer_index.js line: 2337 > > I'll link in the bug I'm filing presently with more info, but we think it's > a pretty good chance that's coming from > https://github.com/mozilla-b2g/gaia/commit/ > 413a4dea46b52afcba7cb110542edb4c4c1f188d since it just started this morning. There is an unguarded this.element access at https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_window.js#L1322 which *could* be the problem? I can't imagine when we would have a .screenshotOverlay but not a .element. cc me though on the bug and I'll look into it. Or back out this one.
Well, I looked further, and the tests I have that bracket the issue have the same Gaia and different Geckos. Unfortunately, I don't have an easy way to bisect Gaia UI tests over Gecko, so that's a tough one. Bracketing tinderbox builds are: tinderbox-builds/b2g-inbound-flame-kk-eng/20150319180839 https://hg.mozilla.org/integration/b2g-inbound/rev/069d3d6722ad tinderbox-builds/b2g-inbound-flame-kk-eng/20150319193241 https://hg.mozilla.org/integration/b2g-inbound/rev/ed6fdb6e238e In both cases, Gaia is: https://github.com/mozilla-b2g/gaia/commit/6e24aff11fbb385a6fb1651536a97263457b6c50 Still working on the logs/stacks on the primary bug, will update here when filed.
I won't block/depends, since the bracket suggests a Gecko change rather than Gaia
Blocks: 1145875
Assignee | ||
Comment 79•9 years ago
|
||
Comment on attachment 8580385 [details] [review] [gaia] KevinGrandon:bug_1107139_retry_autoland > mozilla-b2g:master [Approval Request Comment] [Bug caused by] (feature/regressing bug #): Task Manager / Tab View [User impact] if declined: Some apps can appear blank in card view [Testing completed]: Tested on device, new integration tests [Risk to taking this patch] (and alternatives if risky): Low risk [String changes made]: None
Attachment #8580385 -
Flags: approval-gaia-v2.2?
Comment 80•9 years ago
|
||
Hi Sam, I follow this steps to try to repro it on flame 2.2, I'm not sure if this issue is fixed,the details as follow: 1. Open full screen app (e.g. Camera) 2. Tap home to return to homescreen 3-1. If we Long-tap home button in short time after backing to homescreen, there is a grey screenshort of camera displayed. 3-2. If we waiting for 2-3 sec after backing to homscreen, and then long tap home button,a corrected screenshort of camera displayed. Please check the attachmet: verify1.3gp and logcat324.txt. Environmental variables: Flame 2.2: Build ID 20150323162503 Gaia Revision e54c4ed1cc188f70ddf1156534d364005dc45490 Gaia Date 2015-03-23 19:09:26 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/7ba1778d237b Gecko Version 37.0 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150323.200543 Firmware Date Mon Mar 23 20:05:54 EDT 2015 Bootloader L1TC000118D0
Comment 81•9 years ago
|
||
Comment 82•9 years ago
|
||
Flags: needinfo?(sfoster)
Comment 83•9 years ago
|
||
Alissa, the patch has not been landed to 2.2 yet and is still waiting for approval (comment 79). I think the build will be ready for verification in a couple of days. Clearing NI for Sam. (In reply to Alissa from comment #80) > Hi Sam, I follow this steps to try to repro it on flame 2.2, I'm not sure if > this issue is fixed,the details as follow: > 1. Open full screen app (e.g. Camera) > 2. Tap home to return to homescreen > 3-1. If we Long-tap home button in short time after backing to homescreen, > there is a grey screenshort of camera displayed. > 3-2. If we waiting for 2-3 sec after backing to homscreen, and then long tap > home button,a corrected screenshort of camera displayed. > > Please check the attachmet: verify1.3gp and logcat324.txt. > > Environmental variables: > Flame 2.2: > Build ID 20150323162503 > Gaia Revision e54c4ed1cc188f70ddf1156534d364005dc45490 > Gaia Date 2015-03-23 19:09:26 > Gecko Revision > https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/7ba1778d237b > Gecko Version 37.0 > Device Name flame > Firmware(Release) 4.4.2 > Firmware(Incremental) eng.cltbld.20150323.200543 > Firmware Date Mon Mar 23 20:05:54 EDT 2015 > Bootloader L1TC000118D0
Flags: needinfo?(sfoster)
Comment 84•9 years ago
|
||
Assignee | ||
Comment 85•9 years ago
|
||
bug 1146680 turns out to be a regression caused by attachment #8580385 [details] [review] that landed. Will backout and create a new updated PR - as we want to uplift this one, that seems the cleaner route.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 86•9 years ago
|
||
Assignee | ||
Comment 87•9 years ago
|
||
Comment on attachment 8580385 [details] [review] [gaia] KevinGrandon:bug_1107139_retry_autoland > mozilla-b2g:master Will need a new patch for 2.2 uplift, cancelling approval request.
Attachment #8580385 -
Flags: approval-gaia-v2.2?
Assignee | ||
Comment 88•9 years ago
|
||
Comment on attachment 8582674 [details] [review] [gaia] sfoster:task-manager-no-screenshot-bug-1107139 > mozilla-b2g:master This is same patch as before (cset 413a4dea46b52afcba7cb110542edb4c4c1f188d) with fix/un-regressing bug 1146680 and fixed test that should have caught that.
Attachment #8582674 -
Flags: review?(kgrandon)
Assignee | ||
Comment 89•9 years ago
|
||
Test run is shiny green. Just needs r+ to land
Comment 90•9 years ago
|
||
Comment on attachment 8582674 [details] [review] [gaia] sfoster:task-manager-no-screenshot-bug-1107139 > mozilla-b2g:master Review stamping this based on Etienne's review earlier.
Attachment #8582674 -
Flags: review?(kgrandon) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Assignee | ||
Updated•9 years ago
|
Attachment #8580385 -
Attachment is obsolete: true
Updated•9 years ago
|
Keywords: checkin-needed
Assignee | ||
Updated•9 years ago
|
Attachment #8582610 -
Attachment is obsolete: true
Comment 91•9 years ago
|
||
Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/ab301b6930ca2385d0ea59c184a101de007c8e75
Updated•9 years ago
|
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
Comment 92•9 years ago
|
||
Assignee | ||
Comment 93•9 years ago
|
||
Comment on attachment 8583592 [details] [review] [gaia] sfoster:task-manager-no-screenshot-v2-bug-1107139 > mozilla-b2g:v2.2 Treeherder still unhappy. Michael, could you possibly push this over the line when v2.2 branch tests get cleared up? I had requested uplift before, but as the master patch got backed out, we'll need to do it again. Note that this PR includes the patch to fix AppWindow element unit test stubs, which is tracked in bug 1143886. I guess it will need flattening before landing, but I wanted to see a clean test run in the meantime as I've not been able to land that.
Flags: needinfo?(mhenretty)
Comment 94•9 years ago
|
||
Comment on attachment 8583592 [details] [review] [gaia] sfoster:task-manager-no-screenshot-v2-bug-1107139 > mozilla-b2g:v2.2 [Approval Request Comment] [Bug caused by] (feature/regressing bug #): Task Manager / Tab View [User impact] if declined: Some apps can appear blank in card view [Testing completed]: Tested on device, new integration tests [Risk to taking this patch] (and alternatives if risky): Low risk [String changes made]: None Note: this was originally landed and then backed out of master. Bug 1143886 (test-only) was landed on v2.2 recently to address that problem, so this is ready to land on v2.2 again.
Flags: needinfo?(mhenretty)
Attachment #8583592 -
Flags: approval-gaia-v2.2?
Comment 95•9 years ago
|
||
(In reply to Sam Foster [:sfoster] from comment #93) > Comment on attachment 8583592 [details] [review] > [gaia] sfoster:task-manager-no-screenshot-v2-bug-1107139 > mozilla-b2g:v2.2 > > Treeherder still unhappy. Michael, could you possibly push this over the > line when v2.2 branch tests get cleared up? I had requested uplift before, > but as the master patch got backed out, we'll need to do it again. Note that > this PR includes the patch to fix AppWindow element unit test stubs, which > is tracked in bug 1143886. I guess it will need flattening before landing, > but I wanted to see a clean test run in the meantime as I've not been able > to land that. Sam, unfortunately we aren't going to get green runs on Gaia-Try vor v2.2 until they uplift a bunch of task cluster stuff to that branch. This is an unfortunate side effect of the TC migration, so the best we can do is make sure it's working on master for now, uplift, and watch.
Updated•9 years ago
|
status-b2g-master:
--- → fixed
Target Milestone: --- → 2.2 S9 (3apr)
Comment 96•9 years ago
|
||
Parul, can you please help verify this on master ?
Flags: needinfo?(pmathur)
Verified not fixed on master. Repro Steps: 1) Open the Browser app. 2) Open the Camera app and then the Gallery app. 3) Press and hold the home button to enter card view. 4) Observe the cards. Actual: Gallery app in card view appears gray and then Gallery app closes. Camera app in card view first appears gray for a brief flash, then shows the correct screenshot, then sometimes shows a black screenshot with camera controls. Expected: The camera and gallery apps appear correctly in card view. Test Environment: Gaia-Rev 67ad91f3f660b1f16b354ee4c5159ddc5a74d149 Gecko-Rev https://hg.mozilla.org/mozilla-central/rev/385840329d91 Build-ID 20150329160203 Version 39.0a1 Device-Name flame FW-Release 4.4.2 FW-Incremental eng.cltbld.20150329.192807 FW-Date Sun Mar 29 19:28:16 EDT 2015 Bootloader L1TC000118D0
Flags: needinfo?(pmathur)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 98•9 years ago
|
||
(In reply to Parul Mathur [:pragmatic] from comment #97) > Actual: > Gallery app in card view appears gray and then Gallery app closes. > Camera app in card view first appears gray for a brief flash, then shows the > correct screenshot, then sometimes shows a black screenshot with camera > controls. I think this may be a separate issue with the Gallery app - it seems to crash if its closed before it finishes initializing, I'll look into it and file a bug if necessary. The camera app behavior is expected - it is changing state as it opens and as it transitions to card view. I'll check its doing the right thing but I don't want to boil the ocean with this bug.
Comment 99•9 years ago
|
||
Here's what I found on today's nightly build: 1) When entering the card view, the Camera app always shows the gray screen with the icon in the center. 2) When entering the card view, the Gallery app shows the screenshot correctly when the Gallery app is open with no or a few other apps open in the background. 3) When entering the card view, the Gallery app shows the gray screen with the icon in the center when several other apps are open in the background. 4) When entering card view while the Gallery app is still loading images, the card view is shown for a brief moment, and the app and card view close. The user is returned to the homescreen. I'm not sure if any of these symptoms are expected behaviors. Please let me know if there has to be a new bug filed. Environmental Variables: Device: Flame 3.0 (KK, 319mb, full flash) Build ID: 20150330010204 Gaia: be25b16efa19bab8d54be08f8fe45dcc93bf93d0 Gecko: dfe60814eda7 Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b Version: 39.0a1 (3.0) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0
Flags: needinfo?(ktucker)
Updated•9 years ago
|
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Reporter | ||
Comment 100•9 years ago
|
||
I am not seeing any of these issues stated in Comment 99 occur on today's master build besides number 4. The gallery app is suppose to force close if the user leaves the app while it is loading. This is by design unless it has been changed. Piwei, can you please double check the results as well in Comment 99? We are seeing conflicting results so there might be something else going on here.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage-]
Flags: needinfo?(ktucker) → needinfo?(pcheng)
Assignee | ||
Comment 101•9 years ago
|
||
(In reply to KTucker [:KTucker] from comment #100) > I am not seeing any of these issues stated in Comment 99 occur on today's > master build besides number 4. The gallery app is suppose to force close if > the user leaves the app while it is loading. This is by design unless it has > been changed. If that is the case then card view is working as expected. If the app terminates and it is the only app running then there's nothing to show and we go back to homescreen. Note that the premise for this bug is all about timing, so we expect some variation, especially if there are several apps running. The fix implemented in the patch is to show the identification overlay - the icon in the middle - if no screenshot was immediately available *for the active app* when going into card view.
Assignee | ||
Comment 102•9 years ago
|
||
This patch already landed on master. If we determine it doesn't fix the problem, we need to either back out the patch (and re-open the bug) or file follow-up bugs. In the meantime it remains resolved but not verified.
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
Comment 103•9 years ago
|
||
I am hesitant to uplift on 2.2 given QA's feedback. Sam, can you confirm you cannot reproduce the issue on latest master? Has this been tested by you on the device ? Looks like QA is using the exact STR in the description.
Flags: needinfo?(sfoster)
Assignee | ||
Comment 104•9 years ago
|
||
(In reply to bhavana bajaj [:bajaj] from comment #103) > I am hesitant to uplift on 2.2 given QA's feedback. Sam, can you confirm you > cannot reproduce the issue on latest master? Has this been tested by you on > the device ? Looks like QA is using the exact STR in the description. I agree we should clear up any questions before uplifting. For my part I've tested this extensively on device, on master. I'll also do some testing with the patch on v2.2. It looks like gallery will close itself if we go into card view while it is still scanning for images/videos. This is the 'backgroundScanKiller' function being called as the app is closed. I'm not sure if that is new behavior or if it should be considered a regression caused by this patch. As the original STR use gallery specifically, lets get clarification from David on that. David: What this patch changes is that it no longer waits as long for the event loop to idle before showing the card view and transitioning the active app (Gallery in this case) to "closed". This is to avoid blank cards in cards view, and to ensure the transition to card view is smooth and responsive. If we are seeing expected behavior from Gallery, then both comment #97 and comment #99 indicate verification that this bug is fixed.
Flags: needinfo?(sfoster) → needinfo?(dflanagan)
Reporter | ||
Updated•9 years ago
|
Flags: needinfo?(pcheng) → needinfo?(pcheng)
Comment 105•9 years ago
|
||
On devices with < 512mb of memory, the Gallery app intentionally closes itself when sent to the background if it is still scanning images. This is to prevent a background scan from using memory and slowing down the entire system. If you test with a Flame with 512mb or more you should see that the gallery does not exit even if it is still scanning when you go to the card view. I don't know about the camera app, but it would not suprise me if it is not possible to get a screenshot that includes the contents of the viewfinder. There might be too much magic going on with that, so the gray screen for camera in the cards view might be what is expected. I don't know anything about the timing around displaying the card view, so I can't comment on that. I will add that you can configure the timing of the camera app killing itself if that would improve things. Currently it waits 500ms after getting its visibilitychange event. If you changed that to 3 seconds or something you might not notice the app exiting in the cards view as often. See gallery.js:776
Updated•9 years ago
|
Flags: needinfo?(dflanagan)
Reporter | ||
Comment 106•9 years ago
|
||
Please see bug 1039943 like David said this is intentional in regards to the gallery app being closed.
Comment 107•9 years ago
|
||
As per comment 100's Needinfo request, I'm posting my testing results. (In reply to Yeojin Chung [:YeojinC] from comment #99) > 1) When entering the card view, the Camera app always shows the gray screen > with the icon in the center. I can get this to reliably occur if Camera is the only app in Card View and I enter Card View while in Camera. > 2) When entering the card view, the Gallery app shows the screenshot > correctly when the Gallery app is open with no or a few other apps open in > the background. I also see this. > 3) When entering the card view, the Gallery app shows the gray screen with > the icon in the center when several other apps are open in the background. I have only seen this once after I'd taken ~10 pictures using the Camera (I originally had ~4 in Gallery), then I went to Gallery, and entered card view once it finished loading. I couldn't reproduce this afterward, with the same steps. > 4) When entering card view while the Gallery app is still loading images, > the card view is shown for a brief moment, and the app and card view close. > The user is returned to the homescreen. This behavior is expected and I can reproduce as well. I'm on the same build as Comment 99. Full flashed 319MB memory.
Flags: needinfo?(pcheng)
Assignee | ||
Comment 108•9 years ago
|
||
So these are all vindications of the patch doing the thing it was supposed to do. Instead of showing an empty card when - for whatever reason - no screenshot of the app was available, it shows the icon (identification overlay) The horizontal task switcher is on v2.2, but this patch is not, so we can evaluate the behavior meaningfully by comparing master to v2.2. This patch cant fix all the many ways an app can end up looking different in the card view, but it does try to make it not look broken. After trying out the 4 cases from comment #107, I get the same results in v2.2, except that I see an empty-looking blue/grey space where I would expect a screenshot - vs on master where I see the app's icon on the translucent overlay. I don't think we have regressed anything here?
Assignee | ||
Comment 109•9 years ago
|
||
Naoki, can you confirm this is verified, and is there anything else we want to do before getting approval to land on v2.2?
Flags: needinfo?(nhirata.bugzilla)
So I tested on 2.2 and Master. 2.2 doesn't have the patch, and master does. 1) the gallery app closing in task manager is intentional. I don't really like it because it makes the app look like it crashed. 2) if the app is in a state that's not ready, this patch doesn't fix that issue; ie camera, and gallery with lots of images. 3) if he app has the data base loaded and the state is ready, this patch will address that issue. So what most of QA is seeing I think is issue 2, and what sfoster fixed is issue 3. I think we should split issue 2 into a separate bug. To note, you can get in a state where you get a bad screenshot of things not fully loaded even though the app has fully loaded based on timing of the screenshot taken.
The patch doesn't address 100 % of the issues. If we are to keep the patch, we should probably split this off into two bugs; app has database loaded and app doesn't have database loaded. In the condition that the app doesn't have the database loaded or is ready when the home button is hit, it tends to capture screenshots of black screens. In the condition that we have no screenshot, we have a replacement of the icon of the app so we can tell which app it is easier ( though we do have subtitles for the app ). This is similar to what we did with Tarako, I think? This is what part of the fix is. In the condition that we have the database loaded and we have a screenshot we show the appropriate image. I don't have a strong opinion either way for it landing on 2.2 now. Personally I think we should just do away with screenshoting altogether because it slows down performance and just do app cards...
Flags: needinfo?(nhirata.bugzilla)
Comment 112•9 years ago
|
||
Has anyone spoken to anyone on the gfx team about improving the screenshot ability? I think currently it renders the whole app off-screen, and there used to be some odd behaviour with fixed-pos content (which might have been fixed?), but it seems there's no need for this to be so slow. I suggest that if we aren't doing this already (and I assume we aren't, as screenshotting is slow), it'd be much more efficient for screenshots to just do a composite + readback from GPU. Not knowing what the API is like, maybe there are some complications I haven't considered, but given how fast composition is, screenshotting really oughtn't be something so slow (unless the slow part is image encoding/decoding?)
Assignee | ||
Comment 113•9 years ago
|
||
Let me clarify the scope of this bug and the patch: Sometimes, when showing the task manager from an app (vs. the homescreen), the card in the task manager may appear an empty blue/grey color. There are multiple reasons an app may appear different in card view, this bug focusses on an entirely missing screenshot - the blue/grey color is the background normally obscured by the image. STR: In principal any application could exhibit this behavior, but it is easier to reproduce with apps that do a lot of work at initialization. 1. Open Camera (or Contacts, or Gallery - but see comment #110) 2. Quickly long-tap the home button while the app is still starting up Expected: The task manager shows a single card with some recognizable representation of the last active app Actual: The task manager shows the title and icon/button, but an empty space where the screenshot should be. The root cause of this issue is that when an app first starts, we have no screenshot for it yet. When task manager shows, we have been requesting and waiting for one. That gave us bug 1081321. The fix for that was to timeout and bail on the screenshot if its was taking too long - which left the card empty. This patch takes the approach that if we have no screenshot already, we don't want to wait too long for one, so instead we show the identification overlay - the semi-opaque background with icon in the middle. So the expected result now is that the active app should be represented in task manager by the icon, if a screenshot was not immediately available. Note that this change alters the timing of the show task manager sequence. As we no longer block on the screenshot (which waits for the event loop to idle before timing out), apps may be transitioned to hidden faster than before. That is good for overall UX as the long-press on home appears more responsive. But we may see some fallout with apps like Gallery that watch and react to visibility changes - they may get during their startup. In the case of Gallery, it closes itself if it gets hidden while still scanning for media, to avoid that background processing lagging the experience. With this change we are more likely to see that and the result is that Gallery starts up (STR step1) and then quits (disappears) at step 2.
Assignee | ||
Comment 114•9 years ago
|
||
Comment on attachment 8582674 [details] [review] [gaia] sfoster:task-manager-no-screenshot-bug-1107139 > mozilla-b2g:master Can UX chime in on: a) if the patch we landed adequately fixes the problem outlined in original STR and in comment #113 b) if we need to continue to block v2.2 on this c) if the current fix is something we want for sure on v2.2 A simple before/after comparison can be had by testing current master vs. current v2.2, or by applying attachment #8583592 [details] [review] to a v2.2 build
Attachment #8582674 -
Flags: ui-review?(fdjabri)
Comment 115•9 years ago
|
||
I have filed bug 1150168 to address the issue observed at comment 107 that is specific to Camera app.
I filed bug 1150278 for the condition of an app still loading the database and the screenshot takes an image. To note: the condition where the screenshot hasn't had time to be taken is fixed under this bug and shows an icon instead.
Comment 117•9 years ago
|
||
I agree with the approach that Sam outlines in comment #113. If a screenshot is not ready, then we should show the title and an icon. The responsiveness of the home button should be the paramount concern, and if that means that we might not show a screenshot, I think that's a tradeoff worth making. Also, in reality, I think users will take a second or two to register a screen before users access task manager - so the fallout that Sam mentions should be very rare IMO. Overall, I'm happy with the patch landing.
Updated•9 years ago
|
Attachment #8582674 -
Flags: ui-review?(fdjabri) → ui-review+
Assignee | ||
Comment 118•9 years ago
|
||
Naoki/QA, can you summarize where we stand with this now. Can it be verified fixed? Ready/appropriate for uplift? (comment #103)
Flags: needinfo?(nhirata.bugzilla)
We filed the other two bugs as fall outs from this one, we got approval from ux to land in 2.2; so I guess you just need to get landing approval for 2.2 and land on 2.2
Flags: needinfo?(nhirata.bugzilla) → needinfo?(sfoster)
Updated•9 years ago
|
Attachment #8583592 -
Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
Assignee | ||
Comment 120•9 years ago
|
||
I've rebased and picked up a late fix from the master PR, v2.2 PR is updated. I'll merge shortly
Flags: needinfo?(sfoster)
Assignee | ||
Comment 121•9 years ago
|
||
Attachment #8583592 [details] merged to v2.2: https://github.com/mozilla-b2g/gaia/commit/1feca352a1dee33a42eab032a82074a24fb05906 Commit: f619797054896e0a31a43bd61bedc48ebe55ecbf
Assignee | ||
Updated•9 years ago
|
Comment 122•9 years ago
|
||
This issue is verified fixed on the latest Nightly Flame 3.0 and 2.2 builds. Actual Results: The card view shows screenshots if available or shows a card for the last app containing that app's icon and name instead. Environmental Variables: Device: Flame 3.0 BuildID: 20150403010203 Gaia: 7969b367a7da62877c3a24a26d3cb5fda89d766c Gecko: 70a113676b21 Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b Version: 40.0a1 (3.0) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0 Environmental Variables: Device: Flame 2.2 BuildID: 20150403002503 Gaia: 022eeb91197ba4a9adfd67bd6db5aa03cc69eb31 Gecko: 77fdc6fbcae9 Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429 Version: 37.0 (2.2) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage-] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Keywords: verifyme
Reporter | ||
Updated•9 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in
before you can comment on or make changes to this bug.
Description
•