RDKEMW-23773 : Ad Request Failure on Linear CDAI and CDVR TS CDAI

[gurpreet319]

Reason for change: Added support for JSONP and window.location
Test Procedure: Ads should play during Linear Channel
Risk: low
Priority: P2

[Copilot]

Pull request overview

Adds runtime support needed for ad/SDK code that relies on browser-like window.location and dynamic JSONP-style script loading in the JSDOM/MiniJSDOM execution path.

Changes:

• Replaces the inline window.location injection with a helper that derives common location fields (protocol/host/path/query/hash/origin).
• Ensures window and navigator are available on global in the linked JSDOM wrapper.
• Implements a script-tag appendChild hook that fetches external script src content and executes it to enable JSONP-like flows.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File	Description
src/NativeJSRenderer.cpp	Adds setLocation() helper and uses it to inject a fuller window.location before running app code.
src/jsc/modules/linkedjsdomwrapper.js	Exposes window/navigator globally and adds a script appendChild hook to load/execute remote scripts.
include/NativeJSRenderer.h	Declares the new setLocation() helper.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

src/NativeJSRenderer.cpp:754

• linkedjsdomwrapper.js sets document.location = window.location once during initialization, but runApplicationInternal() later overwrites window.location via injected script. That leaves document.location pointing at the stale object, and the URL change detection logic prefers document.location.href when it differs from window.location.href, which can hide the updated URL. Update document.location alongside window.location in the injected script.

    window << "window.location={"
           << "\"href\":\""      << url       << "\","
           << "\"protocol\":\""  << protocol  << "\","
           << "\"host\":\""      << host      << "\","
           << "\"hostname\":\""  << hostname  << "\","
           << "\"port\":\""      << port      << "\","
           << "\"pathname\":\""  << pathname  << "\","
           << "\"search\":\""    << search    << "\","
           << "\"hash\":\""      << hash      << "\","
           << "\"origin\":\""    << origin    << "\""
           << "};";
}

src/NativeJSRenderer.cpp:386

• Building localUrl as "file://" + url produces file:////... when url is an absolute path (starts with /). This results in a non-canonical file URL and can confuse the URL parsing in setLocation (host becomes empty and pathname starts with //).

			    std::stringstream window;
			    std::string localUrl = "file://" + url;
			    setLocation(window, localUrl);
			    NativeJSLogger::log(INFO, "Adding the window location: %s to js file\n", window.str().c_str());

src/NativeJSRenderer.cpp:753

• setLocation() directly interpolates URL components into JavaScript string literals without escaping. If url (or derived parts) contains \, ", or control characters, the injected script becomes invalid JS (or can be used for injection if url is externally supplied via the server API). Escape strings for JavaScript/JSON before writing them into window << ....

    window << "window.location={"
           << "\"href\":\""      << url       << "\","
           << "\"protocol\":\""  << protocol  << "\","
           << "\"host\":\""      << host      << "\","
           << "\"hostname\":\""  << hostname  << "\","
           << "\"port\":\""      << port      << "\","
           << "\"pathname\":\""  << pathname  << "\","
           << "\"search\":\""    << search    << "\","
           << "\"hash\":\""      << hash      << "\","
           << "\"origin\":\""    << origin    << "\""

src/NativeJSRenderer.cpp:725

• setLocation() produces a non-browser-like pathname for URLs with no explicit path (e.g., http://example.com yields an empty pathname, whereas browsers return /). Also, using std::min here relies on <algorithm> being included somewhere else. Consider defaulting pathname to / when pathStart is npos and avoid std::min to remove the header dependency.

        if (pathStart != std::string::npos)
        {
            auto queryPos = url.find('?', pathStart);
            auto hashPos = url.find('#', pathStart);

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (5)

Previously missed (1) — in code that hasn't changed since the last review.

src/NativeJSRenderer.cpp:754

• linkedjsdomwrapper.js sets document.location = window.location once during initialization. Since this code replaces window.location with a new object, document.location can become stale. Keeping them in sync avoids diverging values for code that reads document.location.href.

           << "\"origin\":\""    << origin    << "\""
           << "};";

src/NativeJSRenderer.cpp:742

• For hierarchical URLs like http://example.com (no explicit path) or http://example.com?x=1, pathname ends up empty. Browsers default location.pathname to "/" in these cases, and some clients rely on that.

        if (protocol != "file:")
            origin = protocol + "//" + host;
    }

src/NativeJSRenderer.cpp:728

• std::min is used here but the file does not include <algorithm>, which can cause build failures on toolchains that don't pull it in transitively. Either include <algorithm> or avoid std::min here.

            auto pathEnd = std::min(
                queryPos == std::string::npos ? url.size() : queryPos,
                hashPos  == std::string::npos ? url.size() : hashPos);

include/NativeJSRenderer.h:141

• setLocation is only used as an internal helper in this translation unit (no external callers found). Keeping it public unnecessarily expands the class's public API surface.

                std::string getBaseUserAgent();
		void setLocation(std::stringstream& window, const std::string& url);
	    private:

include/NativeJSRenderer.h:140

• This header now exposes std::stringstream in the class interface but does not include <sstream>. The header may fail to compile when included from a TU that doesn't already include <sstream>.

		void setLocation(std::stringstream& window, const std::string& url);