Closed
Bug 810732
Opened 12 years ago
Closed 11 years ago
Fennec tests are not running on Nightly
Categories
(Firefox for Android Graveyard :: General, defect, P1)
Tracking
(firefox17 unaffected, firefox18 unaffected, firefox19+ fixed, firefox20 fixed)
RESOLVED
FIXED
Firefox 20
Tracking | Status | |
---|---|---|
firefox17 | --- | unaffected |
firefox18 | --- | unaffected |
firefox19 | + | fixed |
firefox20 | --- | fixed |
People
(Reporter: evold, Assigned: bnicholson)
References
Details
Attachments
(4 files, 1 obsolete file)
355 bytes,
text/html
|
Details | |
11.17 KB,
patch
|
mfinkle
:
review+
akeybl
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
7.52 KB,
text/plain
|
Details | |
9.88 KB,
text/plain
|
Details |
The last version of nightly I tried, which was from a few days ago, I couldn't run Fennec tests on..
Reporter | ||
Comment 1•12 years ago
|
||
Basically Fennec would open and then nothing would happen, the tests didn't even start.
Is this still happening? Might have been a bad Nightly?
Flags: needinfo?(evold)
Priority: -- → P1
Is this still happening? Might have been a bad Nightly?
Reporter | ||
Updated•12 years ago
|
Flags: needinfo?(evold)
Comment 5•12 years ago
|
||
It is still happening, to me and Jeff as well, and as I wrote on bug 812739, it's not just the test is not working but everything: cfx run as well is not working. Notice that on previous version of Fennec Nightly 19 everything works fine.
Do we have a date of the most recently working Nightly?
Reporter | ||
Comment 7•12 years ago
|
||
(In reply to Wes Kocher (:KWierso) from comment #6) > Do we have a date of the most recently working Nightly? No, can you check that Wes? or should I?
(In reply to Erik Vold [:erikvold] [:ztatic] from comment #7) > No, can you check that Wes? or should I? I don't have anything set up to test Android nightlies at the moment. It'd probably be quicker for you or Matteo to do it. You can grab Fennec Nightlies back to mid-October here: http://ftp.mozilla.org/pub/mozilla.org/mobile/nightly/
(In reply to Wes Kocher (:KWierso) from comment #8) > You can grab Fennec Nightlies back to mid-October here: > http://ftp.mozilla.org/pub/mozilla.org/mobile/nightly/ With the folders ending with "mozilla-central-android/", I should probably add.
Reporter | ||
Comment 11•12 years ago
|
||
10-31 is the first nightly to fail.
Reporter | ||
Comment 12•12 years ago
|
||
Pointer to Github pull-request
Reporter | ||
Updated•12 years ago
|
Attachment #684812 -
Flags: review?(zer0)
Comment 13•12 years ago
|
||
Erik, could you explain (both here and commenting the code, in case we proceed with it) your solution? It seems that Fennec doesn't emit "sessionstore-windows-restored" properly: have you contacted the mobile team to understand why Fennec behaves in this way now? What they said? At first sight seems a regression they should fix, but if they introduce this behavior on purpose we should at least know why. And in that case, if they fire any other similar event instead of the "sessionstore-windows-restored" that we can use as APP_STARTUP, instead of branching.
Flags: needinfo?(evold)
Reporter | ||
Comment 14•12 years ago
|
||
I'll try to contact the mobile team to see why 'sessionstore-windows-restored' is no longer being emitted. It appears that we might be able to use 'final-ui-startup' instead but that fires before 'sessionstore-windows-restored' and before the browser window is ready, and a portion of the windows/tabs module would need to be rewritten to support this change, since it is assumed that the main browser window will be ready to use.
Flags: needinfo?(evold)
Comment 15•12 years ago
|
||
`final-ui-startup` was the one used in Fennec before they implemented properly `sessionstore-windows-restored`; but it wasn't good enough for a bunch of stuff, as you pointed out as well. We need to figured out why they introduced this behavior and if it's intentional or it's a regression, and then let's see how to proceed.
Reporter | ||
Comment 16•12 years ago
|
||
Hmm on Friday I thought it was intentional, but I brought the issue up in #mobile and it may not have been. Can you chime in for us Mark?
Assignee | ||
Comment 17•12 years ago
|
||
This is a regression from bug 722661. I had assumed that this message should only be sent after we actually restore a session, but it looks like this message should be dispatched unconditionally.
Blocks: 722661
Comment 18•12 years ago
|
||
(In reply to Brian Nicholson (:bnicholson) from comment #17) > This is a regression from bug 722661. I had assumed that this message should > only be sent after we actually restore a session, but it looks like this > message should be dispatched unconditionally. Correct
Reporter | ||
Updated•12 years ago
|
Attachment #684812 -
Flags: review?(zer0)
Assignee | ||
Comment 19•12 years ago
|
||
This patch ensures that sessionstore-windows-restored is always dispatched. Session:Restore is now always sent from Java to indicate that we've read the session file. Session:Restore is handled in SessionStore.js instead of browser.js so that both session restore calls ([1] and [2]) can be consolidated into a single place. Since SessionStore.js now handles the session restore on its own, we could remove shouldRestore() and restoreLastSession() from the IDL since we no longer call these outside of the component. [1] http://hg.mozilla.org/integration/mozilla-inbound/file/9aea73623c64/mobile/android/chrome/content/browser.js#l287 [2] http://hg.mozilla.org/integration/mozilla-inbound/file/9aea73623c64/mobile/android/chrome/content/browser.js#l1152
Assignee: evold → bnicholson
Attachment #685332 -
Flags: review?(mark.finkle)
Comment 20•12 years ago
|
||
Comment on attachment 685332 [details] [diff] [review] Ensure that sessionstore-windows-restored is always dispatched >diff --git a/mobile/android/components/SessionStore.js b/mobile/android/components/SessionStore.js >+ case "Session:Restore": { >+ } else { >+ if (this._shouldRestore) { >+ // Do a restore triggered by Gecko (e.g., if browser.sessionstore.resume_session_once is true) >+ this.restoreLastSession(false, null); I don't see where you removed this from a different place in the code. Was it just missing?
Assignee | ||
Comment 21•12 years ago
|
||
(In reply to Mark Finkle (:mfinkle) from comment #20) > Comment on attachment 685332 [details] [diff] [review] > Ensure that sessionstore-windows-restored is always dispatched > > >diff --git a/mobile/android/components/SessionStore.js b/mobile/android/components/SessionStore.js > > >+ case "Session:Restore": { > > >+ } else { > >+ if (this._shouldRestore) { > >+ // Do a restore triggered by Gecko (e.g., if browser.sessionstore.resume_session_once is true) > >+ this.restoreLastSession(false, null); > > I don't see where you removed this from a different place in the code. Was > it just missing? It's here (without the comment): http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#286 Though now that you point this part out, I think we need to make sure the cleanup code is called if we do a restore here, too.
Assignee | ||
Comment 22•12 years ago
|
||
(In reply to Brian Nicholson (:bnicholson) from comment #21) > Though now that you point this part out, I think we need to make sure the > cleanup code is called if we do a restore here, too. Actually, we don't. Our Java front-end doesn't know we're doing a restore in these cases, so it assumes we're doing a normal startup, which means we've already opened an about:home (or external URL) tab. So we're guaranteed to have at least one tab open, so we don't need to worry about failures here. I've updated the patch with this comment, along with other minor changes.
Attachment #685332 -
Attachment is obsolete: true
Attachment #685332 -
Flags: review?(mark.finkle)
Attachment #685711 -
Flags: review?(mark.finkle)
Updated•12 years ago
|
Attachment #685711 -
Flags: review?(mark.finkle) → review+
Assignee | ||
Comment 23•12 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/3f225d53e909
Comment 24•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3f225d53e909
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 25•11 years ago
|
||
I'm re-opening this because we seem to have a regression affecting aurora. To repro, create an SDK-based add-on with this code: require('tabs').open('http://mozilla.org'); To run on an android device, ensure adb etc is working, then run Firefox with this command-line incantation: cfx -b /usr/local/bin/adb --mobile-app=fennec_aurora --app=fennec-on-device --force-mobile -v run The behavior I see is: * on release and beta, the add-on starts up as normal and the tab opens in the foreground * on aurora ( 2012-12-27 ), the add-on does not start. You can go into add-on manager and enable / disable the add-on to get it going. * on nightly, the add-on does start, but opens the tab in the background. It feels like we have a fix for this that needs to be uplifted to aurora.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Updated•11 years ago
|
status-firefox17:
--- → unaffected
status-firefox18:
--- → unaffected
status-firefox19:
--- → affected
status-firefox20:
--- → unaffected
tracking-firefox19:
--- → ?
Updated•11 years ago
|
Comment 26•11 years ago
|
||
Brian - are you actively looking into this? Firefox 19 is now on mozilla-beta.
Assignee | ||
Comment 27•11 years ago
|
||
I don't know if I'm the best person to look at the problem from comment 25, but I can try. Jeff, do you see any errors in your logcat when the add-on doesn't start?
Flags: needinfo?(jgriffiths)
Comment 28•11 years ago
|
||
Flags: needinfo?(jgriffiths)
Comment 29•11 years ago
|
||
I've attached two log files, gathered using adb logcat | grep Gecko I don't see anything obviously different except that, in the case of the beta log, my code actually gets run. The code in main.js of my add-on is very simple: var { setTimeout } = require('timers'); try { console.log('got here...'); var tabs = require("tabs"); setTimeout(function() { console.log('in setTimeout...'); // tabs.open("http://mozilla.org/"); }, 5000); } catch (e) { console.log('Exception: '+e); }
Assignee | ||
Comment 30•11 years ago
|
||
It looks like the patch here actually landed in 20. We'll need to uplift it to 19.
Assignee | ||
Updated•11 years ago
|
Product: Add-on SDK → Firefox for Android
Target Milestone: --- → Firefox 20
Version: unspecified → Firefox 20
Assignee | ||
Comment 31•11 years ago
|
||
Comment on attachment 685711 [details] [diff] [review] Ensure that sessionstore-windows-restored is always dispatched, v2 [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 722661 User impact if declined: SDK tests don't run Testing completed (on m-c, etc.): m-c Risk to taking this patch (and alternatives if risky): low risk String or UUID changes made by this patch: none
Attachment #685711 -
Flags: approval-mozilla-aurora?
Updated•11 years ago
|
Attachment #685711 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 32•11 years ago
|
||
Comment on attachment 685711 [details] [diff] [review] Ensure that sessionstore-windows-restored is always dispatched, v2 [Triage Comment] Firefox 19 is now on mozilla-beta, changing the flag.
Attachment #685711 -
Flags: approval-mozilla-aurora+ → approval-mozilla-beta+
Assignee | ||
Comment 33•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/6a8ed2f15c9f
Comment 34•11 years ago
|
||
I can help verify this bug if there is a build I can be pointed towards.
Comment 35•11 years ago
|
||
(In reply to Jeff Griffiths (:canuckistani) from comment #34) > I can help verify this bug if there is a build I can be pointed towards. Based on tbpl, you should find a build here: http://ftp-scl3.mozilla.com/pub/mozilla.org/mobile/tinderbox-builds/mozilla-beta-android/1357675439/
Comment 36•11 years ago
|
||
(In reply to Mark Finkle (:mfinkle) from comment #35) ... > Based on tbpl, you should find a build here: > http://ftp-scl3.mozilla.com/pub/mozilla.org/mobile/tinderbox-builds/mozilla- > beta-android/1357675439/ I initially got an error that Firefox Beta was already installed with a different cert, so I uninstalled it. Now when I try to install this build, I get 'application not installed'. adb logcat gives me this: E/PackageManager( 718): Package org.mozilla.firefox_beta has no signatures that match those in shared user org.mozilla.firefox.sharedID; ignoring! D/dalvikvm( 718): WAIT_FOR_CONCURRENT_GC blocked 0ms W/PackageManager( 718): Package couldn't be installed in /data/app/org.mozilla.firefox_beta-1.apk Is there any easy way to get past this?
Comment 37•11 years ago
|
||
If you have release firefox installed try uninstalling that too. Beta and release have the same sharedID and I've found in the past that if I roll my own of either of those it won't coexist with the official builds. I believe aurora and nightly also share a sharedID but one that is different from the one release/beta share so they shouldn't affect things.
Comment 38•11 years ago
|
||
Works for me, using the build :mfinkle linked to after uninstalling Firefox as well.
Status: REOPENED → RESOLVED
Closed: 12 years ago → 11 years ago
Resolution: --- → FIXED
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
•