Commit d76bdbba authored by Robbie Gibson's avatar Robbie Gibson Committed by Commit Bot

[iOS] Prevent re-entrance in omnibox edit menu clipboard

As seen in the attached bug, sometimes accessing the pasteboard from
the PasteboardChangeNotification callback causes the notification to be
posted again, causing an infinite recursion and crash. The bug is most
likely on the Apple end, but without reproduction steps, I'm not sure
if they will respond. Nevertheless, we can work around this by only
updating in response to the notification once at a time. Then the second
notification will be ignored.

Bug: 1049053
Change-Id: If15c8d6b17d39b927ac4d21a643de28fa3851a2a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2360021
Commit-Queue: Robbie Gibson <rkgibson@google.com>
Reviewed-by: default avatarStepan Khapugin <stkhapugin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#801366}
parent a3592b13
...@@ -87,6 +87,9 @@ const NSString* kScribbleOmniboxElementId = @"omnibox"; ...@@ -87,6 +87,9 @@ const NSString* kScribbleOmniboxElementId = @"omnibox";
// Stores the current content type in the clipboard. This is only valid if // Stores the current content type in the clipboard. This is only valid if
// |hasCopiedContent| is YES. // |hasCopiedContent| is YES.
@property(nonatomic, assign) ClipboardContentType copiedContentType; @property(nonatomic, assign) ClipboardContentType copiedContentType;
// Stores whether the cached clipboard state is currently being updated. See
// |-updateCachedClipboardState| for more information.
@property(nonatomic, assign) BOOL isUpdatingCachedClipboardState;
// Starts voice search, updating the NamedGuide to be constrained to the // Starts voice search, updating the NamedGuide to be constrained to the
// trailing button. // trailing button.
...@@ -574,6 +577,14 @@ const NSString* kScribbleOmniboxElementId = @"omnibox"; ...@@ -574,6 +577,14 @@ const NSString* kScribbleOmniboxElementId = @"omnibox";
} }
- (void)updateCachedClipboardState { - (void)updateCachedClipboardState {
// Sometimes, checking the clipboard state itself causes the clipboard to
// emit a UIPasteboardChangedNotification, leading to an infinite loop. For
// now, just prevent re-checking the clipboard state, but hopefully this will
// be fixed in a future iOS version (see crbug.com/1049053 for crash details).
if (self.isUpdatingCachedClipboardState) {
return;
}
self.isUpdatingCachedClipboardState = YES;
self.hasCopiedContent = NO; self.hasCopiedContent = NO;
ClipboardRecentContent* clipboardRecentContent = ClipboardRecentContent* clipboardRecentContent =
ClipboardRecentContent::GetInstance(); ClipboardRecentContent::GetInstance();
...@@ -597,6 +608,7 @@ const NSString* kScribbleOmniboxElementId = @"omnibox"; ...@@ -597,6 +608,7 @@ const NSString* kScribbleOmniboxElementId = @"omnibox";
matched_types.end()) { matched_types.end()) {
weakSelf.copiedContentType = ClipboardContentType::Text; weakSelf.copiedContentType = ClipboardContentType::Text;
} }
weakSelf.isUpdatingCachedClipboardState = NO;
})); }));
} }
......
...@@ -89,6 +89,9 @@ const CGFloat kClearButtonSize = 28.0f; ...@@ -89,6 +89,9 @@ const CGFloat kClearButtonSize = 28.0f;
// Stores the current content type in the clipboard. This is only valid if // Stores the current content type in the clipboard. This is only valid if
// |hasCopiedContent| is YES. // |hasCopiedContent| is YES.
@property(nonatomic, assign) ClipboardContentType copiedContentType; @property(nonatomic, assign) ClipboardContentType copiedContentType;
// Stores whether the cached clipboard state is currently being updated. See
// |-updateCachedClipboardState| for more information.
@property(nonatomic, assign) BOOL isUpdatingCachedClipboardState;
@end @end
...@@ -414,6 +417,14 @@ const CGFloat kClearButtonSize = 28.0f; ...@@ -414,6 +417,14 @@ const CGFloat kClearButtonSize = 28.0f;
} }
- (void)updateCachedClipboardState { - (void)updateCachedClipboardState {
// Sometimes, checking the clipboard state itself causes the clipboard to
// emit a UIPasteboardChangedNotification, leading to an infinite loop. For
// now, just prevent re-checking the clipboard state, but hopefully this will
// be fixed in a future iOS version (see crbug.com/1049053 for crash details).
if (self.isUpdatingCachedClipboardState) {
return;
}
self.isUpdatingCachedClipboardState = YES;
self.hasCopiedContent = NO; self.hasCopiedContent = NO;
ClipboardRecentContent* clipboardRecentContent = ClipboardRecentContent* clipboardRecentContent =
ClipboardRecentContent::GetInstance(); ClipboardRecentContent::GetInstance();
...@@ -437,6 +448,7 @@ const CGFloat kClearButtonSize = 28.0f; ...@@ -437,6 +448,7 @@ const CGFloat kClearButtonSize = 28.0f;
matched_types.end()) { matched_types.end()) {
weakSelf.copiedContentType = ClipboardContentType::Text; weakSelf.copiedContentType = ClipboardContentType::Text;
} }
self.isUpdatingCachedClipboardState = NO;
})); }));
} }
......
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