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)

20 Branch
defect

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)

The last version of nightly I tried, which was from a few days ago, I couldn't run Fennec tests on..
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?
Flags: needinfo?(evold)
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?
(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.
I'll take a look soon then
Assignee: nobody → evold
10-31 is the first nightly to fail.
Attachment #684812 - Flags: review?(zer0)
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)
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)
`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.
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?
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
(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
Attachment #684812 - Flags: review?(zer0)
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 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?
(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.
(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)
Attachment #685711 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/mozilla-central/rev/3f225d53e909
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
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 → ---
Brian - are you actively looking into this? Firefox 19 is now on mozilla-beta.
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)
Flags: needinfo?(jgriffiths)
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);
}
It looks like the patch here actually landed in 20. We'll need to uplift it to 19.
Product: Add-on SDK → Firefox for Android
Target Milestone: --- → Firefox 20
Version: unspecified → Firefox 20
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?
Attachment #685711 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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+
I can help verify this bug if there is a build I can be pointed towards.
(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/
(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?
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.
Works for me, using the build :mfinkle linked to after uninstalling Firefox as well.
Status: REOPENED → RESOLVED
Closed: 12 years ago11 years ago
Resolution: --- → FIXED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.