Skip to content

Commit

Permalink
Fix closing tabs quickly (#801)
Browse files Browse the repository at this point in the history
* fix closing tabs quickly

* update evilHackToClearLastLeftHitInWindow

* fix test

* Fix mouse-exit events not handled correctly

* minor adjustment
  • Loading branch information
mallexxx authored Oct 28, 2022
1 parent 79a9623 commit 51267d1
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 28 deletions.
4 changes: 4 additions & 0 deletions DuckDuckGo.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,7 @@
B693956126F1C1BC0015B914 /* DownloadListStoreMock.swift in Sources */ = {isa = PBXBuildFile; fileRef = B693956026F1C1BC0015B914 /* DownloadListStoreMock.swift */; };
B693956326F1C2A40015B914 /* FileDownloadManagerMock.swift in Sources */ = {isa = PBXBuildFile; fileRef = B693956226F1C2A40015B914 /* FileDownloadManagerMock.swift */; };
B693956926F352DB0015B914 /* DownloadsWebViewMock.m in Sources */ = {isa = PBXBuildFile; fileRef = B693956826F352DB0015B914 /* DownloadsWebViewMock.m */; };
B698E5042908011E00A746A8 /* AppKitPrivateMethodsAvailabilityTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B698E5032908011E00A746A8 /* AppKitPrivateMethodsAvailabilityTests.swift */; };
B69B503A2726A12500758A2B /* StatisticsLoader.swift in Sources */ = {isa = PBXBuildFile; fileRef = B69B50342726A11F00758A2B /* StatisticsLoader.swift */; };
B69B503B2726A12500758A2B /* Atb.swift in Sources */ = {isa = PBXBuildFile; fileRef = B69B50352726A11F00758A2B /* Atb.swift */; };
B69B503C2726A12500758A2B /* StatisticsStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = B69B50362726A12000758A2B /* StatisticsStore.swift */; };
Expand Down Expand Up @@ -1555,6 +1556,7 @@
B693956626F352940015B914 /* TestsBridging.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = TestsBridging.h; sourceTree = "<group>"; };
B693956726F352DB0015B914 /* DownloadsWebViewMock.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = DownloadsWebViewMock.h; sourceTree = "<group>"; };
B693956826F352DB0015B914 /* DownloadsWebViewMock.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = DownloadsWebViewMock.m; sourceTree = "<group>"; };
B698E5032908011E00A746A8 /* AppKitPrivateMethodsAvailabilityTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppKitPrivateMethodsAvailabilityTests.swift; sourceTree = "<group>"; };
B69B50342726A11F00758A2B /* StatisticsLoader.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = StatisticsLoader.swift; sourceTree = "<group>"; };
B69B50352726A11F00758A2B /* Atb.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Atb.swift; sourceTree = "<group>"; };
B69B50362726A12000758A2B /* StatisticsStore.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = StatisticsStore.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2738,6 +2740,7 @@
4BA1A6CE258BF58C00F6F690 /* File System */,
B6AE74322609AFBB005B9B1A /* Progress */,
4B0511E6262CAB3700F6079C /* UserDefaultsWrapperUtilities.swift */,
B698E5032908011E00A746A8 /* AppKitPrivateMethodsAvailabilityTests.swift */,
B6B3E0952654DACD0040E0A2 /* UTTypeTests.swift */,
B693956626F352940015B914 /* TestsBridging.h */,
);
Expand Down Expand Up @@ -5181,6 +5184,7 @@
3767190028E58513003A2A15 /* PrivatePlayerURLExtensionTests.swift in Sources */,
B6B3E0962654DACD0040E0A2 /* UTTypeTests.swift in Sources */,
4B2975992828285900187C4E /* FirefoxKeyReaderTests.swift in Sources */,
B698E5042908011E00A746A8 /* AppKitPrivateMethodsAvailabilityTests.swift in Sources */,
4B02199D25E063DE00ED7DEA /* FireproofingURLExtensionsTests.swift in Sources */,
4BB99D0F26FE1A84001E4761 /* ChromiumBookmarksReaderTests.swift in Sources */,
4BB99D1026FE1A84001E4761 /* FirefoxBookmarksReaderTests.swift in Sources */,
Expand Down
2 changes: 1 addition & 1 deletion DuckDuckGo/Common/Extensions/NSViewExtension.swift
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ extension NSView {

func mouseLocationInsideBounds(_ point: NSPoint?) -> NSPoint? {
withMouseLocationInViewCoordinates(point) { locationInView in
guard self.bounds.contains(locationInView) else { return nil }
guard self.visibleRect.contains(locationInView) else { return nil }
return locationInView
}
}
Expand Down
23 changes: 23 additions & 0 deletions DuckDuckGo/Common/Extensions/NSWindowExtension.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,27 @@ extension NSWindow {
setFrameOrigin(frameOrigin)
}

private static let lastLeftHitKey = "_lastLeftHit"
var lastLeftHit: NSView? {
return try? NSException.catch {
self.value(forKey: Self.lastLeftHitKey) as? NSView
}
}

func evilHackToClearLastLeftHitInWindow() {
guard let oldValue = self.lastLeftHit else { return }
let oldValueRetainCount = CFGetRetainCount(oldValue)
defer {
// compensate unbalanced release call
if CFGetRetainCount(oldValue) < oldValueRetainCount {
_=Unmanaged.passUnretained(oldValue).retain()
}
}
NSException.try {
autoreleasepool {
self.setValue(nil, forKey: Self.lastLeftHitKey)
}
}
}

}
28 changes: 1 addition & 27 deletions DuckDuckGo/Common/View/AppKit/MouseClickView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,43 +40,19 @@ final class MouseClickView: NSView {

weak var delegate: MouseClickViewDelegate?

private func repostMultiClickEventIfNeeded(_ event: NSEvent) -> Bool {
// don't let more-than-doubleclicks get in
guard event.clickCount > 2,
let newEvent = NSEvent.mouseEvent(with: event.type,
location: event.locationInWindow,
modifierFlags: event.modifierFlags,
timestamp: event.timestamp,
windowNumber: event.windowNumber,
context: nil,
eventNumber: event.eventNumber,
clickCount: 1,
pressure: event.pressure),
let window = self.window
else { return false }

// break the growing clickCount event cycle by sending a new 1-click event to the window
window.postEvent(newEvent, atStart: true)
return true
}

override func mouseDown(with event: NSEvent) {
let coordinateInWindow = event.locationInWindow
let coordinateInView = self.convert(coordinateInWindow, from: nil)

if !self.bounds.contains(coordinateInView) {
return
}

guard !repostMultiClickEventIfNeeded(event) else { return }

super.mouseDown(with: event)
delegate?.mouseClickView(self, mouseDownEvent: event)
}

override func mouseUp(with event: NSEvent) {
guard !repostMultiClickEventIfNeeded(event) else { return }

super.mouseUp(with: event)
delegate?.mouseClickView(self, mouseUpEvent: event)
}
Expand All @@ -88,8 +64,6 @@ final class MouseClickView: NSView {
}

override func otherMouseDown(with event: NSEvent) {
guard !repostMultiClickEventIfNeeded(event) else { return }

super.otherMouseDown(with: event)

delegate?.mouseClickView(self, otherMouseDownEvent: event)
Expand Down
6 changes: 6 additions & 0 deletions DuckDuckGo/Tab Bar/View/TabBarViewItem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,12 @@ final class TabBarViewItem: NSCollectionViewItem {
private var lastKnownIndexPath: IndexPath?

@IBAction func closeButtonAction(_ sender: NSButton) {
// due to async nature of NSCollectionView views removal
// leaving window._lastLeftHit set to the button will prevent
// continuous clicks on the Close button
// this should be removed when the Tab Bar is redone without NSCollectionView
sender.window?.evilHackToClearLastLeftHitInWindow()

guard let indexPath = self.collectionView?.indexPath(for: self) else {
// doubleclick event arrived at point when we're already removed
// pass the closeButton action to the next TabBarViewItem
Expand Down
87 changes: 87 additions & 0 deletions Unit Tests/Common/AppKitPrivateMethodsAvailabilityTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
//
// AppKitPrivateMethodsAvailabilityTests.swift
//
// Copyright © 2022 DuckDuckGo. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

import XCTest
import AppKit
@testable import DuckDuckGo_Privacy_Browser

final class AppKitPrivateMethodsAvailabilityTests: XCTestCase {

func testLastLeftHitViewIsReleasedCorrectly() {
var window: NSWindow!
autoreleasepool {
window = NSWindow()
window.isReleasedWhenClosed = false

let view = TestHitView(frame: NSRect(x: 0, y: 0, width: 100, height: 100))
window.contentView = view

window.setFrame(NSRect(x: 0, y: 0, width: 100, height: 123), display: false)
NSApp.activate(ignoringOtherApps: true)

let didAppearExpectation = expectation(description: "view did appear")
window.makeKeyAndOrderFront(nil)
DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) {
didAppearExpectation.fulfill()
}
waitForExpectations(timeout: 1.0)

view.mouseDownExpectation = expectation(description: "mouseDown received")

let event = NSEvent.mouseEvent(with: .leftMouseDown,
location: NSPoint(x: 50, y: 50),
modifierFlags: [],
timestamp: CACurrentMediaTime(),
windowNumber: window.windowNumber,
context: nil,
eventNumber: -22966,
clickCount: 1,
pressure: 1)!
window.postEvent(event, atStart: false)

waitForExpectations(timeout: 0.1)

// window should have lastLeftHit set to the clicked view after a click event
XCTAssertEqual(window.lastLeftHit, view)
window.evilHackToClearLastLeftHitInWindow()
XCTAssertEqual(window.lastLeftHit, nil)

view.deinitExpectation = expectation(description: "deinit called")
window.close()
window = nil
}

waitForExpectations(timeout: 0.5)
}

}

private class TestHitView: NSView {
var mouseDownExpectation: XCTestExpectation!
var deinitExpectation: XCTestExpectation!

override func mouseDown(with event: NSEvent) {
mouseDownExpectation.fulfill()
super.mouseDown(with: event)
}

deinit {
deinitExpectation.fulfill()
}

}

0 comments on commit 51267d1

Please sign in to comment.