diff --git a/brightsign/autorun.brs b/brightsign/autorun.brs index a335896..8dc32c2 100644 --- a/brightsign/autorun.brs +++ b/brightsign/autorun.brs @@ -126,10 +126,18 @@ End Function Function MakeWidget(url As String, rect As Object, port As Object, cfg As Object) As Object config = { url: url - nodejs_enabled: true ' Node runtime inside the widget - brightsign_js_objects_enabled: true ' REQUIRED for require("@brightsign/*") — without - ' this the bridge silently degrades to no-ops and - ' the player loses identity AND restart delegation + ' THIS is what gates require("@brightsign/*"). Without it the bridge silently degrades to + ' no-ops and the player loses identity AND restart delegation. ("BrightSign modules are + ' actually part of the firmware, but in terms of usage they are identical to other Node.js + ' modules" — so no Node runtime means no modules.) + nodejs_enabled: true + ' NOT what gates require(). This flag enables the LEGACY GLOBAL objects — BSDeviceInfo, + ' BSMessagePort and friends — which this bridge does not use; BrightSign's own cookbook + ' examples call require("@brightsign/bt") with nodejs_enabled alone. Kept set because + ' several of their samples set both and it costs nothing, but the comment that used to sit + ' here credited it with holding the whole bridge up, which would send the next person + ' debugging a dead bridge to exactly the wrong line. + brightsign_js_objects_enabled: true javascript_enabled: true security_params: { websecurity: true } hwz_default: "on" ' hardware z-order — video on its own plane @@ -139,7 +147,10 @@ Function MakeWidget(url As String, rect As Object, port As Object, cfg As Object ' persist to. The XT245 on alpha exposes navigator.serviceWorker and then refuses to ' register one, which is exactly what a widget with no usable storage would do. storage_path: StorageRoot() + "/cache" ' local storage, on the volume we booted from - storage_quota: "1073741824" ' 1GB, as a STRING — service-worker offline cache + ' 1GB, as a DOUBLE. The docs are explicit: "A BrightScript integer is only guaranteed to be + ' able to represent a count of bytes up to 2GB so avoid using integers... Use float or double + ' instead... (string can also be used but is not recommended)". This was a string. + storage_quota: 1073741824.0 port: port mouse_enabled: false } @@ -289,8 +300,15 @@ Sub SetOrientation(widget As Object, o As String) ' page never learned to fall back. Rotation lives on SetScreenModes(), whose per-screen config ' carries a `transform` of normal|90|180|270 and rotates EVERYTHING including the video plane. ' Implemented in BOS 9.0.15+; an older player simply has no method here and is told so. - ' FindMemberFunction is the guard BrightSign's own scripts use for a method that may not exist - ' on this OS version — safer than naming a member directly, which would attempt the call. + ' FindMemberFunction is the documented way to ask whether a method exists on this OS version — + ' safer than naming a member directly, which would attempt the call. It is itself feature-gated + ' (see HasFindMember), and a player that cannot ask cannot be told the answer is yes: rotation + ' is refused rather than risked, and the page keeps its CSS fallback. + if not HasFindMember() then + print "[st] orientation: cannot probe this OS for SetScreenModes — keeping the CSS fallback" + widget.PostJSMessage({ type: "orientation-result", ok: false, error: "cannot probe this OS version" }) + return + end if if FindMemberFunction(vm, "GetScreenModes") = invalid or FindMemberFunction(vm, "SetScreenModes") = invalid then print "[st] orientation: this OS has no SetScreenModes — the page keeps its CSS fallback" widget.PostJSMessage({ type: "orientation-result", ok: false, error: "SetScreenModes unavailable" }) @@ -342,6 +360,27 @@ End Sub ' interface is physically dead, and the DWS still refuses the capture — internal flash is not ' "primary storage" as that endpoint means it. Counting it would re-create the exact lie this ' probe exists to prevent. +' Drop a trailing "/" from a drive specifier. roStorageHotplug.GetStorages() answers with one +' ("SSD:/"), the rest of this script speaks the bare form ("SSD:"), and roStorageInfo takes either. +Function TrimDrive(raw As String) As String + n% = Len(raw) + if n% > 0 and Mid(raw, n%, 1) = "/" then return Left(raw, n% - 1) + return raw +End Function + +' Turn a drive specifier into the one GetStorageStatus() actually accepts. +' +' GetStorages() -> ["USB1:/", "SD:/", "SD2:/", "SSD:/", "Flash:/"] +' GetStorageStatus() understands "USB:", "SD:", "SSD:", "SD2:/", "Flash:" and is documented as +' UNRELIABLE for "USBn:". So: drop the trailing slash, and collapse any USBn to a bare "USB:". +' roStorageInfo, by contrast, is documented for the NUMBERED form — so the two callers get +' different strings and the numbering is only thrown away where it does harm. +Function StatusDrive(raw As String) As String + d$ = TrimDrive(raw) + if LCase(Left(d$, 3)) = "usb" then return "USB:" + return d$ +End Function + Function StorageProbe() As Object result = { present: false, volume: "", free_mb: 0, total_mb: 0 } @@ -352,14 +391,26 @@ Function StorageProbe() As Object ' Ask the platform which volumes exist rather than guessing; the static list is the fallback for ' an OS without the enumerator. Same shape BrightSign's own boilerplate uses. volumes = ["SSD:", "SD:", "SD2:", "USB:"] - if hp <> invalid and FindMemberFunction(hp, "GetStorages") <> invalid then - found = hp.GetStorages() - if found <> invalid and found.Count() > 0 then volumes = found + ' Feature-gated (see HasFindMember). A player that cannot be probed simply keeps the static list, + ' which is the answer the enumerator would have given anyway on every model we ship. + if hp <> invalid and HasFindMember() then + if FindMemberFunction(hp, "GetStorages") <> invalid then + found = hp.GetStorages() + if found <> invalid and found.Count() > 0 then volumes = found + end if end if - for each v in volumes + for each raw in volumes + ' ⚠️ GetStorages() answers in a DIFFERENT vocabulary to the one GetStorageStatus() accepts: + ' it returns ["USB1:/", "SD:/", "SD2:/", "SSD:/", "Flash:/"] — trailing slash, and USB + ' NUMBERED. GetStorageStatus() is documented as UNRELIABLE when called with a "USBn:" + ' parameter and understands "USB:", "SD:", "SSD:", "SD2:/", "Flash:". So handing the + ' enumerator's own output straight back to it re-creates exactly the bug the static list was + ' written to avoid — silently, and only on the OS versions that HAVE the enumerator, which is + ' why the static fallback looked correct in testing. + v = TrimDrive(raw) mounted = false if hp <> invalid then - st = hp.GetStorageStatus(v) + st = hp.GetStorageStatus(StatusDrive(raw)) if st <> invalid and st.mounted then mounted = true end if @@ -408,6 +459,34 @@ Function FullScreenRect() As Object return CreateObject("roRectangle", 0, 0, vm.GetResX(), vm.GetResY()) End Function +' Where the SECOND output lives inside the combined canvas, or invalid on a single-output player. +' +' GetResX/GetResY only ever describe output 1, so they cannot answer this. The per-screen +' configuration can: each entry carries display_x/display_y (its origin within the canvas built by +' SetScreenModes) and `enabled`. A widget placed at that origin paints that output; there is no +' other mechanism, because roHtmlWidget has no output selector. +' +' Returns invalid unless a SECOND, ENABLED screen genuinely exists — the caller then stays +' single-screen and says so, rather than stacking two widgets on output one. The docs warn that +' GetScreenModes on a player with unconnected outputs "won't get a valid return" for them, so an +' entry that does not describe a real screen is treated as absent. +Function SecondScreenRect() As Object + if not HasFindMember() then return invalid + vm = CreateObject("roVideoMode") + if vm = invalid then return invalid + if FindMemberFunction(vm, "GetScreenModes") = invalid then return invalid + + configs = vm.GetScreenModes() + if configs = invalid or configs.Count() < 2 then return invalid + + s = configs[1] + if s = invalid then return invalid + if s.enabled <> invalid and s.enabled = false then return invalid + if s.display_x = invalid or s.display_y = invalid then return invalid + + return CreateObject("roRectangle", s.display_x, s.display_y, vm.GetResX(), vm.GetResY()) +End Function + '=== self-update ============================================================================ ' ' The package (autorun.zip) can replace THIS SCRIPT. That makes it the most dangerous thing the @@ -437,6 +516,20 @@ Function PackageVersion() As String return "0.0.0-dev" ' ST_PACKAGE_VERSION (stamped at build time — do not edit by hand) End Function +' Can this player use FindMemberFunction() at all? +' +' ⚠️ It is NOT unconditionally available: "It is only available if +' roDeviceInfo.HasFeature("FindMemberFunction") returns true." Calling it on a player without the +' feature is a runtime error — and both call sites are reached FROM THE EVENT LOOP (the capability +' probe on every boot, the storage figures in host telemetry every 60 seconds), so on such a player +' the host script would die within a minute of starting and take the display with it. The guard it +' was being used AS is the thing that needed guarding. +Function HasFindMember() As Boolean + di = CreateObject("roDeviceInfo") + if di = invalid then return false + return di.HasFeature("FindMemberFunction") +End Function + ' Does [path] exist? ' ' roReadFile + a type() check — the idiom BrightSign's own boilerplate uses (CheckFile in their @@ -559,6 +652,17 @@ Sub CheckPackageUpdate(cfg As Object, root As String) return end if + ' Guard the manifest HERE, at the call site, because that is the only place a guard can help. + ' VerifyPackage takes `As String` / `As Integer` parameters, and a missing key is `invalid`: + ' handing invalid to a typed parameter is a runtime error raised at the CALL, before a single + ' line inside the function runs. The check inside VerifyPackage reads like it covers this and + ' cannot — the script would already have aborted, from inside the event loop, taking playback + ' down with it. url is checked for the same reason (it is concatenated into a `As String`). + if manifest.url = invalid or manifest.sha256 = invalid or manifest.size = invalid then + print "[st-update] manifest says download but is missing url/sha256/size — ignoring it" + return + end if + print "[st-update] downloading package "; manifest.version ' Any earlier partial is deleted first: resuming into an existing file would concatenate two @@ -783,17 +887,37 @@ Sub Main() widget = MakeWidget(PlayerUrl(cfg, 1), rect, port, cfg) widget.Show() - ' The boot story, delivered late but delivered. Everything above here happened with no page to - ' talk to, which is exactly the window in which the interesting failures live. - FlushLog(widget, boot) - SendHostTelemetry(widget, cfg) - + ' NOT flushed here. Show() only creates the widget — the page has not been fetched, let alone + ' run st-bridge.js, so there is nothing on the other end of PostJSMessage yet and every line + ' would go into the void. Buffered instead until the page says hello (its `probe` message, + ' which st-bridge.js posts as soon as it loads), which is the whole reason the buffer exists. + ' The same window ate SendHostTelemetry; telemetry repeats every 60s so it self-healed and the + ' boot report — the one that only ever happens once — did not. widget2 = invalid if dual then - screen2 = 2 - if cfg.output_mode = "clone" then screen2 = 1 - widget2 = MakeWidget(PlayerUrl(cfg, screen2), rect, port, cfg) - if widget2 <> invalid then widget2.Show() + ' ⚠️ There is NO per-widget output selector. roHtmlWidget takes a rectangle and nothing else: + ' its init parameters have no `screen`/`output` key, and neither does the JavaScript + ' HtmlWidgetParams. A second output is addressed by BUILDING ONE TALL CANVAS with + ' SetScreenModes (display_x/display_y stack the outputs) and then placing the second widget + ' at that offset inside it. + ' + ' Which means the previous version could not work: it passed the SAME full-screen rect for + ' both widgets, so widget 2 was composited directly on top of widget 1 on output ONE — two + ' players fighting over one screen while the second output stayed dark. "dual" and "clone" + ' were configuration options that made the display worse and reported nothing. + rect2 = SecondScreenRect() + if rect2 = invalid then + ' Refused rather than guessed. Multi-output is documented for the XC2055 (two) and + ' XC4055 (four); the XT line has HDMI IN and HDMI OUT, which the series blurb describes + ' as "dual HDMI" and which is not a second output at all. + LogTo(boot, "boot", "output_mode=" + cfg.output_mode + " but this player exposes one output — staying single-screen") + HostEvent(widget, "app_error", "output-mode", "dual/clone requested; this player has a single output") + else + screen2 = 2 + if cfg.output_mode = "clone" then screen2 = 1 + widget2 = MakeWidget(PlayerUrl(cfg, screen2), rect2, port, cfg) + if widget2 <> invalid then widget2.Show() + end if end if retries = 0 @@ -894,6 +1018,11 @@ Sub Main() ' Asked once during boot, before the player registers: the answer decides which ' controls the dashboard is allowed to offer for this display. SendProbeResult(widget) + ' ...and this is the first PROOF that a page is listening, so it is the earliest + ' moment the buffered boot story can actually be delivered. st-bridge.js holds it + ' until the player's socket is up, so late here is still in time. + FlushLog(widget, boot) + SendHostTelemetry(widget, cfg) else if m.type = "set-orientation" then if m.orientation <> invalid then SetOrientation(widget, m.orientation) diff --git a/brightsign/st-bridge.js b/brightsign/st-bridge.js index 67421b2..c872075 100644 --- a/brightsign/st-bridge.js +++ b/brightsign/st-bridge.js @@ -35,7 +35,17 @@ var MessagePortClass = tryRequire('@brightsign/messageport'); var RegistryClass = tryRequire('@brightsign/registry'); var DeviceInfoClass = tryRequire('@brightsign/deviceinfo'); - var VideoOutputClass = tryRequire('@brightsign/videooutput'); + /* + * ⚠️ @brightsign/videooutput does NOT set a video mode. Its surface is read-only plus power + * (getVideoResolution / getEdid / isAttached / setPowerSaveMode / setBackgroundColor); there is + * no setMode on it at all. Mode setting lives on @brightsign/videomodeconfiguration, whose + * setMode() returns a Promise<{restartRequired}>. + * + * The two were conflated here, and the cost was not a broken call — the call was guarded — it was + * a LIE: a widget with no host bridge declared display.resolution purely because videooutput + * resolved, and the dashboard grew a resolution control that could never do anything. + */ + var VideoModeConfigClass = tryRequire('@brightsign/videomodeconfiguration'); var CecClass = tryRequire('@brightsign/cec'); var port = null; @@ -243,6 +253,65 @@ * describes the widget's cache quota, not the disk: a panel can report gigabytes free while the * volume holding them is full, and only the host can tell the difference. */ + /* + * Host diagnostics arrive BEFORE anyone is listening, and that is not an edge case — it is the + * normal order of events and the whole reason they are worth carrying. + * + * The host buffers its pre-widget boot lines and posts them the moment the page says hello. The + * player, correctly, does not subscribe until its socket is connected, because a line forwarded + * before that has nowhere to go. Between those two facts every boot line was dropped: the host + * spoke into a page with no listener, and the listener arrived after the words had gone. The + * player's own comment says wiring earlier "would drop the host's boot report on the floor" — + * which was true, and left the report on the floor anyway. + * + * So the bridge holds them. Messages land in these queues from the moment the file loads, and are + * replayed to each consumer as it registers. Bounded, because a host stuck in a reboot loop must + * not grow this without limit on a player that runs for months. + */ + var PENDING_MAX = 200; + var logSinks = []; + var eventSinks = []; + var pendingLogs = []; + var pendingEvents = []; + + function drain(queue, fn) { + // Copied first: fn is free to register another sink, and iterating a live array while it is + // being appended to is how a replay turns into a loop. + var items = queue.slice(); + for (var i = 0; i < items.length; i++) { + try { fn(items[i]); } catch (e) { /* one bad consumer must not eat the rest of the boot log */ } + } + } + + function fanout(sinks, queue, payload) { + if (sinks.length === 0) { + if (queue.length < PENDING_MAX) queue.push(payload); + return; + } + for (var i = 0; i < sinks.length; i++) { + try { sinks[i](payload); } catch (e) { /* ignore */ } + } + } + + listeners.push(function (msg) { + if (!msg) return; + if (msg.type === 'host-log') { + fanout(logSinks, pendingLogs, { + tag: String(msg.tag || 'host').slice(0, 64), + level: String(msg.level || 'i').slice(0, 8), + message: String(msg.message || '').slice(0, 2000) + }); + return; + } + if (msg.type === 'host-event' && msg.event) { + fanout(eventSinks, pendingEvents, { + event: String(msg.event), + reason: String(msg.reason || '').slice(0, 64), + detail: String(msg.detail || '').slice(0, 500) + }); + } + }); + listeners.push(function (msg) { if (msg && msg.type === 'host-telemetry') { var keys = ['uptime_seconds', 'local_ip', 'model', 'os_version', 'video_mode', @@ -327,10 +396,21 @@ */ add('playback.transitions'); add('playback.pip'); - // Service-worker content caching. The quota is configured in autorun.brs (storage_path + - // storage_quota); without a service worker there is no offline story at all. + /* + * Service-worker content caching — and this platform does not have it. + * + * `navigator.serviceWorker` EXISTS on a BrightSign widget and is not usable: our XT245 on alpha + * passes this exact check, then never even fetches sw.js. Presence was therefore the one signal + * that could not distinguish "caches offline" from "cannot", and it answered yes to both — the + * player advertised offline.cache to the whole fleet while being unable to hold a single byte + * through an outage. The web player already learned this (it waits for a worker that is in + * CONTROL, see declareCapabilities in server/player/index.html); this copy had not. + * + * A controller is proof, not a promise: something is actually intercepting this page's fetches. + */ try { - if (global.navigator && global.navigator.serviceWorker) add('offline.cache'); + var sw = global.navigator && global.navigator.serviceWorker; + if (sw && sw.controller) add('offline.cache'); } catch (e) { /* no SW in this widget */ } // ---- needs the host bridge -------------------------------------------------------------- @@ -342,8 +422,8 @@ add('system.reboot'); // RebootSystem add('display.rotation'); // roVideoMode transform — the ONLY way video rotates here add('display.resolution'); // roVideoMode SetMode - } else if (VideoOutputClass) { - // No host, but the JS video-output module resolved: resolution alone is still reachable. + } else if (VideoModeConfigClass) { + // No host, but the JS mode-configuration module resolved: resolution alone is still reachable. add('display.resolution'); } @@ -420,7 +500,12 @@ serial: function () { if (deviceInfo) { try { - var s = deviceInfo.serialNumber || (deviceInfo.getDeviceUniqueId && deviceInfo.getDeviceUniqueId()); + // `serialNumber` is the whole answer. There is no getDeviceUniqueId() on + // @brightsign/deviceinfo — that is the BrightScript roDeviceInfo method name, and + // BrightSign's own migration note maps it to this attribute. `deviceUniqueId` is the + // legacy BSDeviceInfo global's spelling, also an attribute rather than a call, and is + // read here only so a very old widget build still answers with something. + var s = deviceInfo.serialNumber || deviceInfo.deviceUniqueId; if (s) return String(s); } catch (e) { /* fall through to the URL */ } } @@ -556,10 +641,17 @@ }, setVideoMode: function (mode) { - if (VideoOutputClass) { + if (VideoModeConfigClass) { try { - var vo = new VideoOutputClass(); - if (vo && typeof vo.setMode === 'function') { vo.setMode(mode); return true; } + var vmc = new VideoModeConfigClass(); + if (vmc && typeof vmc.setMode === 'function') { + // Promise<{restartRequired}>. Nothing here awaits it — a mode change that restarts the + // application takes this page with it, so there is no "after" to report into. Rejection + // is swallowed rather than left as an unhandled rejection on a signage player. + var r = vmc.setMode(mode); + if (r && typeof r.catch === 'function') r.catch(function () {}); + return true; + } } catch (e) { /* fall back to the host */ } } return post({ type: 'set-video-mode', mode: mode }); @@ -663,26 +755,14 @@ */ onHostLog: function (fn) { if (typeof fn !== 'function') return; - listeners.push(function (msg) { - if (!msg || msg.type !== 'host-log') return; - fn({ - tag: String(msg.tag || 'host').slice(0, 64), - level: String(msg.level || 'i').slice(0, 8), - message: String(msg.message || '').slice(0, 2000) - }); - }); + logSinks.push(fn); + drain(pendingLogs, fn); }, onHostEvent: function (fn) { if (typeof fn !== 'function') return; - listeners.push(function (msg) { - if (!msg || msg.type !== 'host-event' || !msg.event) return; - fn({ - event: String(msg.event), - reason: String(msg.reason || '').slice(0, 64), - detail: String(msg.detail || '').slice(0, 500) - }); - }); + eventSinks.push(fn); + drain(pendingEvents, fn); }, /* diff --git a/server/lib/player-capabilities.js b/server/lib/player-capabilities.js index afdf145..c6e1788 100644 --- a/server/lib/player-capabilities.js +++ b/server/lib/player-capabilities.js @@ -98,7 +98,12 @@ const BASELINE = { 'display.rotation', 'display.power', 'remote.input', 'system.reboot', 'system.restart_player', - 'sync.clock', 'offline.cache', + 'sync.clock', + // NOT offline.cache. A BrightSign widget EXPOSES navigator.serviceWorker and will not run one: + // our XT245 on alpha passes every presence check and then never even fetches sw.js. There is no + // other caching mechanism in the widget either — content comes off the network every time — so + // a legacy BrightSign that declares nothing has no offline story at all, and claiming one told + // the dashboard a panel would survive an outage that will in fact go blank. ], // A browser tab. Deliberately the smallest set: it cannot reboot its host, rotate a panel, or // capture anything outside its own document. diff --git a/server/test/brightscript-api-surface.test.js b/server/test/brightscript-api-surface.test.js index 2b4e6da..56cd48a 100644 --- a/server/test/brightscript-api-surface.test.js +++ b/server/test/brightscript-api-surface.test.js @@ -44,6 +44,8 @@ const ROKU_ONLY = [ ['roUnzip', 'use roBrightPackage — roUnzip is not the reader for a player package'], ['roRegistryKey', 'use roRegistrySection'], ['roAssociativeArrayEx', 'plain roAssociativeArray'], + // roDeviceInfo.GetDeviceUniqueId() is real; a global of that name is not. + ['roDataGramSocket', 'not a BrightSign object'], ]; for (const [obj, advice] of ROKU_ONLY) { @@ -59,6 +61,13 @@ const BAD_METHODS = [ ['PostFromStringWithRetry', 'roUrlTransfer has no retry variant; AsyncPostFromString + roUrlEvent is how a POST body is read'], ['.Final(', 'roMessageDigest-era API; roHashGenerator returns the digest from Hash()'], ['OpenInputFile', 'no roFileSystem here; read with roByteArray.ReadFile'], + // Each of these reads as obviously-correct and is documented NOT to exist on BrightSign. + ['SetMessagePort', 'the setter is SetPort(roMessagePort) — SetMessagePort is Roku'], + ['.WaitEvent(', 'roMessagePort has WaitMessage(timeout_ms); WaitEvent is Roku'], + ['.SetAlgorithm(', 'roHashGenerator takes its algorithm as a CONSTRUCTOR argument'], + ['.VerifyPackage(', 'roBrightPackage has no VerifyPackage — hash the bytes yourself'], + ['.UnpackAll(', 'roBrightPackage has Unpack(path) and UnpackFile(name, path)'], + ['SetOrientation(vm', 'roVideoMode has no SetOrientation — rotation is SetScreenModes()[i].transform'], ]; for (const [needle, advice] of BAD_METHODS) { @@ -225,3 +234,150 @@ test('Str() is never applied to something that is already a string', () => { } } }); + +/* --------------------------------------------------------------------------------------------- + * Rules added after a line-by-line pass against docs.brightsign.biz. Each one is a call that + * compiles, reads correctly, and is documented to do something other than what it looks like. + * ------------------------------------------------------------------------------------------- */ + +test('roVideoMode.SetOrientation does not exist — rotation is a SetScreenModes transform', () => { + // Zero occurrences of SetOrientation in the roVideoMode reference. Rotation has two documented + // routes and neither is a method on roVideoMode: the per-screen `transform` member of + // SetScreenModes ("normal"/"90"/"180"/"270"), or roHtmlWidget's `transform` init parameter + // ("identity"/"rot90"/"rot180"/"rot270") — note the two use DIFFERENT vocabularies. + for (const f of FILES) { + for (const m of code(f).matchAll(/(\w+)\.SetOrientation\s*\(/g)) { + assert.fail(`${f}: ${m[0]} — roVideoMode has no SetOrientation. Use SetScreenModes()[i].transform`); + } + } +}); + +test('FindMemberFunction is itself feature-gated before it is used as a guard', () => { + // "It is only available if roDeviceInfo.HasFeature("FindMemberFunction") returns true." Calling it + // on a player without the feature is a runtime error — and both call sites here are reached from + // the EVENT LOOP (the boot capability probe, and host telemetry every 60s), so on such a player + // the host would die within a minute and take the display with it. The guard needed guarding. + for (const f of FILES) { + const src = code(f); + if (!src.includes('FindMemberFunction(')) continue; + assert.match(src, /HasFeature\("FindMemberFunction"\)/, + `${f}: FindMemberFunction is only available when roDeviceInfo.HasFeature("FindMemberFunction") is true`); + // ...and every call must sit behind that check, not merely somewhere in the same file. + for (const m of src.matchAll(/^(?!.*HasFeature).*\bFindMemberFunction\(/gm)) { + const line = m[0]; + assert.ok(/HasFindMember\(\)|HasFeature/.test(src.slice(Math.max(0, src.indexOf(line) - 400), src.indexOf(line))), + `${f}: this FindMemberFunction call is not behind a HasFeature guard:\n ${line.trim()}`); + } + } +}); + +test('GetStorageStatus is never handed a drive string straight from GetStorages()', () => { + // The two speak DIFFERENT vocabularies. GetStorages() answers ["USB1:/", "SD:/", "SD2:/", "SSD:/", + // "Flash:/"] — trailing slash, USB numbered — while GetStorageStatus() understands "USB:", "SD:", + // "SSD:", "SD2:/", "Flash:" and is documented as UNRELIABLE for "USBn:". Feeding the enumerator's + // output back in re-creates the bug the static fallback list exists to avoid, and only on the OS + // versions that HAVE the enumerator — so it looks fine in testing. + for (const f of FILES) { + const src = code(f); + if (!src.includes('GetStorageStatus')) continue; + for (const m of src.matchAll(/GetStorageStatus\(([^)]*)\)/g)) { + const arg = m[1].trim(); + assert.ok(/^"/.test(arg) || /StatusDrive\(/.test(arg), + `${f}: ${m[0]} — normalise the drive first (StatusDrive) or pass a literal; GetStorages() speaks a different vocabulary`); + } + } +}); + +test('PostJSMessage keys are lowercase — BrightScript canonicalises them on the way to JS', () => { + // BrightScript associative-array keys created with object-literal syntax are case-INSENSITIVE and + // arrive lowercased on the JavaScript side. BrightSign's own sample sends + // `{serialNumber: ...}` and reads `msg["serialnumber"]`. A camelCase key here is therefore a field + // the bridge reads as undefined, silently, with no error anywhere. + for (const f of FILES) { + for (const m of code(f).matchAll(/PostJSMessage\(\{([\s\S]*?)\}\)/g)) { + for (const km of m[1].matchAll(/(?:^|[,\n])\s*([A-Za-z_]\w*)\s*:/g)) { + assert.equal(km[1], km[1].toLowerCase(), + `${f}: PostJSMessage key "${km[1]}" is not lowercase — it arrives in JavaScript lowercased and the reader sees undefined`); + } + } + } +}); + +test('PostJSMessage payloads are flat — nested associative arrays are not supported', () => { + // "This method does not support passing nested associative arrays." A nested value is dropped, so + // the message arrives looking well-formed and missing the part that mattered. + for (const f of FILES) { + for (const m of code(f).matchAll(/PostJSMessage\(\{([\s\S]*?)\}\)/g)) { + assert.ok(!/:\s*\{/.test(m[1]), + `${f}: PostJSMessage carries a nested associative array, which BrightSign drops:\n ${m[0].slice(0, 160)}`); + } + } +}); + +test('roUrlTransfer.SetPort takes a message port, never a TCP port number', () => { + // ifMessagePort.SetPort(port As roMessagePort). There is no integer overload — a TCP port belongs + // in the URL string. Passing a number here reads exactly like configuring a port and configures + // nothing. + for (const f of FILES) { + for (const m of code(f).matchAll(/\.SetPort\(([^)]*)\)/g)) { + assert.ok(!/^\d+$/.test(m[1].trim()), + `${f}: ${m[0]} — SetPort takes an roMessagePort; put a TCP port in the URL`); + } + } +}); + +test('roBrightPackage is constructed with a filename string, not an associative array', () => { + // "created with a filename parameter that specifies the name of the .zip file". A password goes + // through SetPassword() afterwards. An AA here is the Roku-shaped guess. + for (const f of FILES) { + for (const m of code(f).matchAll(/CreateObject\("roBrightPackage"\s*,\s*([^)]*)\)/g)) { + assert.ok(!m[1].trim().startsWith('{'), + `${f}: ${m[0]} — roBrightPackage takes a filename String; use SetPassword() for a password`); + } + } +}); + +test('the widget storage quota is not a string', () => { + // "A BrightScript integer is only guaranteed to be able to represent a count of bytes up to 2GB so + // avoid using integers... Use float or double instead... (string can also be used but is not + // recommended)." + const src = code('autorun.brs'); + assert.ok(!/storage_quota:\s*"/.test(src), 'storage_quota should be a double, not a string'); + assert.match(src, /storage_quota:\s*\d+\.\d/, 'storage_quota must be a double literal'); +}); + +test('nodejs_enabled is what gates require("@brightsign/*")', () => { + // The bridge is entirely require()-based. brightsign_js_objects_enabled gates the LEGACY GLOBALS + // (BSDeviceInfo, BSMessagePort) and not the modules — BrightSign's own cookbook calls + // require("@brightsign/bt") with nodejs_enabled alone. Losing nodejs_enabled costs the player its + // identity and its restart delegation, silently. + const src = code('autorun.brs'); + assert.match(src, /nodejs_enabled:\s*true/, 'without this there are no @brightsign modules at all'); +}); + +test('a second widget is never given the first screen\'s rectangle', () => { + // roHtmlWidget has NO output selector — not in its init parameters, not in the JavaScript + // HtmlWidgetParams. A second output is addressed by building one tall canvas with SetScreenModes + // and placing the widget at that output's display_x/display_y. Reusing the full-screen rect puts + // both widgets on output ONE, on top of each other, while output two stays dark. + const src = code('autorun.brs'); + if (!/widget2\s*=\s*MakeWidget/.test(src)) return; + // Split on TOP-LEVEL commas: the first argument is itself a call (PlayerUrl(cfg, screen2)), and a + // naive split lands on its inner comma and inspects the wrong argument — which is how the first + // draft of this rule passed against the very source it was written to reject. + const call = /widget2\s*=\s*MakeWidget\((.*)\)\s*$/m.exec(src); + assert.ok(call, 'could not find the second widget'); + const args = []; + let depth = 0; + let cur = ''; + for (const ch of call[1]) { + if (ch === '(') depth++; + if (ch === ')') depth--; + if (ch === ',' && depth === 0) { args.push(cur.trim()); cur = ''; continue; } + cur += ch; + } + args.push(cur.trim()); + assert.equal(args.length, 4, `unexpected MakeWidget arity: ${call[0]}`); + assert.notEqual(args[1], 'rect', + 'the second widget must be positioned at the second output\'s canvas offset, not at rect'); +}); diff --git a/server/test/brightsign-capabilities.test.js b/server/test/brightsign-capabilities.test.js index 1843f5b..a3ea2d8 100644 --- a/server/test/brightsign-capabilities.test.js +++ b/server/test/brightsign-capabilities.test.js @@ -26,7 +26,8 @@ const { CAP_SET } = require('../lib/player-capabilities'); * host - a message port exists (nodejs_enabled widget with a live autorun.brs) * probeRes - what the host answers the capability probe with (null = never answers) * modules - which @brightsign/* modules resolve - * sw - navigator.serviceWorker present + * sw - navigator.serviceWorker present (the BrightSign reality: present, never usable) + * swControlled - ...and a worker actually controlling the page (what offline.cache really needs) */ function load(o = {}) { const opts = Object.assign( @@ -38,7 +39,13 @@ function load(o = {}) { const sandbox = { console: { log() {}, warn() {}, error() {} }, - navigator: Object.assign({ userAgent: 'BrightSign/9.0.189 (XT245) Chrome/120' }, opts.sw ? { serviceWorker: {} } : {}), + // `serviceWorker` present but with NO controller is the real BrightSign shape: the property is + // there, registration never happens, and nothing ever controls the page. Modelled separately + // from `swControlled` because a stand-in that conflated them is what let the bug ship. + navigator: Object.assign( + { userAgent: 'BrightSign/9.0.189 (XT245) Chrome/120' }, + opts.swControlled ? { serviceWorker: { controller: {} } } : (opts.sw ? { serviceWorker: {} } : {}) + ), location: { search: '' }, setInterval: () => 1, setTimeout: (fn) => { inbound.push(fn); return 1; }, // deterministic: fired manually @@ -171,9 +178,22 @@ test('NEVER declared: the things BrightSign genuinely has no equivalent for', () } }); -test('offline.cache follows the service worker, not the platform', () => { - assert.ok(load({ probeRes: WITH_DISK }).caps.includes('offline.cache')); +test('offline.cache needs a worker in CONTROL, not merely a navigator property', () => { + // Found on hardware, and this test used to assert the bug. `navigator.serviceWorker` EXISTS on a + // BrightSign widget and is not usable: our XT245 on alpha passes the presence check, then never + // even fetches sw.js. Presence was therefore the one signal that could not tell "caches offline" + // from "cannot", and it answered yes to both — so every BrightSign in the fleet advertised an + // offline capability it could not honour, which is precisely the dead-button failure this whole + // capability model exists to remove. + // + // A controller is proof rather than a promise: something is actually intercepting this page's + // fetches. The web player learned this a release ago (declareCapabilities in + // server/player/index.html); this copy had not. + assert.ok(!load({ probeRes: WITH_DISK, sw: true }).caps.includes('offline.cache'), + 'a runtime that exposes serviceWorker and will not run one must not claim to cache'); assert.ok(!load({ probeRes: WITH_DISK, sw: false }).caps.includes('offline.cache')); + assert.ok(load({ probeRes: WITH_DISK, swControlled: true }).caps.includes('offline.cache'), + 'a worker that IS controlling the page is a real offline story and must still be declared'); }); test('the probe is actually sent to the host', () => { diff --git a/server/test/brightsign-host-diagnostics.test.js b/server/test/brightsign-host-diagnostics.test.js index 3a27dd8..40ef500 100644 --- a/server/test/brightsign-host-diagnostics.test.js +++ b/server/test/brightsign-host-diagnostics.test.js @@ -24,6 +24,46 @@ const bridge = fs.readFileSync(path.join(ROOT, 'brightsign', 'st-bridge.js'), 'u const player = fs.readFileSync(path.join(ROOT, 'server', 'player', 'index.html'), 'utf8'); const code = host.split('\n').filter((l) => !/^\s*'/.test(l)).join('\n'); +// Objects built inside the vm carry the vm realm's prototypes, so deepStrictEqual would compare +// realms rather than values. Round-trip through JSON to compare what actually crossed the bridge. +const norm = (x) => JSON.parse(JSON.stringify(x)); + +/* + * The bridge half, actually EXECUTED against a fake widget rather than pattern-matched. + * + * The two source-regex assertions this replaces both passed while the chain was broken end to end, + * which is the whole argument for running it: "the file contains onHostLog" is not evidence that a + * log line reaches anybody. `deliver` plays the part of roHtmlWidget.PostJSMessage. + */ +function loadBridge() { + const vm = require('node:vm'); + const inbound = []; + const sandbox = { + console: { log() {}, warn() {}, error() {} }, + navigator: { userAgent: 'BrightSign/9.0.189 (XT245) Chrome/120' }, + location: { search: '' }, + setTimeout: () => 1, setInterval: () => 1, clearTimeout: () => {}, + Promise, Object, Array, Uint8Array, Math, Date, RegExp, String, Number, + parseInt, isNaN, isFinite, decodeURIComponent, Error, + localStorage: { getItem: () => null, setItem() {} }, + }; + sandbox.window = sandbox; + sandbox.require = (name) => { + if (name === '@brightsign/messageport') { + return function () { + return { + PostBSMessage: () => {}, + addEventListener: (evt, fn) => { if (evt === 'bsmessage') inbound.push(fn); }, + }; + }; + } + throw new Error('no module ' + name); + }; + vm.createContext(sandbox); + vm.runInContext(bridge, sandbox); + return { api: sandbox.ScreenTinkerBS, deliver: (msg) => inbound.forEach((fn) => fn(msg)) }; +} + test('the host reports its boot story, which happens before there is a page to hear it', () => { // The interesting failures all live in this window: the storage probe, a pending package being // applied, the video mode being set. A design that could only report after the widget existed @@ -31,7 +71,7 @@ test('the host reports its boot story, which happens before there is a page to h assert.match(code, /Sub LogTo\(buf As Object/, 'a buffer the pre-widget phase can log into'); assert.match(code, /Sub FlushLog\(widget As Object, buf As Object\)/, 'and a flush once there is a page'); assert.match(code, /boot = CreateObject\("roArray"/, 'Main must create the buffer'); - assert.match(code, /FlushLog\(widget, boot\)/, 'and flush it once the widget exists'); + assert.match(code, /FlushLog\(widget, boot\)/, 'and flush it once a page is listening'); // The update path is the one that replaces the boot script — its diagnostics are the ones you // most want when a player does not come back. assert.match(code, /Sub ApplyPendingPackage\(root As String, buf As Object\)/); @@ -70,12 +110,74 @@ test('the event types the host emits are ones the server actually accepts', () = }); test('the bridge carries logs and events without interpreting them', () => { - assert.match(bridge, /onHostLog: function/); - assert.match(bridge, /onHostEvent: function/); + const { api, deliver } = loadBridge(); + const logs = []; + const events = []; + api.onHostLog((l) => logs.push(l)); + api.onHostEvent((e) => events.push(e)); + + deliver({ type: 'host-log', tag: 'update', level: 'i', message: 'package applied' }); + deliver({ type: 'host-event', event: 'crash', reason: 'watchdog', detail: 'no heartbeat for 120s' }); + + assert.deepEqual(norm(logs), [{ tag: 'update', level: 'i', message: 'package applied' }]); + assert.deepEqual(norm(events), [{ event: 'crash', reason: 'watchdog', detail: 'no heartbeat for 120s' }]); + // Bounded before they reach the wire: the server truncates too, but a host bug should not be // able to push a megabyte through the socket every second. - assert.match(bridge, /msg\.message \|\| ''\)\.slice\(0, 2000\)/); - assert.match(bridge, /msg\.detail \|\| ''\)\.slice\(0, 500\)/); + deliver({ type: 'host-log', tag: 'x'.repeat(200), message: 'y'.repeat(5000) }); + deliver({ type: 'host-event', event: 'app_error', reason: 'r'.repeat(200), detail: 'd'.repeat(5000) }); + assert.equal(logs[1].message.length, 2000); + assert.equal(logs[1].tag.length, 64); + assert.equal(events[1].detail.length, 500); + assert.equal(events[1].reason.length, 64); +}); + +test('THE DROPPED BOOT REPORT: a diagnostic sent before anyone subscribed is still delivered', () => { + // The regression this whole file exists to prevent, and it was live. The ordering is not an edge + // case, it is the ONLY ordering: the host buffers its pre-widget lines and posts them the instant + // the page says hello, while the player deliberately does not subscribe until its socket is up + // (forwarding earlier would have nowhere to send them). Between those two correct decisions every + // boot line fell on the floor — the host spoke to a page with no listener, and the listener + // arrived after the words had gone. + // + // So the bridge holds them. Nothing else in the chain can: the host has already moved on and the + // player cannot subscribe any earlier. + const { api, deliver } = loadBridge(); + deliver({ type: 'host-log', tag: 'boot', level: 'i', message: 'host 1.2.3 from SSD: -> https://s' }); + deliver({ type: 'host-log', tag: 'update', level: 'i', message: 'package applied — rebooting into it' }); + deliver({ type: 'host-event', event: 'app_error', reason: 'load-error', detail: 'attempt 1: https://s/player' }); + + const logs = []; + const events = []; + api.onHostLog((l) => logs.push(l)); + api.onHostEvent((e) => events.push(e)); + + assert.deepEqual(logs.map((l) => l.tag), ['boot', 'update'], 'the boot story must survive the gap'); + assert.deepEqual(events.map((e) => e.event), ['app_error']); + + // ...and delivery keeps working normally afterwards, oldest-first with no duplication. + deliver({ type: 'host-log', tag: 'tel', level: 'i', message: 'later' }); + assert.deepEqual(logs.map((l) => l.tag), ['boot', 'update', 'tel']); +}); + +test('a second subscriber gets the same history, and one that throws cannot eat it', () => { + const { api, deliver } = loadBridge(); + deliver({ type: 'host-log', tag: 'boot', level: 'i', message: 'early' }); + + api.onHostLog(() => { throw new Error('a consumer blew up'); }); + const logs = []; + api.onHostLog((l) => logs.push(l)); + assert.deepEqual(logs.map((l) => l.message), ['early'], 'a broken consumer must not swallow the replay'); +}); + +test('the pending queue is bounded — a reboot loop must not grow it without limit', () => { + // This player runs for months. An unbounded buffer fed by a host stuck in a loop is a slow leak + // on the one device nobody is watching. + const { api, deliver } = loadBridge(); + for (let i = 0; i < 5000; i++) deliver({ type: 'host-log', tag: 'boot', message: 'line ' + i }); + const logs = []; + api.onHostLog((l) => logs.push(l)); + assert.ok(logs.length > 0 && logs.length <= 200, `queue must be capped, got ${logs.length}`); }); test('host telemetry merges into the snapshot the heartbeat already sends', () => { @@ -87,12 +189,30 @@ test('host telemetry merges into the snapshot the heartbeat already sends', () = }); test('the host telemetry listener is registered at load, not behind the readiness gate', () => { - // The host sends its boot report the moment the widget exists. A listener attached after the + // The host sends its boot report the moment the page says hello. A listener attached after the // bridge finished its own probe would miss precisely the message that says which volume the // player came up from and whether a package applied. - const seg = bridge.slice(bridge.indexOf('var telemetry = {}'), bridge.indexOf('var telemetry = {}') + 1400); - assert.match(seg, /listeners\.push\(function \(msg\)/); - assert.ok(!/onReady\(function/.test(seg), 'must not wait on readiness to start listening'); + // + // Executed, not pattern-matched: the previous form asserted that a `listeners.push` appeared + // within 1400 characters of a variable declaration, which is a statement about formatting. + const { api, deliver } = loadBridge(); + deliver({ type: 'host-telemetry', boot_volume: 'SSD:', storage_free_mb: 90000, package_version: '1.2.3' }); + assert.deepEqual(norm(api.telemetrySnapshot()), { + boot_volume: 'SSD:', storage_free_mb: 90000, package_version: '1.2.3', + }); +}); + +test('the host holds its boot log until a PAGE answers, not until a widget exists', () => { + // Show() only creates the widget: the page has not been fetched, let alone run st-bridge.js, so a + // flush there posts into a void. The `probe` message is the first proof that JavaScript is running + // on the other end, and is therefore the earliest moment the buffer can actually be delivered. + const main = code.slice(code.indexOf('Sub Main()')); + const afterShow = main.slice(main.indexOf('widget.Show()'), main.indexOf('widget.Show()') + 400); + assert.ok(!/FlushLog\(/.test(afterShow), + 'flushing straight after Show() posts the boot story to a page that has not loaded yet'); + + const probeBranch = main.slice(main.indexOf('m.type = "probe"'), main.indexOf('m.type = "probe"') + 400); + assert.match(probeBranch, /FlushLog\(widget, boot\)/, 'flush when the page proves it is listening'); }); test('the player forwards them, and only where the hooks exist', () => { diff --git a/server/test/tizen-media-cache.test.js b/server/test/tizen-media-cache.test.js index 2df0f37..d7fb4c1 100644 --- a/server/test/tizen-media-cache.test.js +++ b/server/test/tizen-media-cache.test.js @@ -221,3 +221,269 @@ test('an item with no revision still caches, and matches a copy stored without o await mc.sync([{ content_id: 'c1' }], () => 'http://s/x'); assert.equal(b.requests, after); }); + +/* ================================================================================================ + * THE ADAPTER ITSELF. + * + * Everything above drives the DECISION layer against a fake backend, which is the right way to test + * decisions — and it is also how the real adapter shipped unable to write a single byte without one + * assertion noticing. The fake was correct; the platform calls underneath it were not: + * + * - `var dir = tizen.filesystem.resolve(...)` — resolve() is declared `void`. dir was undefined, + * available() answered false, and MediaCache.create() returned null on every panel in the + * fleet. The offline cache did not exist. It failed CLOSED, which is the only reason this never + * showed up as corruption: the capability was correctly withheld and the feature was absent. + * - `f.openStream('a', cb)` — asynchronous. `written` was read on the next line, before any + * callback could have run, so appendPart returned 0 forever. + * - `part.moveTo(destPath, name, ...)` — asynchronous, belongs on the parent DIRECTORY, and takes + * (originFullPath, destinationFullPath). Called on a file handle with the arguments transposed: + * three documented errors in one call, each of which alone raises IOError. + * + * So the adapter is exercised here too, against a fake `tizen.filesystem` written from Samsung's + * published IDL rather than from our code — including the deprecated calls, modelled with their + * documented (useless-to-us) semantics, so reaching for them again fails here instead of in a shop. + * ============================================================================================== */ + +/** A fake Samsung TV, per developer.samsung.com/smarttv Filesystem API. */ +function fakeTizen({ version = 5.0 } = {}) { + const files = new Map(); // full virtual path -> number[] + const calls = []; + + function makeFile(p) { + return { + fullPath: p, + isDirectory: false, + get fileSize() { return (files.get(p) || []).length; }, + toURI: () => 'file:///opt/usr/apps/priv/' + p, + // "This operation is performed asynchronously." + openStream(mode, onsuccess) { + calls.push('openStream'); + const stream = { + writeBytes(bytes) { + const cur = files.get(p) || []; + files.set(p, mode === 'a' ? cur.concat(Array.from(bytes)) : Array.from(bytes)); + }, + close() {}, + }; + if (onsuccess) setTimeout(() => onsuccess(stream), 0); + }, + // "IOError - If the File in which the moveTo() method is invoked is a file (not a directory)" + moveTo() { const e = new Error('IOError'); e.name = 'IOError'; throw e; }, + }; + } + + // --- deprecated 1.0 surface, with its REAL semantics --------------------------------------- + const deprecated = { + // "void resolve(...)" — the File arrives only via the callback; the return value is undefined. + resolve(location, onsuccess) { + calls.push('resolve'); + const dir = { + isDirectory: true, + fullPath: location, + resolve(name) { + const p = location + '/' + name; + if (!files.has(p)) { const e = new Error('NotFoundError'); e.name = 'NotFoundError'; throw e; } + return makeFile(p); + }, + createFile(name) { const p = location + '/' + name; files.set(p, []); return makeFile(p); }, + deleteFile(p, ok) { files.delete(p); if (ok) setTimeout(ok, 0); }, + moveTo(origin, dest, overwrite, ok) { + if (!files.has(origin)) { const e = new Error('NotFoundError'); e.name = 'NotFoundError'; throw e; } + files.set(dest, files.get(origin)); files.delete(origin); + if (ok) setTimeout(ok, 0); + }, + }; + if (onsuccess) setTimeout(() => onsuccess(dir), 0); + return undefined; + }, + }; + + if (version < 5) return { filesystem: deprecated, __files: files, __calls: calls }; + + // --- 5.0 synchronous FileSystemManager ----------------------------------------------------- + const modern = Object.assign({}, deprecated, { + pathExists(p) { return files.has(p); }, + toURI(p) { return 'file:///opt/usr/apps/priv/' + p; }, + deleteFile(p, ok) { files.delete(p); if (ok) setTimeout(ok, 0); }, + // "FileHandle openFile(Path path, FileMode openMode, optional boolean makeParents)" — RETURNS. + openFile(p, mode, makeParents) { + calls.push('openFile:' + mode); + if (!files.has(p)) { + if (mode === 'r') { const e = new Error('NotFoundError'); e.name = 'NotFoundError'; throw e; } + files.set(p, []); + } + if (mode === 'w') files.set(p, []); // 'w' truncates + let pos = 0; + return { + path: p, + seek(offset) { pos = offset; return this; }, + writeData(u8) { + const cur = files.get(p).slice(); + // A positioned write: pad any gap, then overwrite in place — what seek+write really does. + while (cur.length < pos) cur.push(0); + for (let i = 0; i < u8.length; i++) cur[pos + i] = u8[i]; + pos += u8.length; + files.set(p, cur); + }, + flush() {}, close() {}, + }; + }, + }); + return { filesystem: modern, __files: files, __calls: calls }; +} + +function withTizen(fake, fn) { + const hadT = 'tizen' in global; const oldT = global.tizen; + const hadL = 'localStorage' in global; const oldL = global.localStorage; + const store = new Map(); + global.tizen = fake; + global.localStorage = { getItem: (k) => (store.has(k) ? store.get(k) : null), setItem: (k, v) => store.set(k, v) }; + try { return fn(); } finally { + if (hadT) global.tizen = oldT; else delete global.tizen; + if (hadL) global.localStorage = oldL; else delete global.localStorage; + } +} + +const MEDIA_DIR = 'wgt-private/st-media/'; + +test('ADAPTER: a Tizen 5.0 panel really can write, resume and hand back a URI', () => { + const fake = fakeTizen({ version: 5.0 }); + withTizen(fake, () => { + const b = MediaCache.tizenBackend(); + assert.equal(b.available(), true, 'the 5.0 synchronous filesystem must be usable'); + + // The contract the decision layer depends on: appendPart returns the number of bytes WRITTEN, + // synchronously. Returning 0 here is what made every download stall forever. + assert.equal(b.appendPart('c1', [1, 2, 3], 0), 3, 'a first write must report what it wrote'); + assert.equal(b.appendPart('c1', [4, 5], 3), 2, 'and so must a resumed one'); + assert.deepEqual(fake.__files.get(MEDIA_DIR + 'c1'), [1, 2, 3, 4, 5]); + + const p = b.promotePart('c1'); + assert.ok(p && p.uri.startsWith('file://'), 'a finished asset must resolve to a playable URI'); + + b.remove('c1'); + assert.equal(fake.__files.has(MEDIA_DIR + 'c1'), false); + assert.equal(b.promotePart('c1'), null, 'and must not claim a file that is gone'); + }); +}); + +test('ADAPTER: writes are POSITIONED, so replaying a chunk cannot corrupt the file', () => { + // The crash window is real and is exactly the event this feature exists for: power is cut between + // the write and the index save, so the next boot replays the last chunk. An append lands it a + // second time and the panel promotes a silently corrupt video that plays as garbage. A positioned + // write overwrites the same bytes with the same bytes, and the window stops mattering. + const fake = fakeTizen({ version: 5.0 }); + withTizen(fake, () => { + const b = MediaCache.tizenBackend(); + b.appendPart('c1', [1, 2, 3, 4], 0); + b.appendPart('c1', [5, 6], 4); + b.appendPart('c1', [5, 6], 4); // the replay + assert.deepEqual(fake.__files.get(MEDIA_DIR + 'c1'), [1, 2, 3, 4, 5, 6], + 'a replayed chunk must overwrite, not append'); + }); +}); + +test('ADAPTER: offset 0 truncates, so a lost index restarts cleanly instead of prepending', () => { + const fake = fakeTizen({ version: 5.0 }); + withTizen(fake, () => { + const b = MediaCache.tizenBackend(); + b.appendPart('c1', [9, 9, 9, 9, 9, 9], 0); + b.appendPart('c1', [1, 2], 0); + assert.deepEqual(fake.__files.get(MEDIA_DIR + 'c1'), [1, 2]); + }); +}); + +test('ADAPTER: a panel without the synchronous filesystem says so instead of writing nothing', () => { + // Tizen 4.0 (2018 models). The deprecated resolve()/openStream() pair cannot serve a synchronous + // backend at all, and a cache that reports itself available and then silently writes zero bytes is + // worse than no cache — capabilities.js declares offline.cache on the strength of create(). + for (const fake of [fakeTizen({ version: 4.0 }), { filesystem: null }, {}]) { + withTizen(fake, () => { + const b = MediaCache.tizenBackend(); + assert.equal(b.available(), false); + assert.equal(b.appendPart('c1', [1, 2, 3], 0), 0, 'and must not pretend to have written'); + assert.equal(b.promotePart('c1'), null); + assert.equal(MediaCache.create(), null, 'so no cache is created, and no capability claimed'); + }); + } +}); + +test('ADAPTER: no deprecated asynchronous call is on the write path', () => { + // Belt and braces against the exact regression: resolve(), openStream() and moveTo() all hand + // their result to a callback, so any backend built on them returns before it has done anything. + const fake = fakeTizen({ version: 5.0 }); + withTizen(fake, () => { + const b = MediaCache.tizenBackend(); + b.appendPart('c1', [1], 0); + b.promotePart('c1'); + b.remove('c1'); + assert.deepEqual(fake.__calls.filter((c) => c === 'resolve' || c === 'openStream'), [], + 'resolve() returns void and openStream() is async — neither can serve a synchronous backend'); + }); +}); + +test('a 206 with no readable Content-Range is a stall, never a completed asset', async () => { + // Content-Length on a 206 is the length of the CHUNK. Trusting it reports the first 1MB of a 50MB + // video as a 1MB asset — complete, promoted, and truncated on screen. A proxy that strips + // Content-Range, or a CORS context where the header simply is not readable, produces exactly it. + const b = fakeBackend(asset(CHUNK * 4, 2)); + const mc = new MediaCache(b); + // The entry fetchStep would have created before the first request. + mc.index.c1 = { rev: 5, bytes: 0, total: 0, validator: null, complete: false, path: null, uri: null }; + const verdict = await mc.applyChunk('c1', 5, { + status: 206, start: 0, total: 0, validator: '"v1"', body: new Array(CHUNK).fill(2), + }); + assert.equal(verdict, 'stalled'); + assert.equal(mc.localUrl('c1', 5), null, 'a truncated file must never be handed to the player'); +}); + +test('a 200 whose body is short of its own length is progress, not done', async () => { + const b = fakeBackend(asset(CHUNK, 1)); + const mc = new MediaCache(b); + mc.index.c1 = { rev: 5, bytes: 0, total: 0, validator: null, complete: false, path: null, uri: null }; + const verdict = await mc.applyChunk('c1', 5, { + status: 200, start: 0, total: CHUNK * 3, validator: '"v1"', body: new Array(CHUNK).fill(1), + }); + assert.equal(verdict, 'progress', "'done' would stop the sweep on an asset with more to fetch"); + assert.equal(mc.localUrl('c1', 5), null); +}); + +test('a server with no validator is given up on, not re-fetched forever', async () => { + // A big asset from a server that sends neither ETag nor Last-Modified can never be resumed, so the + // partial is correctly discarded. Dropping alone meant the next sweep started from zero, pulled + // the same megabyte and discarded it again — every sweep, forever, on precisely the marginal link + // this whole feature exists to be gentle on. + const b = fakeBackend(asset(CHUNK * 4, 3)); + b.asset.etag = null; + const mc = new MediaCache(b); + + await mc.sync([{ content_id: 'c1', content_rev: 5 }], urlFor); + const afterFirst = b.requests; + assert.ok(afterFirst > 0, 'it must at least try once'); + + await mc.sync([{ content_id: 'c1', content_rev: 5 }], urlFor); + await mc.sync([{ content_id: 'c1', content_rev: 5 }], urlFor); + assert.equal(b.requests, afterFirst, 'and then stop asking'); + assert.equal(mc.localUrl('c1', 5), null, 'while never claiming to hold it'); +}); + +test('...and a new revision clears that verdict rather than blacklisting the asset forever', async () => { + const b = fakeBackend(asset(CHUNK * 4, 3)); + b.asset.etag = null; + const mc = new MediaCache(b); + await mc.sync([{ content_id: 'c1', content_rev: 5 }], urlFor); + const afterFirst = b.requests; + + b.asset.etag = '"v2"'; // the server grew validators + await mc.sync([{ content_id: 'c1', content_rev: 6 }], urlFor); + assert.ok(b.requests > afterFirst, 'a fresh revision must be tried again'); + assert.equal(mc.localUrl('c1', 6), 'file:///wgt-private/c1'); +}); + +test('a small asset from a validator-less server still caches — it never needs a resume', async () => { + const b = fakeBackend(asset(Math.floor(CHUNK / 2), 4)); + b.asset.etag = null; + const mc = new MediaCache(b); + await mc.sync([{ content_id: 'c1', content_rev: 5 }], urlFor); + assert.equal(mc.localUrl('c1', 5), 'file:///wgt-private/c1'); +}); diff --git a/server/test/tizen-volume.test.js b/server/test/tizen-volume.test.js new file mode 100644 index 0000000..b4dfe44 --- /dev/null +++ b/server/test/tizen-volume.test.js @@ -0,0 +1,140 @@ +'use strict'; + +/* + * The volume slider, end to end. + * + * The dashboard sends `set_volume` with `{ level: <0..1 fraction> }` + * (frontend/js/views/device-detail.js: `{ level: parseInt(el.value, 10) / 100 }`). The Android + * player reads exactly that (`payload.optDouble("level")`). The Tizen player read `value`/`volume` + * as a 0..100 PERCENTAGE, so it matched nothing the dashboard has ever sent: every slider move + * logged "no usable value in payload" and changed nothing, on a panel that declared audio.volume as + * a working capability. + * + * Two mistakes, and fixing either one alone is worse than fixing neither: + * - the KEY: `level`, not `value`/`volume` + * - the SCALE: a fraction, not a percentage + * Take `level` while still treating it as a percentage and a request for 50% becomes 0.5% — silent, + * and indistinguishable from a slider that works. + * + * The handler is EXECUTED here, lifted out of the shipped app.js the same way the wall-geometry + * parity test lifts the Tizen tile maths. A regex asserting that the file mentions "level" would + * pass on the 0.5%-instead-of-50% version, which is the one failure mode that matters. + */ + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const ROOT = path.join(__dirname, '..', '..'); +const APP = fs.readFileSync(path.join(ROOT, 'tizen', 'js', 'app.js'), 'utf8'); + +/** + * Lift the real applyVolume out of app.js and run it against recording stubs. + * Returns { set(payload) -> {tv, media, warned} }. + */ +function loadHandler({ hasTvAudio = true } = {}) { + const m = /(\n\s*var mediaVolume = null;[\s\S]*?\n function applyVolume\(payload\) \{[\s\S]*?\n \})/.exec(APP); + assert.ok(m, 'could not find applyVolume in tizen/js/app.js'); + + const calls = { tv: null, media: null, logs: [] }; + + // A vm context rather than `new Function`, because the handler reaches STCapabilities as a BARE + // global (`window.STCapabilities ? STCapabilities.tvAudio() : null`) — a local `var window` would + // leave that a ReferenceError, the try/catch would swallow it, and the test would silently + // exercise only the fallback path while claiming to cover the TV one. + const vm = require('node:vm'); + const sandbox = { + STCapabilities: { + tvAudio: () => (hasTvAudio ? { setVolume: (v) => { calls.tv = v; } } : null), + }, + reportCmd: (level, cmd, msg) => calls.logs.push(level + ':' + msg), + Number, Math, isFinite, + __calls: calls, + }; + sandbox.window = sandbox; + vm.createContext(sandbox); + vm.runInContext(` + function applyMediaVolume() { __calls.media = mediaVolume; } + ${m[1]} + `, sandbox); + const harness = sandbox.applyVolume; + assert.equal(typeof harness, 'function'); + + return { + set(payload) { + calls.tv = null; calls.media = null; calls.logs = []; + harness(payload); + return { + tv: calls.tv, + media: calls.media, + warned: calls.logs.some((l) => l.startsWith('warn')), + logs: calls.logs, + }; + }, + }; +} + +test('THE DASHBOARD PAYLOAD: {level: 0..1} reaches the TV as 0..100', () => { + const h = loadHandler(); + // Exactly what frontend/js/views/device-detail.js sends for slider positions 0 / 50 / 100. + assert.deepEqual(h.set({ level: 0 }).tv, 0); + assert.deepEqual(h.set({ level: 0.5 }).tv, 50, 'half volume must be 50, not 0.5'); + assert.deepEqual(h.set({ level: 1 }).tv, 100, 'full volume must be 100, not 1'); + assert.equal(h.set({ level: 0.5 }).warned, false, 'and must not report the payload unusable'); +}); + +test('THE TRAP: reading `level` as a percentage would be inaudible, not merely wrong', () => { + // 0.5 interpreted as a percentage is 0.5% — near silence. It changes the volume, logs success, + // and looks from the dashboard exactly like a working slider. This is the assertion that + // distinguishes a real fix from a plausible one. + const h = loadHandler(); + assert.ok(h.set({ level: 0.5 }).tv > 1, 'a fraction must be scaled, not clamped into near-silence'); +}); + +test('tvaudiocontrol is preferred — it is the volume that reaches the panel speakers', () => { + // Tizen has two volumes and only one of them is audible on a TV: tizen.tvaudiocontrol is the + // SET's own volume and applies to AVPlay video on the hardware plane, which the media elements + // cannot touch at all. Portrait video (#170) plays through AVPlay, so a media-element-only + // implementation would leave a rotated panel at full blast. + const withTv = loadHandler({ hasTvAudio: true }).set({ level: 0.3 }); + assert.equal(withTv.tv, 30); + assert.equal(withTv.media, null, 'the media fallback must not also run'); +}); + +test('...and a build with no TV profile still moves the media elements', () => { + // The URL-Launcher path and a plain browser have no tv.audio surface. Falling through keeps the + // control honest rather than silently doing nothing. + const noTv = loadHandler({ hasTvAudio: false }).set({ level: 0.4 }); + assert.equal(noTv.tv, null); + assert.ok(Math.abs(noTv.media - 0.4) < 1e-9, 'media elements take a 0..1 fraction'); +}); + +test('legacy percentage senders still work, so one dead control is not traded for another', () => { + // The group-command route and hand-issued commands use `value`. These are percentages, not + // fractions — a different key, so there is no ambiguity to resolve. + const h = loadHandler(); + assert.equal(h.set({ value: 25 }).tv, 25); + assert.equal(h.set({ volume: 70 }).tv, 70); +}); + +test('a payload with nothing usable is refused loudly rather than defaulting to silence', () => { + const h = loadHandler(); + const r = h.set({ nothing: true }); + assert.equal(r.tv, null); + assert.ok(r.warned, 'an unusable payload must say so — a silent 0 reads as broken hardware'); +}); + +test('out-of-range values are clamped, not passed through to the panel API', () => { + const h = loadHandler(); + assert.equal(h.set({ level: 5 }).tv, 100); + assert.equal(h.set({ level: -2 }).tv, 0); +}); + +test('the dashboard really does send `level` as a fraction — the other half of the contract', () => { + // Pinned against the sender, because this test is only meaningful while that stays true. If the + // dashboard ever switches to percentages, this fails here instead of on a shop floor. + const ui = fs.readFileSync(path.join(ROOT, 'frontend', 'js', 'views', 'device-detail.js'), 'utf8'); + assert.match(ui, /sendCommand\(device\.id, cmd, \{ level: parseInt\(el\.value, 10\) \/ 100 \}\)/, + 'the set_volume wire format is { level: 0..1 }'); +}); diff --git a/tizen/js/app.js b/tizen/js/app.js index 4c3c007..80b30c9 100644 --- a/tizen/js/app.js +++ b/tizen/js/app.js @@ -530,8 +530,29 @@ // context), and remembers the level so items mounted LATER inherit it — media elements are // created per item, so a one-shot set would last only until the playlist advanced. var mediaVolume = null; // 0..1, null = never set + + /* + * ⚠️ THE WIRE FORMAT IS `level`, AND IT IS A 0..1 FRACTION. + * + * That is what the dashboard sends — `sendCommand(id, 'set_volume', { level: value / 100 })` in + * frontend/js/views/device-detail.js — and what the Android player reads (`optDouble("level")`). + * This handler looked for `value`/`volume` as a 0..100 percentage, so it matched nothing the + * dashboard has ever sent: every slider move reported "no usable value in payload" and changed + * nothing, while the panel declared audio.volume as a working capability. + * + * Both halves had to move together, and that is the trap. Accepting `level` while still treating + * it as a percentage turns a request for 50% into 0.5% — inaudible, indistinguishable from broken, + * and it would have looked exactly like a fix. + */ function applyVolume(payload) { - var pct = payload && (payload.value !== undefined ? payload.value : payload.volume); + var pct; + if (payload && payload.level !== undefined && isFinite(Number(payload.level))) { + pct = Number(payload.level) * 100; // the canonical wire form: a fraction + } else if (payload && payload.value !== undefined) { + pct = payload.value; // legacy/hand-issued: already a percentage + } else if (payload) { + pct = payload.volume; + } var n = Number(pct); if (!isFinite(n)) { reportCmd('warn', 'set_volume', 'no usable value in payload'); return; } n = Math.max(0, Math.min(100, n)); @@ -811,20 +832,22 @@ // that survives an outage and media that does not just means the panel knows precisely what it // cannot show. Deferred off the render path: the sweep is synchronous and this call arrives // while the stage is being repainted. - try {{ + // (The doubled braces this block used to carry were a templating artifact, not syntax: `{{ }}` + // parses as a block inside a block, so it ran correctly and read as a typo in eight places.) + try { if (!window.__stMediaCache && window.MediaCache) window.__stMediaCache = window.MediaCache.create(); var mc = window.__stMediaCache; - if (mc && serverUrl) {{ + if (mc && serverUrl) { var mcItems = payload.assignments || []; var mcBase = serverUrl.replace(/\/+$/, ''); - setTimeout(function () {{ - mc.sync(mcItems, function (it) {{ + setTimeout(function () { + mc.sync(mcItems, function (it) { return mcBase + '/api/content/' + it.content_id + '/file' + (it.content_rev ? '?rev=' + encodeURIComponent(it.content_rev) : ''); - }}); - }}, 2000); - }} - }} catch (e) {{ /* caching must never break the payload path */ }} + }); + }, 2000); + } + } catch (e) { /* caching must never break the payload path */ } // If we have content + we're paired, make sure we're on the stage. if (elPairing.classList.contains('hidden') === false) show(elStage); else if (elStage.classList.contains('hidden')) show(elStage); diff --git a/tizen/js/media-cache.js b/tizen/js/media-cache.js index 5fd8cb8..98b4e02 100644 --- a/tizen/js/media-cache.js +++ b/tizen/js/media-cache.js @@ -81,6 +81,9 @@ e = null; } if (e && e.complete) return Promise.resolve('done'); + // A server that offered no validator for this revision cannot be resumed from, and asking again + // only re-downloads a chunk we already know we will throw away. See applyChunk. + if (e && e.unresumable) return Promise.resolve('stalled'); if (!e) { e = this.index[contentId] = { rev: rev, bytes: 0, total: 0, validator: null, complete: false, path: null, uri: null }; @@ -120,7 +123,11 @@ // WHOLE asset and anything we already hold is wrong. this.drop(contentId); e = this.index[contentId] = { rev: rev, bytes: 0, total: res.total || 0, validator: res.validator || null, complete: false, path: null, uri: null }; - return this.commit(contentId, e, res.body, 0, res.total || (res.body && res.body.length) || 0) ? 'done' : 'stalled'; + if (!this.commit(contentId, e, res.body, 0, res.total || (res.body && res.body.length) || 0)) return 'stalled'; + // 'done' is a claim about the ASSET, not about the write. A 200 whose body is shorter than + // its own Content-Length (a truncated proxy response) wrote successfully and is still + // incomplete; reporting 'done' there stopped the sweep on an asset that had more to fetch. + return e.complete ? 'done' : 'progress'; } if (res.status !== 206) return 'stalled'; @@ -138,7 +145,25 @@ var wrote = this.commit(contentId, e, res.body, e.bytes, res.total); if (!wrote) return 'stalled'; - if (!e.validator && !e.complete) { this.drop(contentId); return 'stalled'; } + + /* + * No validator and more to fetch: there is no safe resume. A later attempt could append the + * tail of a different asset, so the bytes have to go. + * + * What matters is that we then STOP asking. Dropping alone left the entry absent, so the next + * sweep started from zero, pulled the same first megabyte, dropped it again, and did that every + * sweep forever — burning the link this feature exists to be gentle on, permanently, for an + * asset it could never finish. A tombstone at the current revision records "we tried, this + * server will not let us resume" and costs one skipped item instead. Publishing a new revision + * clears it (prune drops anything at a superseded rev), and an asset small enough to arrive in + * one chunk is unaffected — it completes before this branch is reached. + */ + if (!e.validator && !e.complete) { + this.drop(contentId); + this.index[contentId] = { rev: rev, unresumable: true, bytes: 0, total: 0, validator: null, complete: false, path: null, uri: null }; + this.save(); + return 'stalled'; + } return e.complete ? 'done' : 'progress'; }; @@ -237,25 +262,51 @@ /* ------------------------------------------------------------------ * * The Tizen adapter. No decisions live here — only platform calls. + * + * ⚠️ EVERY CALL HERE IS THE 5.0 SYNCHRONOUS FileSystemManager, deliberately, and the version + * before it used the DEPRECATED callback API in a way that could not work at all: + * + * tizen.filesystem.resolve(...) is declared `void`. It hands the directory to a callback and + * returns undefined — so `var dir = tizen.filesystem.resolve(...)` set dir to undefined, + * available() answered false, MediaCache.create() returned null, and the offline cache did + * not exist on a single panel in the field. It failed CLOSED, which is the only reason this + * never showed up as corruption: the capability was correctly withheld, and the feature was + * simply never there. + * File.openStream(...) is asynchronous. `written` was read on the line after the + * call, before any callback could have run, so appendPart returned 0 every time. + * File.moveTo(...) is asynchronous, belongs on the PARENT DIRECTORY, and takes + * (originFullPath, destinationFullPath). It was called on the FILE handle with the + * destination first and a bare name second — three documented errors in one call, each of + * which alone raises IOError. + * + * The 5.0 API (`openFile` -> FileHandle, `toURI`, `pathExists`) is genuinely synchronous, which + * is what the decision layer above actually needs. Tizen 5.0 is the 2019 model year; a 4.0 panel + * has none of it and is told so by available() rather than being handed a cache that writes + * nothing. + * + * There is no rename step. `moveFile` is callback-based even in the 5.0 API, and promoting a + * finished download by renaming it was only ever belt-and-braces: `localUrl` already refuses to + * hand out an entry that is not `complete`, so a partial file at the final name is unreachable. + * Removing the rename removes the last asynchronous operation from this adapter. * ------------------------------------------------------------------ */ - function tizenBackend() { - var dir = null; - try { - // Synchronous resolve is deprecated in newer Web APIs but is what the widget runtime on the - // shipped panels supports; the async form would force this whole module to be callback-based - // for no behavioural gain. - dir = tizen.filesystem.resolve('wgt-private', function (d) { dir = d; }, function () { dir = null; }, 'rw'); - } catch (e) { dir = null; } + var DIR = 'wgt-private/st-media'; - function fileFor(name, create) { - if (!dir) return null; - try { return dir.resolve(name); } catch (e) { /* not there yet */ } - if (!create) return null; - try { return dir.createFile(name); } catch (e) { return null; } - } + function tizenBackend() { + var fsm = null; + try { fsm = (typeof tizen !== 'undefined' && tizen.filesystem) ? tizen.filesystem : null; } + catch (e) { fsm = null; } + + // The whole synchronous surface has to be present. Probing one method and assuming the rest is + // how a half-supported runtime ends up with a cache that half works. + var usable = !!(fsm && + typeof fsm.openFile === 'function' && + typeof fsm.toURI === 'function' && + typeof fsm.pathExists === 'function'); + + function pathFor(contentId) { return DIR + '/' + contentId; } return { - available: function () { return !!dir; }, + available: function () { return usable; }, loadIndex: function () { try { return JSON.parse(localStorage.getItem(INDEX_KEY) || '{}'); } catch (e) { return {}; } }, @@ -286,10 +337,21 @@ body = []; for (var i = 0; i < text.length; i++) body.push(text.charCodeAt(i) & 0xff); } + /* + * total comes from Content-Range on a 206 and from Content-Length ONLY on a 200. + * + * Content-Length on a 206 is the length of the CHUNK, so falling back to it would + * report a 1MB first slice of a 50MB video as a 1MB asset — and the decision layer, + * correctly trusting its input, would mark it complete and hand the panel a truncated + * file to play. It is not hypothetical: a proxy that strips Content-Range, or a CORS + * context where the header is simply not readable, produces exactly this. 0 means + * "unknown", which the decision layer already treats as a stall. + */ + var isPartial = xhr.status === 206; done({ status: xhr.status, start: m ? Number(m[1]) : 0, - total: m ? Number(m[3]) : Number(xhr.getResponseHeader('Content-Length') || 0), + total: m ? Number(m[3]) : (isPartial ? 0 : Number(xhr.getResponseHeader('Content-Length') || 0)), validator: xhr.getResponseHeader('ETag') || xhr.getResponseHeader('Last-Modified') || null, body: body }); @@ -297,37 +359,55 @@ try { xhr.send(null); } catch (e) { done(null); } }); }, + /* + * A POSITIONED write, not an append. + * + * 'a' appends at EOF, so the moment the index and the file disagreed by so much as one chunk + * — a power cut between the write and the index save, which is precisely the event this whole + * feature exists for — every later chunk landed in the wrong place and the panel promoted a + * silently corrupt video. Seeking to the offset makes a repeated write IDEMPOTENT: replaying + * a chunk overwrites the same bytes with the same bytes, so the crash window stops mattering + * instead of being papered over. + */ appendPart: function (contentId, body, offset) { - var f = fileFor(contentId + '.part', true); - if (!f) return 0; - var written = 0; - // 'a' append mode, so a resumed transfer adds to what is already there instead of - // truncating it — which would make every attempt start from zero again. - f.openStream(offset > 0 ? 'a' : 'w', function (stream) { - try { stream.writeBytes(body); written = body.length; } finally { stream.close(); } - }, function () { written = 0; }); - return written; - }, - promotePart: function (contentId) { - var part = fileFor(contentId + '.part', false); - if (!part) return null; + if (!usable || !body || !body.length) return 0; + var fh = null; try { - // Rename rather than copy: an atomic-enough swap, and a copy would need twice the space - // for a large video on a panel that may not have it. - part.moveTo(part.parent.fullPath + '/' + contentId, contentId, true, function () {}, function () {}); - } catch (e) { /* fall through and try to resolve it anyway */ } - var whole = fileFor(contentId, false); - if (!whole) return null; - return { path: whole.fullPath, uri: whole.toURI() }; + // 'w' truncates, which is what a fresh start means; 'rw' keeps what is there to write + // into. makeParents:true creates wgt-private/st-media on first use. + fh = fsm.openFile(pathFor(contentId), offset > 0 ? 'rw' : 'w', true); + if (!fh) return 0; + if (offset > 0) fh.seek(offset, 'BEGIN'); + fh.writeData(new Uint8Array(body)); + if (typeof fh.flush === 'function') fh.flush(); + return body.length; + } catch (e) { + return 0; // no space, no permission, no file — all "did not write" + } finally { + if (fh) { try { fh.close(); } catch (e2) { /* already gone */ } } + } + }, + /* + * Nothing to promote — the bytes have been written to their final name all along. This just + * answers with the URI, and only once the file is really there. + */ + promotePart: function (contentId) { + if (!usable) return null; + var p = pathFor(contentId); + try { + if (!fsm.pathExists(p)) return null; + return { path: p, uri: fsm.toURI(p) }; + } catch (e) { return null; } }, remove: function (contentId) { - if (!dir) return; - [contentId, contentId + '.part'].forEach(function (name) { - try { - var f = dir.resolve(name); - if (f) dir.deleteFile(f.fullPath, function () {}, function () {}); - } catch (e) { /* not present */ } - }); + if (!usable) return; + var p = pathFor(contentId); + try { + if (!fsm.pathExists(p)) return; + // Still callback-based even in the 5.0 API, and best-effort by design: a delete that + // fails costs disk, while blocking on it would cost playback. + fsm.deleteFile(p, function () {}, function () {}); + } catch (e) { /* not present, or refused */ } } }; }