Commit d904b4ad authored by Guillaume Jenkins's avatar Guillaume Jenkins Committed by Commit Bot

[iOS Shortcuts] Fix opening multiple URLs when tab switcher is empty

When opening multiple URLs via Siri Shortcuts while the tab switcher
was opened and empty, only the first 3 URLs were successfully opened.

The cause is as follows:
- URL 1 opens and causes the tab switcher to dismiss asynchronously,
  but callback 1 is still run immediately
- Callback 1 causes URL 2 to start opening, but it must wait for the
  dismiss animation to finish, so callback 2 is stored in the BVC
  properties
- Callback 2 eventually runs, and does two things:
    1) open URL 3
    2) append callback 3 to the BVC tab opening callbacks
- Immediately after callback 2 finishes running, the BVC clears its
  callback property, erasing callback 3 that was just set by
  callback 2
- URL 3 is opened by callback 2, but callback 3 is never run because
  by the time the BVC tries to invoke the tab opening callback, it was
  already set to nil after running callback 2

The simplest fix I found is to simply clear the callback property
before running the callback (the callback is stored to a local var
before clearing the property). That way, callbacks can append new
tab opening callbacks without fear of them being erased.

Bug: 1124291
Change-Id: Ib3684f4ce93136c28bb55711b4a5742510082302
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2405897Reviewed-by: default avatarOlivier Robin <olivierrobin@chromium.org>
Reviewed-by: default avatarMark Cogan <marq@chromium.org>
Reviewed-by: default avatarJustin Cohen <justincohen@chromium.org>
Commit-Queue: Guillaume Jenkins <gujen@google.com>
Cr-Commit-Position: refs/heads/master@{#807487}
parent 60181571
...@@ -4357,8 +4357,16 @@ NSString* const kBrowserViewControllerSnackbarCategory = ...@@ -4357,8 +4357,16 @@ NSString* const kBrowserViewControllerSnackbarCategory =
dispatch_async(dispatch_get_main_queue(), ^{ dispatch_async(dispatch_get_main_queue(), ^{
// Test existence again as the block may have been deleted. // Test existence again as the block may have been deleted.
if (self.foregroundTabWasAddedCompletionBlock) { if (self.foregroundTabWasAddedCompletionBlock) {
self.foregroundTabWasAddedCompletionBlock(); // Clear the property before executing the completion, in case the
// completion calls appendTabAddedCompletion:tabAddedCompletion.
// Clearing the property after running the completion would cause any
// newly appended completion to be immediately cleared without ever
// getting run. An example where this would happen is when opening
// multiple tabs via the "Open URLs in Chrome" Siri Shortcut.
ProceduralBlock completion =
self.foregroundTabWasAddedCompletionBlock;
self.foregroundTabWasAddedCompletionBlock = nil; self.foregroundTabWasAddedCompletionBlock = nil;
completion();
} }
}); });
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment