Skip to content

Commit

Permalink
Fix the issue with MMMNavigation holding the current handler for too …
Browse files Browse the repository at this point in the history
…long

The current handler was captured in `continueRequest:` after calling
`performNavigationRequest` on the next handler, which could be too late if the
request has been marked as finished in that call.
  • Loading branch information
aleh committed Jun 10, 2024
1 parent d5fd6f3 commit 30ac40f
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 18 deletions.
2 changes: 1 addition & 1 deletion MMMCommonUI.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
Pod::Spec.new do |s|

s.name = "MMMCommonUI"
s.version = "3.12.0"
s.version = "3.12.1"
s.summary = "Small UI-related pieces reused in many components from MMMTemple"
s.description = "#{s.summary}."
s.homepage = "https://github.com/mediamonks/#{s.name}"
Expand Down
45 changes: 28 additions & 17 deletions Sources/MMMCommonUIObjC/MMMNavigation.m
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ - (void)checkRequestQueue {
}

if ([_requestQueue count] == 0) {
MMM_LOG_TRACE(@"No more pending navigation requests");
MMM_LOG_TRACE(@"No more navigation requests");
return;
}

Expand All @@ -292,12 +292,12 @@ - (void)checkRequestQueue {
_currentRequest = [_requestQueue firstObject];
[_requestQueue removeObjectAtIndex:0];

MMM_LOG_TRACE(@"Will be executing %@", _currentRequest);
MMM_LOG_TRACE(@"Handling %@", _currentRequest);

for (MMMNavigationHandlerInfo *handlerInfo in _handlers) {
_currentHandler = handlerInfo.handler;
if ([_currentHandler performNavigationRequest:_currentRequest]) {
MMM_LOG_TRACE(@"The current request was accepted by %@", _currentHandler);
MMM_LOG_TRACE(@"The request was accepted/performed by %@", _currentHandler);
return;
}
}
Expand All @@ -313,7 +313,12 @@ - (void)checkRequestQueue {
- (void)didFinishRequest:(MMMNavigationRequest *)request successfully:(BOOL)successfully {

NSAssert([NSThread isMainThread], @"");
NSAssert(request == _currentRequest, @"");

if (request != _currentRequest) {
// Are you calling it twice?
MMM_LOG_ERROR(@"Ignoring an attempt to finish a non-current %@", request);
return;
}

MMMNavigationCompletionBlock completionBlock = _currentRequest.completion;

Expand All @@ -334,23 +339,29 @@ - (void)didFinishRequest:(MMMNavigationRequest *)request successfully:(BOOL)succ
- (void)continueRequest:(MMMNavigationRequest *)request path:(MMMNavigationPath *)path handler:(id<MMMNavigationHandler>)handler {

NSAssert([NSThread isMainThread], @"");
NSAssert(request == _currentRequest, @"");

if ([handler conformsToProtocol:@protocol(MMMNavigationHandler)] && [handler performNavigationRequest:_currentRequest]) {

_currentHandler = handler;
MMM_LOG_TRACE(@"The request is continued by %@", [MMMCommonCoreHelpers typeName:_currentHandler]);
if (request != _currentRequest) {
// Did you mark the request as finished before continuing the request?
MMM_LOG_TRACE(@"Ignoring an attempt to continue a non-current %@", request);
return;
}

NSString *name = [MMMCommonCoreHelpers typeName:handler];
_currentHandler = handler;
MMM_LOG_TRACE(@"The request will be continued by %@", name);
if ([handler conformsToProtocol:@protocol(MMMNavigationHandler)] && [handler performNavigationRequest:_currentRequest]) {
// The handler has accepted it, but not tracing as it could have marked the request as finished in that call.
} else {

if ([path.hops count] == 0) {

MMM_LOG_TRACE(@"All hops are handled");
[self didFinishRequest:_currentRequest successfully:YES];

if (request == _currentRequest) {
if ([path.hops count] == 0) {
MMM_LOG_TRACE(@"%@ has not accept it, but all hops are handled, so assuming it's completed", name);
[self didFinishRequest:_currentRequest successfully:YES];
} else {
MMM_LOG_ERROR(@"Cannot finish the current request: remaining hops (%@) cannot be continued by %@", request.path, handler);
[self didFinishRequest:_currentRequest successfully:NO];
}
} else {
MMM_LOG_ERROR(@"Cannot finish the current request (have some more hops, %@, but they cannot be continued by %@)", request.path, handler);
[self didFinishRequest:_currentRequest successfully:NO];
// The handler has not accepted it, but it looks like the request was marked as failed as well.
}
}
}
Expand Down

0 comments on commit 30ac40f

Please sign in to comment.