Closed Bug 1064834 Opened 6 years ago Closed 6 years ago

move second screen modules to toolkit

Categories

(Toolkit :: General, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: blassey, Assigned: blassey)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch second_screen_modules.patch (obsolete) — Splinter Review
No description provided.
Attachment #8486379 - Flags: review?(mark.finkle)
Comment on attachment 8486379 [details] [diff] [review]
second_screen_modules.patch

>diff --git a/mobile/android/modules/SimpleServiceDiscovery.jsm b/toolkit/modules/secondscreen/SimpleServiceDiscovery.jsm

> // Define the "log" function as a binding of the Log.d function so it specifies
> // the "debug" priority and a log tag.
>-let log = Cu.import("resource://gre/modules/AndroidLog.jsm", {}).AndroidLog.d.bind(null, "SSDP");
>+let log = Cc["@mozilla.org/consoleservice;1"].getService(Ci.nsIConsoleService).logStringMessage

I wouldn't mind this becoming a #ifdef / #else / #endif so we keep our nicer Android logging. We do the same thing in some WebApps toolkit code too.

>+#ifdef ANDROID
>     // We also query Java directly here for any devices that Android might support natively (i.e. Chromecast or Miracast)
>     this.getAndroidDevices();
>   },
>@@ -194,6 +197,7 @@ var SimpleServiceDiscovery = {
>         this._addService(service);
>       }
>     });
>+#endif
>   },

Would you mind treating these blocks separately? Yes, this compiles fine, but I'd like it to be a little cleaner and readable.

r+ with the nits fixed please.

If possible, I'd like to land this after bug 1020564 and bug 1058384 since those would be heavily bitrotted. Give me a few days to push on the reviews and see if I can't land all of these.
Attachment #8486379 - Flags: review?(mark.finkle) → review+
Updated patch with nits addressed
Assignee: nobody → blassey.bugs
Attachment #8486379 - Attachment is obsolete: true
Attachment #8488706 - Flags: review+
NI'ing myself to land this at the right time
Flags: needinfo?(mark.finkle)
https://hg.mozilla.org/mozilla-central/rev/3ead4703741f
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Depends on: 1085702
You need to log in before you can comment on or make changes to this bug.