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

RESOLVED FIXED in Firefox 50

Status

()

Firefox for Android
Awesomescreen
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: ahunt, Assigned: ahunt)

Tracking

Trunk
Firefox 50
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 verified)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

2 years ago
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)

Updated

2 years ago
Assignee: nobody → ahunt
(Assignee)

Comment 1

2 years ago
Created attachment 8761853 [details]
Bug 1279332 - Backed out 61442f7ad442 for breaking background runnables

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)
(Assignee)

Comment 2

2 years ago
Created attachment 8761854 [details]
Bug 1279332 - Backed out 97074800423c since it depends on a commit breaking background runnables

Review commit: https://reviewboard.mozilla.org/r/58896/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/58896/
(Assignee)

Comment 3

2 years ago
Created attachment 8761856 [details]
Bug 1279332 - Post: remove unrelated whitespace to pacify checkstyle

Review commit: https://reviewboard.mozilla.org/r/58898/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/58898/

Comment 4

2 years ago
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 5

2 years ago
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+

Comment 6

2 years ago
Changes above should also fix Bug 1279331 and Bug 1279338.

Updated

2 years ago
Duplicate of this bug: 1279338

Updated

2 years ago
Duplicate of this bug: 1279331
(Assignee)

Comment 9

2 years ago
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
Backed out for various Android robocop test failures:

https://hg.mozilla.org/integration/fx-team/rev/6bbfe13ae5981d877f72b1d1fda6031d432ca62f
https://hg.mozilla.org/integration/fx-team/rev/64c94b98e1ba0581af5b613e9ce06d2c96f9fca3
https://hg.mozilla.org/integration/fx-team/rev/e6ea38f03d27a38f58f50a7dd938e26ec3d8eac8

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=59fbde27dc99259f88e3dddce64707b2884eefff
Flags: needinfo?(ahunt)
Duplicate of this bug: 1279338
Blocks: 1279468
Blocks: 1279444
Blocks: 1279338
Blocks: 1279411
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
(Assignee)

Comment 13

2 years ago
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)
(Assignee)

Comment 14

2 years ago
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/
(Assignee)

Updated

2 years ago
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+
(Assignee)

Comment 17

2 years ago
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
Depends on: 1279488
(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
Duplicate of this bug: 1279444
Duplicate of this bug: 1279468
Duplicate of this bug: 1279411

Comment 22

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c97ec840ee01
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox50: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
(Assignee)

Updated

a year ago
Flags: needinfo?(ahunt)
Verified in 50.1.0 using LG G4 (Android 5.1)
status-firefox50: fixed → verified
You need to log in before you can comment on or make changes to this bug.