Closed Bug 764901 Opened 11 years ago Closed 5 years ago

Intermittent Robocop testPasswordEncrypt | Storing a password while MP was set should fail - got content://org.mozilla.fennec.db.passwords/passwords/2?profilePath=%2Fmnt%2Fsdcard%2Ftests%2Fprofile, expected null

Categories

(Firefox for Android Graveyard :: General, defect, P3)

Product:
Firefox for Android Graveyard
Component:
General
Version:
16 Branch
Platform:
ARM
Android
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED INACTIVE
Milestone:
Firefox 17

People

(Reporter: mbrubeck, Unassigned)

References

Details

(Keywords: intermittent-failure)

Attachments

(2 files, 1 obsolete file)

blockUntilClear in toggleMasterPassword
8.09 KB, patch
jmaher
: review+
Details | Diff | Splinter Review
disable master password assertions
1.81 KB, patch
wesj
: review+
Details | Diff | Splinter Review 
https://tbpl.mozilla.org/php/getParsedLog.php?id=12664943&tree=Firefox
Android Tegra 250 mozilla-central opt test robocop on 2012-06-14 08:56:43 PDT for push 983b91e5aa17

slave: tegra-159

1 INFO TEST-START | testPasswordEncrypt
2 INFO TEST-PASS | testPasswordEncrypt | Found a content resolver - android.app.ContextImpl$ApplicationContentResolver@48520378 should not equal null
3 INFO TEST-PASS | testPasswordEncrypt | Insert returned correct uri - content://org.mozilla.fennec.db.passwords/passwords/1?profilePath=%2Fmnt%2Fsdcard%2Ftests%2Fprofile should equal content://org.mozilla.fennec.db.passwords/passwords/1?profilePath=%2Fmnt%2Fsdcard%2Ftests%2Fprofile
4 INFO TEST-PASS | testPasswordEncrypt | Username was encrypted correctly when inserting - username should equal username
5 INFO TEST-PASS | testPasswordEncrypt | Password was encrypted correctly when inserting - password should equal password
6 INFO TEST-PASS | testPasswordEncrypt | Password has correct encryption type - 1 should equal 1
7 INFO TEST-PASS | testPasswordEncrypt | Username was encrypted when updating - username2 should equal username2
8 INFO TEST-PASS | testPasswordEncrypt | Password was encrypted when updating - password2 should equal password2
9 INFO TEST-UNEXPECTED-FAIL | testPasswordEncrypt | Storing a password while MP was set should fail - got content://org.mozilla.fennec.db.passwords/passwords/2?profilePath=%2Fmnt%2Fsdcard%2Ftests%2Fprofile, expected null
10 INFO TEST-END | testPasswordEncrypt | finished in 13572ms
11 INFO TEST-START | Shutdown
12 INFO Passed: 7
13 INFO Failed: 1
14 INFO Todo: 0
I have not been able to reproduce this locally to debug. 

I wonder if toggleMasterPassword() is failing, or if it somehow hasn't taken effect before the storage attempt is made.
Assignee: nobody → gbrown
Other Fennec code gets and sets preferences and it seems possible that toggleMasterPassword might unblock on a notification for another preference. The current test framework does not allow for verification of the data associated with an event that we have blocked on (I have seen a bug/patch for that somewhere). Even if we could see the event data, we could not definitively associate the Pref:Data message with our request - bug 753312.

However, in renewed testing, I was able to reproduce this once with additional logging and saw this failure occur where we unblocked on a Pref:Data for the masterpassword.enabled and that occurred after browser.js had executed setPassword and updatePref -- suggesting that there is another way for this to fail.
It's difficult to address the concerns in comment 22 without major changes to our framework, but here's a simpler way forward that may help: Use the existing RepeatedEventExpecter interface and blockUntilClear to wait not only for Pref:Data, but for a period of 2 seconds during which no additional Pref:Data messages are received.

I have also tried to improve the exception reporting, in case a caught-and-ignored exception is playing a part.
Attachment #650942 - Flags: review?(jmaher)
Sorry - attached the wrong patch!
Attachment #650942 - Attachment is obsolete: true
Attachment #650942 - Flags: review?(jmaher)
Attachment #650943 - Flags: review?(jmaher)
Comment on attachment 650943 [details] [diff] [review]
blockUntilClear in toggleMasterPassword

Review of attachment 650943 [details] [diff] [review]:
-----------------------------------------------------------------

always a fun hack in robocop.
Attachment #650943 - Flags: review?(jmaher) → review+
Adding dependency on bug 753312 so that hopefully we can remove the hack once that is fixed.
Depends on: 753312
https://hg.mozilla.org/mozilla-central/rev/7084761aa77b
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 17
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
fwiw, when this test succeeds, the request fails with:

GeckoJNI: Throwing error: PK11SDR_Encrypt returned error -8152: The key does not support the requested operation.

and the C call stack is:

PK11SDR_Encrypt
PK11_CreateContextBySymKey
pk11_CreateNewContextInSlot
pk11_context_init (CKA_ENCRYPT) returns crv == 96 (0x60)


This does not appear to be tightly coupled to the operation of setting the MP pref, so I am suspicious of a race condition arising...but you would think the 2000 ms delay introduced by the last patch would deal with that.

:wesj is trying to land patches in another bug to give us better feedback regarding the pref setting.
These intermittent failures have gone on for too long. I still aspire to update the robocop infra to understand event data, so that we can try to wait for the pref change better...but I'm not finding time for that, and I am not sure that will resolve the failures. So here's a patch that disables the failing assertions with big TODO comments pointing back to this bug. My plan is to [leave open] and re-visit the event data/pref change issue when time allows.
Attachment #683626 - Flags: review?(wjohnston)
Attachment #683626 - Flags: review?(wjohnston) → review+
Whiteboard: [orange] [leave open] → [leave open]
The robocop infra changes referenced in Comment 131 have been made now, and seem to allow for re-enabling the code disabled in this patch, except for bug 824067.

https://tbpl.mozilla.org/?tree=Try&rev=f537f6967f52

I don't want to make changes here until the bigger issue is addressed.
Assignee: gbrown → nobody
Keywords: leave-open
Whiteboard: [leave open]
Bulk assigning P3 to all open intermittent bugs without a priority set in Firefox components per bug 1298978.
Priority: -- → P3
The leave-open keyword is there and there is no activity for 6 months.
:sdaswani, maybe it's time to close this bug?
Flags: needinfo?(sdaswani)
Status: REOPENED → RESOLVED
Closed: 11 years ago → 5 years ago
Flags: needinfo?(sdaswani)
Resolution: --- → INVALID
Invalid would only be the correct resolution if the test filled with comment-out parts was removed from the tree. The pointless "resolution" you're looking for that means "we can't be bothered to fix this test, and just want housekeeping bots to not bother us" is inactive.
Resolution: INVALID → INACTIVE
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.