Closed Bug 1454441 Opened 6 years ago Closed 6 years ago

Make remote debugging a runtime setting

Categories

(GeckoView :: General, enhancement, P2)

All
Android
enhancement

Tracking

(firefox61 fixed)

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: jchen, Assigned: jchen)

References

Details

Attachments

(4 files)

We don't really have the ability to control remote debugging on a per-window basis, so remote debugging is really a runtime setting.
Comment on attachment 8968977 [details]
Bug 1454441 - 3. Move remote debugging setting to runtime;

Needs two reviewers for an API change.
Attachment #8968977 - Flags: review?(snorp)
Comment on attachment 8968977 [details]
Bug 1454441 - 3. Move remote debugging setting to runtime;

https://reviewboard.mozilla.org/r/237680/#review243440
Attachment #8968977 - Flags: review?(snorp) → review+
Comment on attachment 8968975 [details]
Bug 1454441 - 1. Add GeckoViewUtils.addLazyPrefObserver;

https://reviewboard.mozilla.org/r/237676/#review243474

::: mobile/android/modules/geckoview/GeckoViewUtils.jsm:198
(Diff revision 1)
> +   *                specified using the scope and name pair.
> +   * @param scope   See handler.
> +   * @param name    See handler.
> +   * @param once    If true, only observe the specified prefs once.
> +   */
> +  addLazyPrefObserver: function(prefs, {handler, scope, name, once}) {

a-prefix
Attachment #8968975 - Flags: review?(esawin) → review+
Comment on attachment 8968976 [details]
Bug 1454441 - 2. Move remote debugger usage to GeckoViewStartup;

https://reviewboard.mozilla.org/r/237678/#review243476
Attachment #8968976 - Flags: review?(esawin) → review+
Comment on attachment 8968977 [details]
Bug 1454441 - 3. Move remote debugging setting to runtime;

https://reviewboard.mozilla.org/r/237680/#review243478

::: mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntimeSettings.java:82
(Diff revision 1)
>          }
>  
>          /**
> +         * Set whether remote debugging support should be enabled.
> +         *
> +         * @param enabled True if remote debugging should be enabled.

Would be great to have consistent wording for Boolean settings (either way).

::: mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntimeSettings.java:84
(Diff revision 1)
>          /**
> +         * Set whether remote debugging support should be enabled.
> +         *
> +         * @param enabled True if remote debugging should be enabled.
> +         */
> +        public @NonNull Builder remoteDebuggingEnabled(final boolean enabled) {

I've used "flag" before for Boolean settings, would be great to make this consistent (either way).
Attachment #8968977 - Flags: review?(esawin) → review+
Comment on attachment 8968978 [details]
Bug 1454441 - 4. Fix Fennec remote debugging;

https://reviewboard.mozilla.org/r/237682/#review243480

Great find!

::: mobile/android/chrome/content/RemoteDebugger.js:21
(Diff revision 1)
> +    this._windowType = "navigator:browser";
> +
>      USBRemoteDebugger.init();
>      WiFiRemoteDebugger.init();
> +
> +    let listener = (event) => {

const
Attachment #8968978 - Flags: review?(esawin) → review+
Assignee: nobody → nchen
Status: NEW → ASSIGNED
Comment on attachment 8968977 [details]
Bug 1454441 - 3. Move remote debugging setting to runtime;

https://reviewboard.mozilla.org/r/237680/#review243478

> Would be great to have consistent wording for Boolean settings (either way).

I think I followed examples like [1] and [2]. I'll keep it the way it is until we have an internal consensus.

[1] https://developer.android.com/reference/android/view/View.html#setFocusable(boolean)
[2] https://developer.android.com/reference/android/view/View.html#getKeepScreenOn()
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s ee3189cd88c11beec1adc909a6989653b040bd91 -d 96867a07e1a3: rebasing 459368:ee3189cd88c1 "Bug 1454441 - 1. Add GeckoViewUtils.addLazyPrefObserver; r=esawin"
rebasing 459369:1d5bc3cf10f5 "Bug 1454441 - 2. Move remote debugger usage to GeckoViewStartup; r=esawin"
merging mobile/android/components/geckoview/GeckoViewStartup.js
warning: conflicts while merging mobile/android/components/geckoview/GeckoViewStartup.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fbcb2eee69fc
1. Add GeckoViewUtils.addLazyPrefObserver; r=esawin
https://hg.mozilla.org/integration/autoland/rev/99fa43f0686e
2. Move remote debugger usage to GeckoViewStartup; r=esawin
https://hg.mozilla.org/integration/autoland/rev/976d9622bf5c
3. Move remote debugging setting to runtime; r=esawin,snorp
https://hg.mozilla.org/integration/autoland/rev/d924aed50460
4. Fix Fennec remote debugging; r=esawin
Backed out 4 changesets (bug 1454441) for Android Mochitest failure on mobile/android/tests/browser/chrome/test_debugger_server.html. CLOSED TREE

Log of the failure:
https://treeherder.mozilla.org/logviewer.html#?job_id=174854714&repo=autoland&lineNumber=2060

 INFO -  35 INFO Tabs closed [+30s] | Resident Memory: 128221184
[task 2018-04-20T21:27:18.172Z] 21:27:18     INFO -  36 INFO Tabs closed [+30s, forced GC] | Resident Memory: 128225280
[task 2018-04-20T21:27:18.173Z] 21:27:18     INFO -  37 INFO PERFHERDER_DATA: {"framework": {"name": "awsy"}, "suites": [{"name": "Resident Memory", "subtests": [{"name": "Fresh start", "value": 120238080}, {"name": "Fresh start [+30s]", "value": 112754688}, {"name": "After tabs", "value": 170393600}, {"name": "After tabs [+30s]", "value": 172781568}, {"name": "After tabs [+30s, forced GC]", "value": 172781568}, {"name": "Tabs closed", "value": 151347200}, {"name": "Tabs closed [+30s]", "value": 128221184}, {"name": "Tabs closed [+30s, forced GC]", "value": 128225280}], "value": 142664714}]}
[task 2018-04-20T21:27:18.173Z] 21:27:18     INFO -  38 INFO TEST-PASS | mobile/android/tests/browser/chrome/test_awsy_lite.html | memory logging complete -- view results in Perfherder
[task 2018-04-20T21:27:18.173Z] 21:27:18     INFO -  39 INFO TEST-OK | mobile/android/tests/browser/chrome/test_awsy_lite.html | took 252861ms
[task 2018-04-20T21:27:18.174Z] 21:27:18     INFO -  40 INFO TEST-START | mobile/android/tests/browser/chrome/test_debugger_server.html
[task 2018-04-20T21:27:18.174Z] 21:27:18     INFO -  41 INFO TEST-UNEXPECTED-FAIL | mobile/android/tests/browser/chrome/test_debugger_server.html | uncaught exception - TypeError: aWindow is undefined at init@chrome://browser/content/RemoteDebugger.js:37:5
[task 2018-04-20T21:27:18.174Z] 21:27:18     INFO -  @chrome://mochitests/content/chrome/mobile/android/tests/browser/chrome/test_debugger_server.html:26:3
[task 2018-04-20T21:27:18.175Z] 21:27:18     INFO -  simpletestOnerror@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:1655:11
[task 2018-04-20T21:27:18.175Z] 21:27:18     INFO -  42 INFO TEST-OK | mobile/android/tests/browser/chrome/test_debugger_server.html | took 1590ms
[task 2018-04-20T21:27:18.175Z] 21:27:18     INFO -  43 INFO TEST-START | mobile/android/tests/browser/chrome/test_desktop_useragent.html
[task 2018-04-20T21:27:28.495Z] 21:27:28     INFO -  44 INFO TEST-OK | mobile/android/tests/browser/chrome/test_desktop_useragent.html | took 7574ms
[task 2018-04-20T21:27:28.496Z] 21:27:28     INFO -  45 INFO TEST-START | mobile/android/tests/browser/chrome/test_device_search_engine.html
[task 2018-04-20T21:27:28.496Z] 21:27:28     INFO -  46 INFO TEST-OK | mobile/android/tests/browser/chrome/test_device_search_engine.html | took 1167ms
[task 2018-04-20T21:27:28.496Z] 21:27:28     INFO -  47 INFO TEST-START | mobile/android/tests/browser/chrome/test_get_last_visited.html
[task 2018-04-20T21:27:38.827Z] 21:27:38     INFO -  48 INFO TEST-OK | mobile/android/tests/browser/chrome/test_get_last_visited.html | took 11489ms
[task 2018-04-20T21:27:38.827Z] 21:27:38     INFO -  49 INFO TEST-START | mobile/android/tests/browser/chrome/test_hidden_select_option.html
[task 2018-04-20T21:27:38.829Z] 21:27:38     INFO -  50 INFO TEST-OK | mobile/android/tests/browser/chrome/test_hidden_select_option.html | took 1608ms
[task 2018-04-20T21:27:49.148Z] 21:27:49     INFO -  51 INFO TEST-START | mobile/android/tests/browser/chrome/test_home_provider.html
[task 2018-04-20T21:27:49.151Z] 21:27:49     INFO -  52 INFO TEST-OK | mobile/android/tests/browser/chrome/test_home_provider.html | took 2348ms
[task 2018-04-20T21:27:49.151Z] 21:27:49     INFO -  53 INFO TEST-START | mobile/android/tests/browser/chrome/test_identity_mode.html
[task 2018-04-20T21:27:49.151Z] 21:27:49     INFO -  54 INFO TEST-OK | mobile/android/tests/browser/chrome/test_identity_mode.html | took 707ms
[task 2018-04-20T21:27:49.152Z] 21:27:49     INFO -  55 INFO TEST-START | mobile/android/tests/browser/chrome/test_media_playback.html
[task 2018-04-20T21:27:59.477Z] 21:27:59     INFO -  56 INFO TEST-OK | mobile/android/tests/browser/chrome/test_media_playback.html | took 13392ms
[task 2018-04-20T21:27:59.478Z] 21:27:59     INFO -  57 INFO TEST-START | mobile/android/tests/browser/chrome/test_migrate_ui.html
[task 2018-04-20T21:28:09.798Z] 21:28:09     INFO -  58 INFO TEST-OK | mobile/android/tests/browser/chrome/test_migrate_ui.html | took 2283ms
[task 2018-04-20T21:28:09.798Z] 21:28:09     INFO -  59 INFO TEST-START | mobile/android/tests/browser/chrome/test_network_manager.html
[task 2018-04-20T21:28:09.798Z] 21:28:09     INFO -  60 INFO TEST-OK | mobile/android/tests/browser/chrome/test_network_manager.html | took 655ms
[task 2018-04-20T21:28:09.799Z] 21:28:09     INFO -  61 INFO TEST-START | mobile/android/tests/browser/chrome/test_reader_view.html
[task 2018-04-20T21:28:20.127Z] 21:28:20     INFO -  62 INFO TEST-OK | mobile/android/tests/browser/chrome/test_reader_view.html | took 8763ms
[task 2018-04-20T21:28:20.128Z] 21:28:20     INFO -  63 INFO TEST-START | mobile/android/tests/browser/chrome/test_resource_substitutions.html
[task 2018-04-20T21:28:20.128Z] 21:28:20     INFO -  64 INFO TEST-OK | mobile/android/tests/browser/chrome/test_resource_substitutions.html | took 814ms
[task 2018-04-20T21:28:20.128Z] 21:28:20     INFO -  65 INFO TEST-START | mobile/android/tests/browser/chrome/test_restricted_profiles.html
[task 2018-04-20T21:28:20.128Z] 21:28:20     INFO -  66 INFO TEST-OK | mobile/android/tests/browser/chrome/test_restricted_profiles.html | took 920ms
[task 2018-04-20T21:28:20.128Z] 21:28:20     INFO -  67 INFO TEST-START | mobile/android/tests/browser/chrome/test_select_disabled.html
[task 2018-04-20T21:28:20.128Z] 21:28:20     INFO -  68 INFO TEST-OK | mobile/android/tests/browser/chrome/test_select_disabled.html | took 1411ms
[task 2018-04-20T21:28:20.128Z] 21:28:20     INFO -  69 INFO TEST-START | mobile/android/tests/browser/chrome/test_selectoraddtab.html
[task 2018-04-20T21:28:30.444Z] 21:28:30     INFO -  70 INFO TEST-OK | mobile/android/tests/browser/chrome/test_selectoraddtab.html | took 10085ms
[task 2018-04-20T21:28:30.444Z] 21:28:30     INFO -  71 INFO TEST-START | mobile/android/tests/browser/chrome/test_session_clear_history.html
[task 2018-04-20T21:29:01.602Z] 21:29:01     INFO -  72 INFO TEST-OK | mobile/android/tests/browser/chrome/test_session_clear_history.html | took 32286ms
[task 2018-04-20T21:29:12.016Z] 21:29:12     INFO -  73 INFO TEST-START | mobile/android/tests/browser/chrome/test_session_form_data.html
[task 2018-04-20T21:30:04.216Z] 21:30:04     INFO -  74 INFO TEST-OK | mobile/android/tests/browser/chrome/test_session_form_data.html | took 59208ms
[task 2018-04-20T21:30:04.217Z] 21:30:04     INFO -  75 INFO TEST-START | mobile/android/tests/browser/chrome/test_session_parentid.html
[task 2018-04-20T21:30:25.065Z] 21:30:25     INFO -  76 INFO TEST-OK | mobile/android/tests/browser/chrome/test_session_parentid.html | took 15178ms
[task 2018-04-20T21:30:25.066Z] 21:30:25     INFO -  77 INFO TEST-START | mobile/android/tests/browser/chrome/test_session_scroll_position.html
[task 2018-04-20T21:31:06.653Z] 21:31:06     INFO -  78 INFO TEST-OK | mobile/android/tests/browser/chrome/test_session_scroll_position.html | took 42467ms
[task 2018-04-20T21:31:06.653Z] 21:31:06     INFO -  79 INFO TEST-START | mobile/android/tests/browser/chrome/test_session_zombification.html
[task 2018-04-20T21:31:27.490Z] 21:31:27     INFO -  80 INFO TEST-OK | mobile/android/tests/browser/chrome/test_session_zombification.html | took 21885ms
[task 2018-04-20T21:31:27.490Z] 21:31:27     INFO -  81 INFO TEST-START | mobile/android/tests/browser/chrome/test_settings_fontinflation.html
[task 2018-04-20T21:32:09.181Z] 21:32:09     INFO -  82 INFO TEST-OK | mobile/android/tests/browser/chrome/test_settings_fontinflation.html | took 37454ms
[task 2018-04-20T21:32:09.181Z] 21:32:09     INFO -  83 INFO TEST-START | mobile/android/tests/browser/chrome/test_shared_preferences.html
[task 2018-04-20T21:32:09.185Z] 21:32:09     INFO -  84 INFO TEST-OK | mobile/android/tests/browser/chrome/test_shared_preferences.html | took 1841ms
[task 2018-04-20T21:32:09.185Z] 21:32:09     INFO -  85 INFO TEST-START | mobile/android/tests/browser/chrome/test_simple_discovery.html
[task 2018-04-20T21:32:09.186Z] 21:32:09     INFO -  86 INFO TEST-OK | mobile/android/tests/browser/chrome/test_simple_discovery.html | took 1059ms
[task 2018-04-20T21:32:09.186Z] 21:32:09     INFO -  87 INFO TEST-START | mobile/android/tests/browser/chrome/test_video_discovery.html
[task 2018-04-20T21:32:19.608Z] 21:32:19     INFO -  88 INFO TEST-OK | mobile/android/tests/browser/chrome/test_video_discovery.html | took 10134ms
[task 2018-04-20T21:32:19.609Z] 21:32:19     INFO -  89 INFO TEST-START | mobile/android/tests/browser/chrome/test_web_channel.html
[task 2018-04-20T21:32:30.032Z] 21:32:30     INFO -  90 INFO TEST-OK | mobile/android/tests/browser/chrome/test_web_channel.html | took 9847ms
[task 2018-04-20T21:32:30.032Z] 21:32:30     INFO -  91 INFO TEST-START | Shutdown
[task 2018-04-20T21:32:30.032Z] 21:32:30     INFO -  92 INFO Passed:  335
[task 2018-04-20T21:32:30.032Z] 21:32:30  WARNING -  93 INFO Failed:  1
[task 2018-04-20T21:32:30.032Z] 21:32:30  WARNING -  One or more unittests failed.
[task 2018-04-20T21:32:30.033Z] 21:32:30     INFO -  94 INFO Todo:    0
[task 2018-04-20T21:32:30.033Z] 21:32:30     INFO -  95 INFO Mode:    non-e10s
[task 2018-04-20T21:32:30.034Z] 21:32:30     INFO -  96 INFO Slowest: 252863ms - chrome://mochitests/content/chrome/mobile/android/tests/browser/chrome/test_awsy_lite.html
[task 2018-04-20T21:32:30.034Z] 21:32:30     INFO -  97 INFO SimpleTest FINISHED

Backout:
https://hg.mozilla.org/integration/autoland/rev/85d1752bed2ed20849993ae3669157cbd42a3ed8

Push with the failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=d924aed5046046f3530d1e3f24e60acab505f64c
Flags: needinfo?(nchen)
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fad6be573021
1. Add GeckoViewUtils.addLazyPrefObserver; r=esawin
https://hg.mozilla.org/integration/autoland/rev/deb0e12bfbd7
2. Move remote debugger usage to GeckoViewStartup; r=esawin
https://hg.mozilla.org/integration/autoland/rev/79fa5c2950a3
3. Move remote debugging setting to runtime; r=esawin,snorp
https://hg.mozilla.org/integration/autoland/rev/d4fcd301a168
4. Fix Fennec remote debugging; r=esawin
Flags: needinfo?(nchen)
I believe the lint failure is from bug 1445798.
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/debcd27d2476
1. Add GeckoViewUtils.addLazyPrefObserver; r=esawin
https://hg.mozilla.org/integration/autoland/rev/96c2b0eb9283
2. Move remote debugger usage to GeckoViewStartup; r=esawin
https://hg.mozilla.org/integration/autoland/rev/e5c101f347c5
3. Move remote debugging setting to runtime; r=esawin,snorp
https://hg.mozilla.org/integration/autoland/rev/8bb14db6389c
4. Fix Fennec remote debugging; r=esawin
Flags: needinfo?(nchen)
Product: Firefox for Android → GeckoView
Target Milestone: Firefox 61 → mozilla61
You need to log in before you can comment on or make changes to this bug.