Closed Bug 1279332 Opened 8 years ago Closed 8 years ago

Background threads do not run anymore after onPrepareOptionsMenu() has been called

Categories

(Firefox for Android Graveyard :: Awesomescreen, defect)

defect
Not set
normal

Tracking

(firefox50 verified)

RESOLVED FIXED
Firefox 50
Tracking Status
firefox50 --- verified

People

(Reporter: ahunt, Assigned: ahunt)

References

Details

Attachments

(2 files, 1 obsolete file)

1) select URL bar
2) type search term
3) press "Go" / "Enter" / right-arrow / equivalent

Expected: search page in your default search engine is opened
Actual: currently opened webpage reappears, search term is visible in URL bar.
Assignee: nobody → ahunt
We're now calling setIntent on the background thread. The onTargetSelected callback expects
to be run on the UI thread, and it looks like Android hangs up when we call UI methods
on the background thread. This in turn means the entire background thread gets locked up
(we execute tasks on the background thread sequentially) and no further background runnables
are executed. This breaks anything else requiring use of the background thread, e.g.
favicon generation, search, permissions doorhangers, etc.

Review commit: https://reviewboard.mozilla.org/r/58894/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/58894/
Attachment #8761853 - Flags: review?(gkruglov)
Attachment #8761854 - Flags: review?(gkruglov)
Comment on attachment 8761853 [details]
Bug 1279332 - Backed out 61442f7ad442 for breaking background runnables

https://reviewboard.mozilla.org/r/58894/#review55798
Attachment #8761853 - Flags: review?(gkruglov) → review+
Comment on attachment 8761854 [details]
Bug 1279332 - Backed out 97074800423c since it depends on a commit breaking background runnables

https://reviewboard.mozilla.org/r/58896/#review55800

::: mobile/android/base/java/org/mozilla/gecko/widget/GeckoActionProvider.java:57
(Diff revision 1)
>       * When setting a provider, the activity can listen to this,
>       * to close the menu.
> +     *
> +     * Is run on the UI thread.
>       */
>      public interface OnTargetSelectedListener {

Perhaps use android.support.annotation.UiThread?
Attachment #8761854 - Flags: review?(gkruglov) → review+
Changes above should also fix Bug 1279331 and Bug 1279338.
https://hg.mozilla.org/integration/fx-team/rev/50b1d3506786c0d2b07b686f157af8e828e38c11
Bug 1279332 - Perform UI callback on UI thread to avoid crashing the background thread r=grisha

https://hg.mozilla.org/integration/fx-team/rev/6a35c7a791a6a317204c9d144eac4ed902de3f86
Bug 1279332 - Document that onTargetSelected should be run on the UI thread r=grisha

https://hg.mozilla.org/integration/fx-team/rev/59fbde27dc99259f88e3dddce64707b2884eefff
Bug 1279332 - Post: remove unrelated whitespace to pacify checkstyle r=me
I assume this was caused by bug 1264489?
Blocks: 1264489
Summary: Pressing "Go" / "Enter" / etc on (software) keyboard does not perform search → Background threads do not run anymore after onPrepareOptionsMenu() has been called
Comment on attachment 8761853 [details]
Bug 1279332 - Backed out 61442f7ad442 for breaking background runnables

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/58894/diff/1-2/
Attachment #8761853 - Attachment description: Bug 1279332 - Perform UI callback on UI thread to avoid crashing the background thread → Bug 1279332 - Backed out 61442f7ad442 for breaking background runnables
Attachment #8761854 - Attachment description: Bug 1279332 - Document that onTargetSelected should be run on the UI thread → Bug 1279332 - Backed out 97074800423c since it depends on a commit breaking background runnables
Attachment #8761853 - Flags: review+ → review?(s.kaspari)
Attachment #8761854 - Flags: review+ → review?(s.kaspari)
Comment on attachment 8761854 [details]
Bug 1279332 - Backed out 97074800423c since it depends on a commit breaking background runnables

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/58896/diff/1-2/
Attachment #8761856 - Attachment is obsolete: true
Comment on attachment 8761853 [details]
Bug 1279332 - Backed out 61442f7ad442 for breaking background runnables

https://reviewboard.mozilla.org/r/58894/#review55840
Attachment #8761853 - Flags: review?(s.kaspari) → review+
Comment on attachment 8761854 [details]
Bug 1279332 - Backed out 97074800423c since it depends on a commit breaking background runnables

https://reviewboard.mozilla.org/r/58896/#review55842
Attachment #8761854 - Flags: review?(s.kaspari) → review+
https://hg.mozilla.org/integration/fx-team/rev/c97ec840ee011a41f076e3d6c8543770445628d5
Bug 1279332 - Backed out 61442f7ad442 for breaking background runnables r=sebastian

https://hg.mozilla.org/integration/fx-team/rev/4fb66e8edacad1ab21a6b52d6ef8c0f078728090
Bug 1279332 - Backed out 97074800423c since it depends on a commit breaking background runnables r=sebastian
(In reply to Andrzej Hunt :ahunt from comment #17)
> https://hg.mozilla.org/integration/fx-team/rev/
> c97ec840ee011a41f076e3d6c8543770445628d5
> Bug 1279332 - Backed out 61442f7ad442 for breaking background runnables
> r=sebastian
> 
> https://hg.mozilla.org/integration/fx-team/rev/
> 4fb66e8edacad1ab21a6b52d6ef8c0f078728090
> Bug 1279332 - Backed out 97074800423c since it depends on a commit breaking
> background runnables r=sebastian

backed this also directly out from central and will retrigger nightlys
https://hg.mozilla.org/mozilla-central/rev/c97ec840ee01
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Flags: needinfo?(ahunt)
Verified in 50.1.0 using LG G4 (Android 5.1)
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: