https://tbpl.mozilla.org/php/getParsedLog.php?id=28103252&tree=Mozilla-Aurora Android 2.2 Tegra mozilla-aurora opt test robocop-1 on 2013-09-19 10:58:26 PDT for push ad33a1cc4aab slave: tegra-357 0 INFO SimpleTest START 1 INFO TEST-START | testLinkContextMenu EventExpecter: no longer listening for Gecko:Ready 2 INFO TEST-PASS | testLinkContextMenu | waiting for urlbar text to gain focus - urlbar text gained focus 3 INFO TEST-PASS | testLinkContextMenu | URL typed properly - http://mochi.test:8888/tests/robocop/robocop_big_link.html should equal http://mochi.test:8888/tests/robocop/robocop_big_link.html EventExpecter: no longer listening for DOMContentLoaded long-clicking at 512.0, 103.0 4 INFO TEST-PASS | testLinkContextMenu | Checking that the option: Open Link in New Tab is available - The option is available 5 INFO TEST-PASS | testLinkContextMenu | Checking that the option: Open Link in Private Tab is available - The option is available 6 INFO TEST-PASS | testLinkContextMenu | Checking that the option: Copy Link is available - The option is available 7 INFO TEST-PASS | testLinkContextMenu | Checking that the option: Share Link is available - The option is available 8 INFO TEST-PASS | testLinkContextMenu | Checking that the option: Bookmark Link is available - The option is available EventExpecter: no longer listening for Tab:Added 9 INFO TEST-PASS | testLinkContextMenu | The correct number of tabs are opened - 2 should equal 2 long-clicking at 512.0, 103.0 EventExpecter: no longer listening for Tab:Added 10 INFO TEST-PASS | testLinkContextMenu | The correct number of tabs are opened - 2 should equal 2 long-clicking at 512.0, 103.0 Clipboard text = , expected text = http://mochi.test:8888/tests/robocop/robocop_blank_01.html Clipboard text = http://mochi.test:8888/tests/robocop/robocop_blank_01.html , expected text = http://mochi.test:8888/tests/robocop/robocop_blank_01.html 11 INFO TEST-PASS | testLinkContextMenu | Checking if the text is correctly copied - The text was correctly copied long-clicking at 512.0, 103.0 12 INFO TEST-PASS | testLinkContextMenu | Checking that the share pop-up is displayed - The pop-up has been displayed long-clicking at 512.0, 103.0 13 INFO TEST-PASS | testLinkContextMenu | Waiting for the Bookmark added toaster notification - The notification has been displayed 14 INFO TEST-UNEXPECTED-FAIL | testLinkContextMenu | Checking if the link has been added as a bookmark - The link has been bookmarked Exception caught during test! junit.framework.AssertionFailedError: 14 INFO TEST-UNEXPECTED-FAIL | testLinkContextMenu | Checking if the link has been added as a bookmark - The link has been bookmarked at junit.framework.Assert.fail(Assert.java:47) at org.mozilla.fennec_aurora.FennecMochitestAssert._logMochitestResult(FennecMochitestAssert.java:111) at org.mozilla.fennec_aurora.FennecMochitestAssert.ok(FennecMochitestAssert.java:140) at org.mozilla.fennec_aurora.tests.ContentContextMenuTest.verifyBookmarkLinkOption(ContentContextMenuTest.java:91) at org.mozilla.fennec_aurora.tests.testLinkContextMenu.testLinkContextMenu(testLinkContextMenu.java:32) at java.lang.reflect.Method.invokeNative(Native Method) at java.lang.reflect.Method.invoke(Method.java:521) at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:204) at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:194) at android.test.ActivityInstrumentationTestCase2.runTest(ActivityInstrumentationTestCase2.java:186) at org.mozilla.fennec_aurora.tests.BaseTest.runTest(BaseTest.java:149) at junit.framework.TestCase.runBare(TestCase.java:127) at junit.framework.TestResult$1.protect(TestResult.java:106) at junit.framework.TestResult.runProtected(TestResult.java:124) at junit.framework.TestResult.run(TestResult.java:109) at junit.framework.TestCase.run(TestCase.java:118) at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:169) at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:154) at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:520) at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1447) 15 INFO TEST-UNEXPECTED-FAIL | testLinkContextMenu | Exception caught - junit.framework.AssertionFailedError: 14 INFO TEST-UNEXPECTED-FAIL | testLinkContextMenu | Checking if the link has been added as a bookmark - The link has been bookmarked 16 INFO TEST-END | testLinkContextMenu | finished in 79162ms 17 INFO TEST-START | Shutdown 18 INFO Passed: 12 19 INFO Failed: 2 20 INFO Todo: 0 21 INFO SimpleTest FINISHED

This failure is seen intermittently on Android 2.3: https://tbpl.mozilla.org/php/getParsedLog.php?id=36171063&tree=Ash&full=1 16:59:05 INFO - 0 INFO SimpleTest START 16:59:05 INFO - 1 INFO TEST-START | testLinkContextMenu 16:59:05 INFO - 2 INFO TEST-PASS | testLinkContextMenu | Given message occurred for registered event: {"type":"Gecko:Ready"} - Gecko:Ready should equal Gecko:Ready 16:59:05 INFO - EventExpecter: no longer listening for Gecko:Ready 16:59:05 INFO - 3 INFO TEST-PASS | testLinkContextMenu | waiting for urlbar text to gain focus - urlbar text gained focus 16:59:05 INFO - 4 INFO TEST-PASS | testLinkContextMenu | URL typed properly - http://mochi.test:8888/tests/robocop/robocop_big_link.html should equal http://mochi.test:8888/tests/robocop/robocop_big_link.html 16:59:05 INFO - 5 INFO TEST-PASS | testLinkContextMenu | Given message occurred for registered event: {"errorType":"","bgColor":"transparent","type":"DOMContentLoaded","tabID":0} - DOMContentLoaded should equal DOMContentLoaded 16:59:05 INFO - EventExpecter: no longer listening for DOMContentLoaded 16:59:05 INFO - long-clicking at 512.0, 103.0 16:59:05 INFO - 6 INFO TEST-PASS | testLinkContextMenu | Checking that the option: Open Link in New Tab is available - The option is available 16:59:05 INFO - 7 INFO TEST-PASS | testLinkContextMenu | Checking that the option: Open Link in Private Tab is available - The option is available 16:59:05 INFO - 8 INFO TEST-PASS | testLinkContextMenu | Checking that the option: Copy Link is available - The option is available 16:59:05 INFO - 9 INFO TEST-PASS | testLinkContextMenu | Checking that the option: Share Link is available - The option is available 16:59:05 INFO - 10 INFO TEST-PASS | testLinkContextMenu | Checking that the option: Bookmark Link is available - The option is available 16:59:05 INFO - 11 INFO TEST-PASS | testLinkContextMenu | Given message occurred for registered event: {"parentId":0,"delayLoad":false,"title":"http:\/\/mochi.test:8888\/tests\/robocop\/robocop_blank_01.html","selected":false,"isPrivate":false,"stub":false,"external":false,"desktopMode":false,"tabID":1,"type":"Tab:Added","uri":"http:\/\/mochi.test:8888\/tests\/robocop\/robocop_blank_01.html"} - Tab:Added should equal Tab:Added 16:59:05 INFO - EventExpecter: no longer listening for Tab:Added 16:59:05 INFO - 12 INFO TEST-PASS | testLinkContextMenu | The correct number of tabs are opened - 2 should equal 2 16:59:05 INFO - long-clicking at 512.0, 103.0 16:59:05 INFO - 13 INFO TEST-PASS | testLinkContextMenu | Given message occurred for registered event: {"parentId":0,"delayLoad":false,"title":"http:\/\/mochi.test:8888\/tests\/robocop\/robocop_blank_01.html","selected":false,"isPrivate":true,"stub":false,"external":false,"desktopMode":false,"tabID":2,"type":"Tab:Added","uri":"http:\/\/mochi.test:8888\/tests\/robocop\/robocop_blank_01.html"} - Tab:Added should equal Tab:Added 16:59:05 INFO - EventExpecter: no longer listening for Tab:Added 16:59:05 INFO - 14 INFO TEST-PASS | testLinkContextMenu | The correct number of tabs are opened - 2 should equal 2 16:59:05 INFO - long-clicking at 512.0, 103.0 16:59:05 INFO - Clipboard text = , expected text = http://mochi.test:8888/tests/robocop/robocop_blank_01.html 16:59:05 INFO - Clipboard text = , expected text = http://mochi.test:8888/tests/robocop/robocop_blank_01.html 16:59:05 INFO - Clipboard text = , expected text = http://mochi.test:8888/tests/robocop/robocop_blank_01.html 16:59:05 INFO - Clipboard text = , expected text = http://mochi.test:8888/tests/robocop/robocop_blank_01.html 16:59:05 INFO - Clipboard text = , expected text = http://mochi.test:8888/tests/robocop/robocop_blank_01.html 16:59:05 INFO - Clipboard text = http://mochi.test:8888/tests/robocop/robocop_blank_01.html , expected text = http://mochi.test:8888/tests/robocop/robocop_blank_01.html 16:59:05 INFO - 15 INFO TEST-PASS | testLinkContextMenu | Checking if the text is correctly copied - The text was correctly copied 16:59:05 INFO - long-clicking at 512.0, 103.0 16:59:05 INFO - 16 INFO TEST-PASS | testLinkContextMenu | Checking that the share pop-up is displayed - The pop-up has been displayed 16:59:05 INFO - long-clicking at 512.0, 103.0 16:59:05 INFO - 17 INFO TEST-PASS | testLinkContextMenu | Waiting for the Bookmark added toaster notification - The notification has been displayed 16:59:05 INFO - 18 INFO TEST-UNEXPECTED-FAIL | testLinkContextMenu | Checking if the link has been added as a bookmark - The link has been bookmarked 16:59:05 INFO - Exception caught during test! 16:59:05 INFO - junit.framework.AssertionFailedError: 18 INFO TEST-UNEXPECTED-FAIL | testLinkContextMenu | Checking if the link has been added as a bookmark - The link has been bookmarked 16:59:05 INFO - at junit.framework.Assert.fail(Assert.java:47) 16:59:05 INFO - at org.mozilla.gecko.FennecMochitestAssert._logMochitestResult(FennecMochitestAssert.java:110) 16:59:05 INFO - at org.mozilla.gecko.FennecMochitestAssert.ok(FennecMochitestAssert.java:139) 16:59:05 INFO - at org.mozilla.gecko.tests.ContentContextMenuTest.verifyBookmarkLinkOption(ContentContextMenuTest.java:82) 16:59:05 INFO - at org.mozilla.gecko.tests.testLinkContextMenu.testLinkContextMenu(testLinkContextMenu.java:31) 16:59:05 INFO - at java.lang.reflect.Method.invokeNative(Native Method) 16:59:05 INFO - at java.lang.reflect.Method.invoke(Method.java:507) 16:59:05 INFO - at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:204) 16:59:05 INFO - at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:194) 16:59:05 INFO - at android.test.ActivityInstrumentationTestCase2.runTest(ActivityInstrumentationTestCase2.java:186) 16:59:05 INFO - at org.mozilla.gecko.tests.BaseTest.runTest(BaseTest.java:146) 16:59:05 INFO - at junit.framework.TestCase.runBare(TestCase.java:127) 16:59:05 INFO - at junit.framework.TestResult$1.protect(TestResult.java:106) 16:59:05 INFO - at junit.framework.TestResult.runProtected(TestResult.java:124) 16:59:05 INFO - at junit.framework.TestResult.run(TestResult.java:109) 16:59:05 INFO - at junit.framework.TestCase.run(TestCase.java:118) 16:59:05 INFO - at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:169) 16:59:05 INFO - at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:154) 16:59:05 INFO - at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:529) 16:59:05 INFO - at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1448) 16:59:05 INFO - 19 INFO TEST-UNEXPECTED-FAIL | testLinkContextMenu | Exception caught - junit.framework.AssertionFailedError: 18 INFO TEST-UNEXPECTED-FAIL | testLinkContextMenu | Checking if the link has been added as a bookmark - The link has been bookmarked 16:59:05 INFO - 20 INFO TEST-END | testLinkContextMenu | finished in 122755ms 16:59:05 INFO - 21 INFO TEST-START | Shutdown 16:59:05 INFO - 22 INFO Passed: 16 16:59:05 WARNING - 23 INFO Failed: 2 16:59:05 WARNING - One or more unittests failed. 16:59:05 INFO - 24 INFO Todo: 0 16:59:05 INFO - 25 INFO SimpleTest FINISHED 16:59:05 INFO - INFO | automation.py | Application ran for: 0:02:10.542731 16:59:05 INFO - 03-14 16:43:35.153 I/Robocop ( 1214): 15 INFO TEST-PASS | testLinkContextMenu | Checking if the text is correctly copied - The text was correctly copied 16:59:05 INFO - 03-14 16:43:35.212 W/InputManagerService( 64): Window already focused, ignoring focus gain of: com.android.internal.view.IInputMethodClient$Stub$Proxy@406de710 16:59:05 INFO - 03-14 16:43:36.232 I/Robocop ( 1214): long-clicking at 512.0, 103.0 16:59:05 INFO - 03-14 16:43:37.342 E/GeckoConsole( 1214): Sending message 16:59:05 INFO - 03-14 16:43:40.532 E/GeckoConsole( 1214): observe {"guid":"","button":3,"list":[3],"__guid__":"{700f1f34-67bb-4604-915a-927d4206a806}"} 16:59:05 INFO - 03-14 16:43:40.612 I/ActivityManager( 64): Starting: Intent { act=android.intent.action.CHOOSER flg=0x4000000 cmp=android/com.android.internal.app.ChooserActivity (has extras) } from pid 1214 16:59:05 INFO - 03-14 16:43:40.881 D/GeckoHealthRec( 1214): Recording session end: P 16:59:05 INFO - 03-14 16:43:41.092 V/GeckoHealthRec( 1214): Recorded session entry for env 1, current is 1 16:59:05 INFO - 03-14 16:43:41.132 D/GeckoSessInfo( 1214): Recording session done: 1394840513475 16:59:05 INFO - 03-14 16:43:41.272 D/GeckoBrowserProvider( 1214): Expiring history. 16:59:05 INFO - 03-14 16:43:41.292 D/GeckoBrowserProvider( 1214): Expiring thumbnails. 16:59:05 INFO - 03-14 16:43:41.303 W/InputManagerService( 64): Window already focused, ignoring focus gain of: com.android.internal.view.IInputMethodClient$Stub$Proxy@406a6a78 16:59:05 INFO - 03-14 16:43:41.782 D/ ( 1214): HostConnection::get() New Host Connection established 0x2fc890, tid 1227 16:59:05 INFO - 03-14 16:43:41.792 E/EGL_emulation( 1214): rcCreateWindowSurface returned 0 16:59:05 INFO - 03-14 16:43:41.792 E/EGL_emulation( 1214): tid 1227: eglCreatePbufferSurface(727): error 0x3003 (EGL_BAD_ALLOC) 16:59:05 INFO - 03-14 16:43:43.162 I/GeckoHealth( 1214): fennec :: HealthReportBroadcastService :: Registering HealthReportPruneService. 16:59:05 INFO - 03-14 16:43:43.183 I/GeckoHealth( 1214): fennec :: BackgroundService :: Setting inexact repeating alarm for interval 86400000 16:59:05 INFO - 03-14 16:43:43.722 I/ActivityManager( 64): Displayed android/com.android.internal.app.ChooserActivity: +2s608ms 16:59:05 INFO - 03-14 16:43:43.822 I/Robocop ( 1214): 16 INFO TEST-PASS | testLinkContextMenu | Checking that the share pop-up is displayed - The pop-up has been displayed 16:59:05 INFO - 03-14 16:43:44.443 W/KeyCharacterMap( 1214): No keyboard for id 0 16:59:05 INFO - 03-14 16:43:44.443 W/KeyCharacterMap( 1214): Using default keymap: /system/usr/keychars/qwerty.kcm.bin 16:59:05 INFO - 03-14 16:43:45.037 W/GeckoBatteryManager( 1214): Already started! 16:59:05 INFO - 03-14 16:43:45.117 D/GeckoSessInfo( 1214): Recording start of session: 1394840625118 16:59:05 INFO - 03-14 16:43:45.182 W/InputManagerService( 64): Window already focused, ignoring focus gain of: com.android.internal.view.IInputMethodClient$Stub$Proxy@408e3700 16:59:05 INFO - 03-14 16:43:45.901 I/Robocop ( 1214): long-clicking at 512.0, 103.0 16:59:05 INFO - 03-14 16:43:47.762 E/Profiler( 1214): BPUnw: [8 total] thread_unregister_for_profiling(me=0x2adb30) 16:59:05 INFO - 03-14 16:43:47.782 E/Profiler( 1214): BPUnw: [7 total] thread_unregister_for_profiling(me=0x29d558) 16:59:05 INFO - 03-14 16:43:47.812 E/GeckoConsole( 1214): Sending message 16:59:05 INFO - 03-14 16:43:50.524 E/GeckoConsole( 1214): observe {"guid":"","button":4,"list":[4],"__guid__":"{1c76f22f-177c-465f-997c-9dd7f98ac606}"} 16:59:05 INFO - 03-14 16:43:50.972 W/InputManagerService( 64): Window already focused, ignoring focus gain of: com.android.internal.view.IInputMethodClient$Stub$Proxy@4083cbf8 16:59:05 INFO - 03-14 16:43:51.592 I/Robocop ( 1214): 17 INFO TEST-PASS | testLinkContextMenu | Waiting for the Bookmark added toaster notification - The notification has been displayed 16:59:05 INFO - 03-14 16:43:51.662 I/Robocop ( 1214): 18 INFO TEST-UNEXPECTED-FAIL | testLinkContextMenu | Checking if the link has been added as a bookmark - The link has been bookmarked 16:59:05 INFO - 03-14 16:43:52.002 I/Robocop ( 1214): Exception caught during test! 16:59:05 INFO - 03-14 16:43:52.002 I/Robocop ( 1214): junit.framework.AssertionFailedError: 18 INFO TEST-UNEXPECTED-FAIL | testLinkContextMenu | Checking if the link has been added as a bookmark - The link has been bookmarked 16:59:05 INFO - 03-14 16:43:52.002 I/Robocop ( 1214): at junit.framework.Assert.fail(Assert.java:47) 16:59:05 INFO - 03-14 16:43:52.002 I/Robocop ( 1214): at org.mozilla.gecko.FennecMochitestAssert._logMochitestResult(FennecMochitestAssert.java:110) 16:59:05 INFO - 03-14 16:43:52.002 I/Robocop ( 1214): at org.mozilla.gecko.FennecMochitestAssert.ok(FennecMochitestAssert.java:139) 16:59:05 INFO - 03-14 16:43:52.002 I/Robocop ( 1214): at org.mozilla.gecko.tests.ContentContextMenuTest.verifyBookmarkLinkOption(ContentContextMenuTest.java:82) 16:59:05 INFO - 03-14 16:43:52.002 I/Robocop ( 1214): at org.mozilla.gecko.tests.testLinkContextMenu.testLinkContextMenu(testLinkContextMenu.java:31) 16:59:05 INFO - 03-14 16:43:52.002 I/Robocop ( 1214): at java.lang.reflect.Method.invokeNative(Native Method) 16:59:05 INFO - 03-14 16:43:52.002 I/Robocop ( 1214): at java.lang.reflect.Method.invoke(Method.java:507) 16:59:05 INFO - 03-14 16:43:52.002 I/Robocop ( 1214): at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:204) 16:59:05 INFO - 03-14 16:43:52.002 I/Robocop ( 1214): at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:194) 16:59:05 INFO - 03-14 16:43:52.002 I/Robocop ( 1214): at android.test.ActivityInstrumentationTestCase2.runTest(ActivityInstrumentationTestCase2.java:186) 16:59:05 INFO - 03-14 16:43:52.002 I/Robocop ( 1214): at org.mozilla.gecko.tests.BaseTest.runTest(BaseTest.java:146) 16:59:05 INFO - 03-14 16:43:52.002 I/Robocop ( 1214): at junit.framework.TestCase.runBare(TestCase.java:127) 16:59:05 INFO - 03-14 16:43:52.002 I/Robocop ( 1214): at junit.framework.TestResult$1.protect(TestResult.java:106) 16:59:05 INFO - 03-14 16:43:52.002 I/Robocop ( 1214): at junit.framework.TestResult.runProtected(TestResult.java:124) 16:59:05 INFO - 03-14 16:43:52.002 I/Robocop ( 1214): at junit.framework.TestResult.run(TestResult.java:109) 16:59:05 INFO - 03-14 16:43:52.002 I/Robocop ( 1214): at junit.framework.TestCase.run(TestCase.java:118) 16:59:05 INFO - 03-14 16:43:52.002 I/Robocop ( 1214): at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:169) 16:59:05 INFO - 03-14 16:43:52.002 I/Robocop ( 1214): at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:154) 16:59:05 INFO - 03-14 16:43:52.002 I/Robocop ( 1214): at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:529) 16:59:05 INFO - 03-14 16:43:52.002 I/Robocop ( 1214): at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1448) 16:59:05 INFO - 03-14 16:43:52.011 I/Robocop ( 1214): 19 INFO TEST-UNEXPECTED-FAIL | testLinkContextMenu | Exception caught - junit.framework.AssertionFailedError: 18 INFO TEST-UNEXPECTED-FAIL | testLinkContextMenu | Checking if the link has been added as a bookmark - The link has been bookmarked 16:59:05 INFO - 03-14 16:43:53.022 D/GeckoTransProvider( 1214): Cleaning up deleted records from bookmarks 16:59:05 INFO - 03-14 16:43:53.081 I/Robocop ( 1214): 20 INFO TEST-END | testLinkContextMenu | finished in 122755ms 16:59:05 INFO - 03-14 16:43:53.092 I/Robocop ( 1214): 21 INFO TEST-START | Shutdown 16:59:05 INFO - 03-14 16:43:53.092 I/Robocop ( 1214): 22 INFO Passed: 16 16:59:05 INFO - 03-14 16:43:53.102 I/Robocop ( 1214): 23 INFO Failed: 2 16:59:05 INFO - 03-14 16:43:53.102 I/Robocop ( 1214): 24 INFO Todo: 0 16:59:05 INFO - 03-14 16:43:53.112 I/Robocop ( 1214): 25 INFO SimpleTest FINISHED

I think the problem here is that we wait for the "Bookmark added" toast to appear, then check if the bookmark has been added. [1] However, the Toast occurs whether or not the bookmark was actually added, or the query has completed, leading to a race condition. [2] I'm not sure whether we want to only show the Toast to users after the query is a guaranteed success (which could be delayed and unintuitive to the user), or do some waiting in ContentContextMenuTest. [1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tests/ContentContextMenuTest.java?rev=d26a2463dc6b#106 [2]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoApp.java?rev=5d9621220437#544

Let's think about this a bit. Is the "right" solution the same as the "quick" solution? Sounds like the "quick" solution is to add some delay to the test. Is the "right" solution to show the toast after we know the bookmark was really saved? Is that simple? Can we flip the "post to thread" calls here: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoApp.java?rev=5d9621220437#544

(In reply to Mark Finkle (:mfinkle) from comment #9) > Can we flip the "post to thread" calls here: > https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoApp. > java?rev=5d9621220437#544 Do you mean: ThreadUtils.postToBackgroundThread(new Runnable() { @Override public void run() { // Ideally, we have BrowserDB.addBookmark return a boolean status of whether or not it passed, // and show an appropriate toast accordingly. BrowserDB.addBookmark(getContentResolver(), title, url); ThreadUtils.postToUiThread(new Runnable() { @Override public void run() { Toast.makeText(context, R.string.bookmark_added, Toast.LENGTH_SHORT).show(); } }); } }); Isn't this the same as showing the toast after we know the bookmark was saved (the solution you described as "right")?

It is indeed. Are you suggesting the "right" solution can be the "quick" solution? Well done. My work here is done.

try: https://tbpl.mozilla.org/?tree=Try&rev=67c1e339dd5d The failures are from bug 932208 (which has been backed out) - the changes here appear to pass.

Should we also make this change for the bookmark removal toast?

(In reply to Michael Comella (:mcomella) from comment #13) > Should we also make this change for the bookmark removal toast? Sounds OK to me, especially if we have the same race situation.

https://hg.mozilla.org/integration/fx-team/rev/8da1a657058f (In reply to Mark Finkle (:mfinkle) from comment #14) > (In reply to Michael Comella (:mcomella) from comment #13) > > Should we also make this change for the bookmark removal toast? > > Sounds OK to me, especially if we have the same race situation. bug 1024145

Please request Aurora and Beta approval on this patch when you get a chance.

Spoke with Ryan via IRC - since this changes user facing code, we're not uplifting to Beta.

Comment on attachment 8438572 [details] [diff] [review] Show a toast only after a bookmark is added and re-enable testLinkContextMenu. [Approval Request Comment] Bug caused by (feature/regressing bug #): N/A User impact if declined: We're trying to EOL Android 2.2 support in 32 which makes it preferable if the 2.3 emulators could provide test coverage for pre-ICS devices. This patch fixes this issue, but does not affect the user experience directly. Testing completed (on m-c, etc.): Local, try in comment 12 Risk to taking this patch (and alternatives if risky): Low - worst case, we prevent the "Bookmark:Insert" event, which will break bookmarking from web content (but not the java UI). However, it's a small reversal in the order in which we display the "Bookmark Added" toast, so this is unlikely. String or IDL/UUID changes made by this patch: None

(In reply to Michael Comella (:mcomella) from comment #19) > User impact if declined: > We're trying to EOL Android 2.2 support in 32 which makes it preferable if > the 2.3 emulators could provide test coverage for pre-ICS devices. This > patch fixes this issue, but does not affect the user experience directly. Does this change indirectly affect the user experience? If so, how? > Risk to taking this patch (and alternatives if risky): > Low - worst case, we prevent the "Bookmark:Insert" event, which will break > bookmarking from web content (but not the java UI). However, it's a small > reversal in the order in which we display the "Bookmark Added" toast, so > this is unlikely. Have you seen any issues since this landed on m-c? How can we determine if bookmarking is broken from Web content? Will this be an intermittent failure?

(In reply to Lawrence Mandel [:lmandel] from comment #20) > Does this change indirectly affect the user experience? If so, how? We show the "Bookmark Added" toast only after the bookmark has been added, rather than when the DB request has been initiated. Users will more or less see the same thing, though there may be a slight delay on devices that cannot access the DB quickly. > Have you seen any issues since this landed on m-c? No, but I haven't tested extensively. > How can we determine if bookmarking is broken from Web content? Long-press a link and select "Bookmark link". > Will this be an intermittent failure? I don't see any reason why it should be.

Comment on attachment 8438572 [details] [diff] [review] Show a toast only after a bookmark is added and re-enable testLinkContextMenu. Aurora approval granted.

remote: https://hg.mozilla.org/releases/mozilla-aurora/rev/75f28c247632

Note for the future: this would have been a good opportunity to use an AsyncTask and remove a level of nesting. AsyncTasks do some operation on the background thread, then update the UI accordingly. That's exactly what we're doing here.