Skip to content

Commit ea2a342

Browse files
committed
refactor: harden browser page host view transfers
1 parent be14f4d commit ea2a342

3 files changed

Lines changed: 72 additions & 11 deletions

File tree

ora/Core/BrowserEngine/BrowserPageView.swift

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,23 +25,29 @@ final class BrowserPageHostView: NSView {
2525

2626
let shouldRestoreFirstResponder = shouldTransferFirstResponder(from: previousContentView)
2727

28-
previousContentView?.removeFromSuperview()
29-
hostedContentView = nil
28+
if isSameContentView {
29+
hostedContentView = nil
30+
} else {
31+
detachHostedContentView()
32+
}
3033

3134
guard let newContentView else {
32-
needsLayout = true
33-
layoutSubtreeIfNeeded()
34-
displayIfNeeded()
35+
refreshHostingLayout()
3536
return
3637
}
3738

3839
configure(contentView: newContentView)
3940

41+
if let previousHost = newContentView.superview as? BrowserPageHostView,
42+
previousHost !== self
43+
{
44+
previousHost.releaseHostedContentView(newContentView)
45+
}
46+
4047
if newContentView.superview !== self {
41-
if let previousHost = newContentView.superview as? BrowserPageHostView {
42-
previousHost.hostedContentView = nil
48+
if newContentView.superview != nil {
49+
newContentView.removeFromSuperview()
4350
}
44-
newContentView.removeFromSuperview()
4551
addSubview(newContentView)
4652
}
4753

@@ -72,6 +78,33 @@ final class BrowserPageHostView: NSView {
7278
contentView.layer?.drawsAsynchronously = true
7379
}
7480

81+
private func detachHostedContentView() {
82+
guard let hostedContentView else {
83+
return
84+
}
85+
86+
if hostedContentView.superview === self {
87+
hostedContentView.removeFromSuperview()
88+
}
89+
90+
self.hostedContentView = nil
91+
}
92+
93+
private func releaseHostedContentView(_ contentView: NSView) {
94+
guard hostedContentView === contentView else {
95+
return
96+
}
97+
98+
detachHostedContentView()
99+
refreshHostingLayout()
100+
}
101+
102+
private func refreshHostingLayout() {
103+
needsLayout = true
104+
layoutSubtreeIfNeeded()
105+
displayIfNeeded()
106+
}
107+
75108
private func shouldTransferFirstResponder(from previousContentView: NSView?) -> Bool {
76109
guard let previousContentView,
77110
let firstResponder = window?.firstResponder as? NSView

ora/Features/Tabs/Switcher/FloatingTabSwitcher.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,12 +99,12 @@ struct FloatingTabSwitcher: View {
9999
}
100100
}
101101
}
102-
.padding(.horizontal, 28)
103-
.padding(.vertical, 20)
102+
.padding(.horizontal, 20)
103+
.padding(.vertical, 18)
104104
.background(BlurEffectView(material: .popover, blendingMode: .withinWindow))
105105
.background(theme.background.opacity(0.3))
106106
.cornerRadius(Constants.containerCornerRadius)
107-
.shadow(color: .blue.opacity(0.07), radius: 16, x: 0, y: 12)
107+
.shadow(color: theme.primary.opacity(0.07), radius: 16, x: 0, y: 12)
108108
.background(keyboardHandler)
109109
.overlay(containerBorder)
110110
}

oraTests/BrowserPageHostViewTests.swift

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,28 @@ struct BrowserPageHostViewTests {
7171
#expect(secondHost.subviews.first === contentView)
7272
#expect(secondHost.hostedContentView === contentView)
7373
#expect(contentView.superview === secondHost)
74+
#expect(contentView.removeFromSuperviewCalls == 1)
75+
}
76+
77+
@Test func switchingToAStaleSubviewAlreadyAttachedToTheSameHostDoesNotReparentIt() {
78+
let host = makeHost()
79+
let firstContentView = TrackingContentView()
80+
let staleContentView = TrackingContentView()
81+
82+
host.host(contentView: firstContentView)
83+
host.addSubview(staleContentView)
84+
85+
let staleSuperviewTransitions = staleContentView.superviewTransitions
86+
87+
host.host(contentView: staleContentView)
88+
89+
#expect(host.subviews.count == 1)
90+
#expect(host.subviews.first === staleContentView)
91+
#expect(host.hostedContentView === staleContentView)
92+
#expect(firstContentView.superview == nil)
93+
#expect(staleContentView.superview === host)
94+
#expect(staleContentView.superviewTransitions == staleSuperviewTransitions)
95+
#expect(staleContentView.removeFromSuperviewCalls == 0)
7496
}
7597

7698
private func makeHost() -> BrowserPageHostView {
@@ -80,6 +102,12 @@ struct BrowserPageHostViewTests {
80102

81103
private final class TrackingContentView: NSView {
82104
var superviewTransitions = 0
105+
var removeFromSuperviewCalls = 0
106+
107+
override func removeFromSuperview() {
108+
removeFromSuperviewCalls += 1
109+
super.removeFromSuperview()
110+
}
83111

84112
override func viewWillMove(toSuperview newSuperview: NSView?) {
85113
superviewTransitions += 1

0 commit comments

Comments
 (0)