Closed Bug 1624675 Opened 9 months ago Closed 8 months ago

Call onLoadRequest directly if we're already on the UI thread

Categories

(GeckoView :: General, enhancement, P1)

Unspecified
All
enhancement

Tracking

(firefox76 fixed)

RESOLVED FIXED
mozilla76
Tracking Status
firefox76 --- fixed

People

(Reporter: agi, Assigned: agi)

Details

(Whiteboard: [geckoview:m76])

Attachments

(10 files, 1 obsolete file)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

Looks like posting to the UI thread unconditionally causes some slowdowns: https://github.com/mozilla-mobile/fenix/issues/8782#issuecomment-603411945 because some unrelated UI code is stealing the thread before we have a chance to run onLoadRequest.

We should just call it synchronously if we're already on the UI thread: https://searchfox.org/mozilla-central/rev/c80fa7258c935223fe319c5345b58eae85d4c6ae/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java#1670-1682

Assignee: nobody → agi
Priority: -- → P1
Whiteboard: [geckoview:m76]

This flag allows embedders to know when a onLoadRequest call is generated by
the app or not. Embedders can use this to skip expensive checks in
app-generated onLoadRequest calls.

Attachment #9136080 - Attachment is obsolete: true
Pushed by asferro@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/287ebe964741
Don't post on UI thread if already on the UI thread. r=snorp
https://hg.mozilla.org/integration/autoland/rev/14ef2c4ea463
Remove unused UIAsyncTask. r=snorp
https://hg.mozilla.org/integration/autoland/rev/4b50bc44475c
Remove dead code from ThreadUtils.java. r=geckoview-reviewers,snorp
https://hg.mozilla.org/integration/autoland/rev/61f4496d0745
Remove obsolete code in AndroidGamepadManager. r=snorp
https://hg.mozilla.org/integration/autoland/rev/d10bf633fdac
Remove dead code in HardwareUtils. r=snorp
https://hg.mozilla.org/integration/autoland/rev/0588bd37cf89
Remove dead code around profiles. r=snorp
https://hg.mozilla.org/integration/autoland/rev/2e5e33501074
Remove ContextUtils and inline only caller. r=snorp
https://hg.mozilla.org/integration/autoland/rev/2df39ae63e81
Remove unused org.mozilla.gecko.sqlite r=snorp
https://hg.mozilla.org/integration/autoland/rev/839dc1384f95
Remove unused org.mozilla.gecko.permissions. r=snorp
https://hg.mozilla.org/integration/autoland/rev/42f9061d299a
Add isDirectNavigation to LoadRequest. r=snorp,droeh

Backed out 10 changesets (Bug 1624675) for lints failure

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=linting%2Copt%2Candroid%2Cgradle%2Ctests%2Csource-test-mozlint-android-lints%2Ca%28lints%29&fromchange=9bbf5c41c5ce4ddbf35c6c2a53f4dc7353c916de&tochange=ee7af5d355e930cbf603daab0b323b4f0c1d5c35&selectedJob=295130064

Backout link: https://hg.mozilla.org/integration/autoland/rev/ee7af5d355e930cbf603daab0b323b4f0c1d5c35

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=295130064&repo=autoland&lineNumber=1200

...
[task 2020-03-27T21:39:40.273Z] BUILD SUCCESSFUL in 2m 32s
[task 2020-03-27T21:39:40.274Z] 173 actionable tasks: 158 executed, 15 up-to-date
[task 2020-03-27T21:39:40.275Z] Packaging quitter@mozilla.org.xpi...
[task 2020-03-27T21:39:40.433Z] Automation steps completed.
[task 2020-03-27T21:39:40.451Z] 0 compiler warnings present.
[task 2020-03-27T21:39:40.530Z] Overall system resources - Wall time: 302s; CPU: 0%; Read bytes: 0; Write bytes: 0; Read time: 0; Write time: 0
[task 2020-03-27T21:39:40.619Z] Notification center failed: Install notify-send (usually part of the libnotify package) to get a notification when the build finishes.
[task 2020-03-27T21:39:40.620Z] To view resource usage of the build, run |mach resource-usage|.
[task 2020-03-27T21:39:40.620Z] Your build was successful!
[task 2020-03-27T21:39:40.620Z] For more information on what to do now, see https://developer.mozilla.org/docs/Developer_Guide/So_You_Just_Built_Firefox
[task 2020-03-27T21:39:40.645Z] + ./mach --log-no-times lint -f treeherder -f json:/builds/worker/mozlint.json --linter android-api-lint --linter android-javadoc --linter android-checkstyle --linter android-lint --linter android-test
[task 2020-03-27T21:42:50.771Z] Traceback (most recent call last):
[task 2020-03-27T21:42:50.771Z]   File "/builds/worker/checkouts/gecko/python/mozlint/mozlint/roller.py", line 60, in _run_worker
[task 2020-03-27T21:42:50.771Z]     res = func(paths, config, **lintargs) or []
[task 2020-03-27T21:42:50.771Z]   File "/builds/worker/checkouts/gecko/python/mozlint/mozlint/types.py", line 54, in __call__
[task 2020-03-27T21:42:50.771Z]     return self._lint(paths, config, **lintargs)
[task 2020-03-27T21:42:50.771Z]   File "/builds/worker/checkouts/gecko/python/mozlint/mozlint/types.py", line 159, in _lint
[task 2020-03-27T21:42:50.771Z]     return func(config, **lintargs)
[task 2020-03-27T21:42:50.771Z]   File "/builds/worker/checkouts/gecko/tools/lint/android/lints.py", line 289, in test
[task 2020-03-27T21:42:50.772Z]     report_dir=report_dir))
[task 2020-03-27T21:42:50.772Z]   File "/builds/worker/checkouts/gecko/tools/lint/android/lints.py", line 223, in _parse_android_test_results
[task 2020-03-27T21:42:50.772Z]     raise RuntimeError('No reports found under {}'.format(report_dir))
[task 2020-03-27T21:42:50.772Z] RuntimeError: No reports found under /builds/worker/checkouts/gecko/obj-firefox/gradle/build/mobile/android/geckoview/test-results/testWithGeckoBinariesDebugUnitTest
[task 2020-03-27T21:42:50.775Z] No lint issues found.
[fetches 2020-03-27T21:42:50.815Z] removing /builds/worker/fetches
[fetches 2020-03-27T21:42:53.416Z] finished
Flags: needinfo?(agi)
Pushed by asferro@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e6b95c075c16
Don't post on UI thread if already on the UI thread. r=snorp
https://hg.mozilla.org/integration/autoland/rev/8ec7f988f0d1
Remove unused UIAsyncTask. r=snorp
https://hg.mozilla.org/integration/autoland/rev/b0aab18b5713
Remove dead code from ThreadUtils.java. r=geckoview-reviewers,snorp
https://hg.mozilla.org/integration/autoland/rev/e0fa24bb0550
Remove obsolete code in AndroidGamepadManager. r=snorp
https://hg.mozilla.org/integration/autoland/rev/714d51af18ac
Remove dead code in HardwareUtils. r=snorp
https://hg.mozilla.org/integration/autoland/rev/8a7426a75293
Remove dead code around profiles. r=snorp
https://hg.mozilla.org/integration/autoland/rev/474dcd599498
Remove ContextUtils and inline only caller. r=snorp
https://hg.mozilla.org/integration/autoland/rev/030ce751e8f1
Remove unused org.mozilla.gecko.sqlite r=snorp
https://hg.mozilla.org/integration/autoland/rev/a0915aad4326
Remove unused org.mozilla.gecko.permissions. r=snorp
https://hg.mozilla.org/integration/autoland/rev/fb9ae33ad7e0
Add isDirectNavigation to LoadRequest. r=snorp,droeh
Flags: needinfo?(agi)
You need to log in before you can comment on or make changes to this bug.