Merge pull request #1695 from lightpanda-io/iframe_src_nav

Iframe src nav

src/TestHTTPServer.zig

@@ -17,6 +17,7 @@
 // along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
 const std = @import("std");
+const URL = @import("browser/URL.zig");
 
 const TestHTTPServer = @This();
 
@@ -97,7 +98,10 @@ fn handleConnection(self: *TestHTTPServer, conn: std.net.Server.Connection) !voi
 }
 
 pub fn sendFile(req: *std.http.Server.Request, file_path: []const u8) !void {
-    var file = std.fs.cwd().openFile(file_path, .{}) catch |err| switch (err) {
+    var url_buf: [1024]u8 = undefined;
+    var fba = std.heap.FixedBufferAllocator.init(&url_buf);
+    const unescaped_file_path = try URL.unescape(fba.allocator(), file_path);
+    var file = std.fs.cwd().openFile(unescaped_file_path, .{}) catch |err| switch (err) {
         error.FileNotFound => return req.respond("server error", .{ .status = .not_found }),
         else => return err,
     };

src/browser/Page.zig

@@ -236,7 +236,7 @@ version: usize = 0,
 // ScriptManager, so all scripts just count as 1 pending load.
 _pending_loads: u32,
 
-_parent_notified: if (IS_DEBUG) bool else void = if (IS_DEBUG) false else {},
+_parent_notified: bool = false,
 
 _type: enum { root, frame }, // only used for logs right now
 _req_id: u32 = 0,
@@ -346,7 +346,10 @@ pub fn deinit(self: *Page) void {
     session.browser.env.destroyContext(self.js);
 
     self._script_manager.shutdown = true;
-    session.browser.http_client.abort();
+    if (self.parent == null) {
+        // only the root frame needs to abort this. It's more efficient this way
+        session.browser.http_client.abort();
+    }
     self._script_manager.deinit();
 
     if (comptime IS_DEBUG) {
@@ -750,11 +753,15 @@ fn _documentIsComplete(self: *Page) !void {
 }
 
 fn notifyParentLoadComplete(self: *Page) void {
-    if (comptime IS_DEBUG) {
+    if (self._parent_notified == true) {
-        std.debug.assert(self._parent_notified == false);
+        if (comptime IS_DEBUG) {
-        self._parent_notified = true;
+            std.debug.assert(false);
+        }
+        // shouldn't happen, don't want to crash a release build over it
+        return;
     }
 
+    self._parent_notified = true;
     if (self.parent) |p| {
         p.iframeCompletedLoading(self.iframe.?);
     }
@@ -796,7 +803,12 @@ fn pageDataCallback(transfer: *Http.Transfer, data: []const u8) !void {
         } orelse .unknown;
 
         if (comptime IS_DEBUG) {
-            log.debug(.page, "navigate first chunk", .{ .content_type = mime.content_type, .len = data.len, .type = self._type, .url = self.url });
+            log.debug(.page, "navigate first chunk", .{
+                .content_type = mime.content_type,
+                .len = data.len,
+                .type = self._type,
+                .url = self.url,
+            });
         }
 
         switch (mime.content_type) {
@@ -850,7 +862,11 @@ fn pageDoneCallback(ctx: *anyopaque) !void {
     try self._session.navigation.commitNavigation(self);
 
     defer if (comptime IS_DEBUG) {
-        log.debug(.page, "page.load.complete", .{ .url = self.url, .type = self._type });
+        log.debug(.page, "page load complete", .{
+            .url = self.url,
+            .type = self._type,
+            .state = std.meta.activeTag(self._parse_state),
+        });
     };
 
     const parse_arena = try self.getArena(.{ .debug = "Page.parse" });
@@ -962,21 +978,28 @@ pub fn iframeAddedCallback(self: *Page, iframe: *Element.Html.IFrame) !void {
     }
 
     iframe._executed = true;
-
     const session = self._session;
-    const frame_id = session.nextFrameId();
+
+    // A frame can be re-navigated by setting the src.
+    const existing_window = iframe._content_window;
+
     const page_frame = try self.arena.create(Page);
+    const frame_id = blk: {
+        if (existing_window) |w| {
+            const existing_frame_id = w._page._frame_id;
+            session.browser.http_client.abortFrame(existing_frame_id);
+            break :blk existing_frame_id;
+        }
+        break :blk session.nextFrameId();
+    };
+
     try Page.init(page_frame, frame_id, session, self);
+    errdefer page_frame.deinit();
 
     self._pending_loads += 1;
     page_frame.iframe = iframe;
     iframe._content_window = page_frame.window;
-
+    errdefer iframe._content_window = null;
-    self._session.notification.dispatch(.page_frame_created, &.{
-        .frame_id = frame_id,
-        .parent_id = self._frame_id,
-        .timestamp = timestamp(.monotonic),
-    });
 
     // navigate will dupe the url
     const url = try URL.resolve(
@@ -986,6 +1009,15 @@ pub fn iframeAddedCallback(self: *Page, iframe: *Element.Html.IFrame) !void {
         .{ .encode = true },
     );
 
+    if (existing_window == null) {
+        // on first load, dispatch frame_created evnet
+        self._session.notification.dispatch(.page_frame_created, &.{
+            .frame_id = frame_id,
+            .parent_id = self._frame_id,
+            .timestamp = timestamp(.monotonic),
+        });
+    }
+
     page_frame.navigate(url, .{ .reason = .initialFrameNavigation }) catch |err| {
         log.warn(.page, "iframe navigate failure", .{ .url = url, .err = err });
         self._pending_loads -= 1;
@@ -994,6 +1026,25 @@ pub fn iframeAddedCallback(self: *Page, iframe: *Element.Html.IFrame) !void {
         return error.IFrameLoadError;
     };
 
+    if (existing_window) |w| {
+        const existing_page = w._page;
+        if (existing_page._parent_notified == false) {
+            self._pending_loads -= 1;
+        }
+
+        for (self.frames.items, 0..) |p, i| {
+            if (p == existing_page) {
+                self.frames.items[i] = page_frame;
+                break;
+            }
+        } else {
+            lp.assert(false, "Existing frame not found", .{ .len = self.frames.items.len });
+        }
+
+        existing_page.deinit();
+        return;
+    }
+
     // window[N] is based on document order. For now we'll just append the frame
     // at the end of our list and set frames_sorted == false. window.getFrame
     // will check this flag to decide if it needs to sort the frames or not.

src/browser/tests/frames/frames.html

@@ -3,7 +3,7 @@
 
 <script>
   function frame1Onload() {
-    window.f1_onload = true;
+    window.f1_onload = 'f1_onload_loaded';
   }
 </script>
 
@@ -11,6 +11,9 @@
 <iframe id=f2 src="support/sub2.html"></iframe>
 
 <script id="basic">
+   // reload it
+  $('#f2').src = 'support/sub2.html';
+
   testing.eventually(() => {
     testing.expectEqual(undefined, window[10]);
 
@@ -47,8 +50,11 @@
     // child frame's top.parent is itself (root has no parent)
     testing.expectEqual(window, window[0].top.parent);
 
+    // Todo: Context security tokens
     // testing.expectEqual(true, window.sub1_loaded);
     // testing.expectEqual(true, window.sub2_loaded);
+    // testing.expectEqual(1, window.sub1_count);
+    // testing.expectEqual(2, window.sub2_count);
   });
 </script>
 
@@ -63,7 +69,7 @@
     document.documentElement.appendChild(f3);
 
     testing.eventually(() => {
-      testing.expectEqual(true, window.f1_onload);
+      testing.expectEqual('f1_onload_loaded', window.f1_onload);
       testing.expectEqual(true, f3_load_event);
     });
   }

src/browser/tests/frames/support/sub 1.html

@@ -3,4 +3,5 @@
 <script>
   // should not have access to the parent's JS context
   window.top.sub1_loaded = window.testing == undefined;
+  window.top.sub1_count = (window.top.sub1_count || 0) + 1;
 </script>

src/browser/tests/frames/support/sub2.html

@@ -4,4 +4,5 @@
 <script>
   // should not have access to the parent's JS context
   window.top.sub2_loaded = window.testing == undefined;
+  window.top.sub2_count = (window.top.sub2_count || 0) + 1;
 </script>

src/browser/webapi/element/html/IFrame.zig

@@ -58,6 +58,9 @@ pub fn setSrc(self: *IFrame, src: []const u8, page: *Page) !void {
     try element.setAttributeSafe(comptime .wrap("src"), .wrap(src), page);
     self._src = element.getAttributeSafe(comptime .wrap("src")) orelse unreachable;
     if (element.asNode().isConnected()) {
+        // unlike script, an iframe is reloaded every time the src is set
+        // even if it's set to the same URL.
+        self._executed = false;
         try page.iframeAddedCallback(self);
     }
 }