1000676 - Add about:devices page to display available second screens

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Screencasting, defect)

Description

[Nick Alexander :nalexander [he/him]]

At the moment, Fennec doesn't surface what additional screens have been discovered.  We should do that, and an about: page seems simplest.

This could:

* list screens currently reachable;
* show screens reachable in the past;
* show casting history;
* allow for toggling the casting pref;
* show details about the current network connection;
* allow to manually add a screen (by IP address).

Comment 1

[Nick Alexander :nalexander [he/him]]

Attachment: Part 1: Add aboutCastingDevices.* files. r=mfinkle

Comment 2

[Nick Alexander :nalexander [he/him]]

Attachment: Part 2: Add MOZ_CASTING and register about:devices. r=mfinkle

Comment 3

[Nick Alexander :nalexander [he/him]]

Attachment: Post: Expose SimpleServiceDiscovery.EVENT* constants. r=mfinkle

Comment 4

[Nick Alexander :nalexander [he/him]]

Attachment: Post: Return existing interval from search(). r=mfinkle

Comment 5

[Nick Alexander :nalexander [he/him]]

Let's paint these sheds!  I called the code "aboutCastingDevices" since I find "aboutDevices" too generic.  I registered about:devices since that's what you (mfinkle) suggested.  And I defined MOZ_CASTING because I reckon this will grow to be lots of casting-related functionality, and because I hope this won't be Android specific.

Comment 6

[Nick Alexander :nalexander [he/him]]

wesj: feel free to steal reviews.

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8413256 [details] [diff] [review]
Part 1: Add aboutCastingDevices.* files. r=mfinkle

re: CastingDevices vs Devices

I'm glad you thought about the naming, but I kinda want this to be somewhat generic. Many types of devices can be "discovered" via SSDP, maybe even other Firefoxes running on laptops and such. I noticed that Windows and Linux both have "Devices", where Rokus, Chromecasts and Smart TVs show up.

If it's not a pain, I'd rather go with a "Device" naming.

>diff --git a/mobile/android/chrome/content/aboutCastingDevices.js b/mobile/android/chrome/content/aboutCastingDevices.js
>+// TODO: Export these from SimpleServiceDiscovery.
>+const EVENT_SERVICE_FOUND = "ssdp-service-found";
>+const EVENT_SERVICE_LOST  = "ssdp-service-lost";

personally I'm not a fan of aligning "=" but I can look away.

>+  uninit: function() {
>+	  dump("uninit");
>+	  Services.obs.removeObserver(this, EVENT_SERVICE_FOUND);
>+	  Services.obs.removeObserver(this, EVENT_SERVICE_LOST);
>+
>+    if (this._savedSearchInterval > 0) {
>+	    SimpleServiceDiscovery.search(this._savedSearchInterval);
>+    }
>+  },

Indents are off in this function

>+  _createItemForDevice: function(device) {

I can see us adding more details here (model manufacturer, etc) but that's a new bug

>+  updateDeviceList: function() {

>+    let list = document.getElementById("devices-list");
>+    while (list.firstChild) {
>+      list.removeChild(list.firstChild);
>+    }

There's a "faster" way to do this, but I'm fine with this since the list should be small.

fwiw:

let list = document.getElementById("devices-list");
list.innerHTML = "";

(each removeChild causes a layout and render. setting innerHTML is one shot)

>+  observe: function(subject, topic, data) {
>+    if (topic == EVENT_SERVICE_FOUND || EVENT_SERVICE_LOST) {

fix:
      if (topic == EVENT_SERVICE_FOUND || topic == EVENT_SERVICE_LOST) {

Great start. Fixup the few issues and consider the naming. Let's wait until after Merge Day to land though.

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8413257 [details] [diff] [review]
Part 2: Add MOZ_CASTING and register about:devices. r=mfinkle

In a new bug we could use MOZ_CASTING to wrap some more code

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8413259 [details] [diff] [review]
Post: Return existing interval from search(). r=mfinkle

>diff --git a/mobile/android/chrome/content/aboutCastingDevices.js b/mobile/android/chrome/content/aboutCastingDevices.js

>+    this._savedSearchInterval =
>+      SimpleServiceDiscovery.search(SEARCH_INTERVAL_IN_MILLISECONDS);

No need to wrap

Comment 10

[Wesley Johnston (:wesj)]

Clearing needinfo. I'm not really a fan of us hiding more system info across tons of different about pages, but finkle already knows that.

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Wesley Johnston (:wesj) from comment #10)
> Clearing needinfo. I'm not really a fan of us hiding more system info across
> tons of different about pages, but finkle already knows that.

I don't think we have decided that about:devices would ship to release, but it will be very handy for debugging. That said, I'm not sure where else we'd put this kind of capabilities. This could easily grow into something like about:addons or about:downloads. We might convert it into a native UI, but I think we'll have enough features and information related to devices to need a specific UI for it.

Comment 12

[Nick Alexander :nalexander [he/him]]

mfinkle: so:

* MOZ_DEVICES build flag;
* about:devices registration;
* aboutDevices.* files.

good?

Comment 13

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Nick Alexander :nalexander from comment #12)
> mfinkle: so:
> 
> * MOZ_DEVICES build flag;
> * about:devices registration;
> * aboutDevices.* files.
> 
> good?

Good

Comment 14

[Nick Alexander :nalexander [he/him]]

https://hg.mozilla.org/integration/fx-team/rev/9935f6263940
https://hg.mozilla.org/integration/fx-team/rev/a76dad2f409d
https://hg.mozilla.org/integration/fx-team/rev/6331d53c851f
https://hg.mozilla.org/integration/fx-team/rev/53f602cae343

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9935f6263940
https://hg.mozilla.org/mozilla-central/rev/a76dad2f409d
https://hg.mozilla.org/mozilla-central/rev/6331d53c851f
https://hg.mozilla.org/mozilla-central/rev/53f602cae343