Commit e679b81e authored by Christopher Cameron's avatar Christopher Cameron Committed by Commit Bot

RemoteRWHVMac: Fix missed nullptr check in firstRectForCharacterRange

In crrev.com/549621, the RenderWidgetHostViewCocoa method
firstRectForCharacterRange was changed from doing an early-out if
!renderWidgetHostView_->GetFocusedWidget() to doing an early-out if
clientWasDestroyed_.

These are not the same thing -- clientWasDestroyed_ is equivalent to
checking host(). Now we're seeing crashes downstream because
OnNSViewSyncGetFirstRectForRange is getting called when
renderWidgetHostView_->GetFocusedWidget() is nullptr.

Fold firstViewRectForCharacterRange into firstRectForCharacterRange,
(since it had no other callers), and update the synchronous method
OnNSViewSyncGetFirstRectForRange to query GetFocusedWidget() and
return a success parameter indicating if it is non-nullptr.

Bug: 835272, 821651
Change-Id: I5063ed3c154cc1c1efc1f1e903153efc9aef66d1
Reviewed-on: https://chromium-review.googlesource.com/1024411
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: default avatarccameron <ccameron@chromium.org>
Reviewed-by: default avatarSidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552856}
parent 29fc747e
...@@ -128,11 +128,13 @@ class RenderWidgetHostNSViewClient { ...@@ -128,11 +128,13 @@ class RenderWidgetHostNSViewClient {
// Synchronously query the composition character boundary rectangle and return // Synchronously query the composition character boundary rectangle and return
// it in |*rect|. Set |*actual_range| to the range actually used for the // it in |*rect|. Set |*actual_range| to the range actually used for the
// returned rectangle. // returned rectangle. If there was no focused RenderWidgetHost to query,
// then set |*success| to false.
virtual void OnNSViewSyncGetFirstRectForRange( virtual void OnNSViewSyncGetFirstRectForRange(
const gfx::Range& requested_range, const gfx::Range& requested_range,
gfx::Rect* rect, gfx::Rect* rect,
gfx::Range* actual_range) = 0; gfx::Range* actual_range,
bool* success) = 0;
// Forward the corresponding edit menu command to the RenderWidgetHost's // Forward the corresponding edit menu command to the RenderWidgetHost's
// delegate. // delegate.
......
...@@ -199,8 +199,6 @@ struct DidOverscrollParams; ...@@ -199,8 +199,6 @@ struct DidOverscrollParams;
// Confirm ongoing composition. // Confirm ongoing composition.
- (void)finishComposingText; - (void)finishComposingText;
- (void)updateCursor:(NSCursor*)cursor; - (void)updateCursor:(NSCursor*)cursor;
- (NSRect)firstViewRectForCharacterRange:(NSRange)theRange
actualRange:(NSRangePointer)actualRange;
- (void)tabletEvent:(NSEvent*)theEvent; - (void)tabletEvent:(NSEvent*)theEvent;
- (void)quickLookWithEvent:(NSEvent*)event; - (void)quickLookWithEvent:(NSEvent*)event;
- (void)showLookUpDictionaryOverlayFromRange:(NSRange)range; - (void)showLookUpDictionaryOverlayFromRange:(NSRange)range;
......
...@@ -107,7 +107,8 @@ class NoopClient : public RenderWidgetHostNSViewClient { ...@@ -107,7 +107,8 @@ class NoopClient : public RenderWidgetHostNSViewClient {
uint32_t* index) override {} uint32_t* index) override {}
void OnNSViewSyncGetFirstRectForRange(const gfx::Range& requested_range, void OnNSViewSyncGetFirstRectForRange(const gfx::Range& requested_range,
gfx::Rect* rect, gfx::Rect* rect,
gfx::Range* actual_range) override {} gfx::Range* actual_range,
bool* success) override {}
void OnNSViewExecuteEditCommand(const std::string& command) override {} void OnNSViewExecuteEditCommand(const std::string& command) override {}
void OnNSViewUndo() override {} void OnNSViewUndo() override {}
void OnNSViewRedo() override {} void OnNSViewRedo() override {}
...@@ -1487,34 +1488,28 @@ extern NSString* NSTextInputReplacementRangeAttributeName; ...@@ -1487,34 +1488,28 @@ extern NSString* NSTextInputReplacementRangeAttributeName;
return NSUInteger(char_index); return NSUInteger(char_index);
} }
- (NSRect)firstViewRectForCharacterRange:(NSRange)theRange - (NSRect)firstRectForCharacterRange:(NSRange)theRange
actualRange:(NSRangePointer)actualRange { actualRange:(NSRangePointer)actualRange {
gfx::Rect gfxRect; gfx::Rect gfxRect;
gfx::Range gfxActualRange; gfx::Range gfxActualRange;
bool success = false;
if (actualRange) if (actualRange)
gfxActualRange = gfx::Range(*actualRange); gfxActualRange = gfx::Range(*actualRange);
client_->OnNSViewSyncGetFirstRectForRange(gfx::Range(theRange), &gfxRect, client_->OnNSViewSyncGetFirstRectForRange(gfx::Range(theRange), &gfxRect,
&gfxActualRange); &gfxActualRange, &success);
if (!success) {
// The call to cancelComposition comes from https://crrev.com/350261.
[self cancelComposition];
return NSZeroRect;
}
if (actualRange) if (actualRange)
*actualRange = gfxActualRange.ToNSRange(); *actualRange = gfxActualRange.ToNSRange();
// The returned rectangle is in WebKit coordinates (upper left origin), so // The returned rectangle is in WebKit coordinates (upper left origin), so
// flip the coordinate system. // flip the coordinate system.
NSRect viewFrame = [self frame]; NSRect viewFrame = [self frame];
NSRect flippedRect = NSRectFromCGRect(gfxRect.ToCGRect()); NSRect rect = NSRectFromCGRect(gfxRect.ToCGRect());
flippedRect.origin.y = NSHeight(viewFrame) - NSMaxY(flippedRect); rect.origin.y = NSHeight(viewFrame) - NSMaxY(rect);
return flippedRect;
}
- (NSRect)firstRectForCharacterRange:(NSRange)theRange
actualRange:(NSRangePointer)actualRange {
if ([self clientIsDisconnected]) {
[self cancelComposition];
return NSZeroRect;
}
NSRect rect =
[self firstViewRectForCharacterRange:theRange actualRange:actualRange];
// Convert into screen coordinates for return. // Convert into screen coordinates for return.
rect = [self convertRect:rect toView:nil]; rect = [self convertRect:rect toView:nil];
......
...@@ -334,7 +334,8 @@ class CONTENT_EXPORT RenderWidgetHostViewMac ...@@ -334,7 +334,8 @@ class CONTENT_EXPORT RenderWidgetHostViewMac
uint32_t* index) override; uint32_t* index) override;
void OnNSViewSyncGetFirstRectForRange(const gfx::Range& requested_range, void OnNSViewSyncGetFirstRectForRange(const gfx::Range& requested_range,
gfx::Rect* rect, gfx::Rect* rect,
gfx::Range* actual_range) override; gfx::Range* actual_range,
bool* success) override;
void OnNSViewExecuteEditCommand(const std::string& command) override; void OnNSViewExecuteEditCommand(const std::string& command) override;
void OnNSViewUndo() override; void OnNSViewUndo() override;
void OnNSViewRedo() override; void OnNSViewRedo() override;
......
...@@ -1540,7 +1540,13 @@ void RenderWidgetHostViewMac::OnNSViewSyncGetCharacterIndexAtPoint( ...@@ -1540,7 +1540,13 @@ void RenderWidgetHostViewMac::OnNSViewSyncGetCharacterIndexAtPoint(
void RenderWidgetHostViewMac::OnNSViewSyncGetFirstRectForRange( void RenderWidgetHostViewMac::OnNSViewSyncGetFirstRectForRange(
const gfx::Range& requested_range, const gfx::Range& requested_range,
gfx::Rect* rect, gfx::Rect* rect,
gfx::Range* actual_range) { gfx::Range* actual_range,
bool* success) {
if (!GetFocusedWidget()) {
*success = false;
return;
}
*success = true;
if (!GetCachedFirstRectForCharacterRange(requested_range, rect, if (!GetCachedFirstRectForCharacterRange(requested_range, rect,
actual_range)) { actual_range)) {
*rect = TextInputClientMac::GetInstance()->GetFirstRectForRange( *rect = TextInputClientMac::GetInstance()->GetFirstRectForRange(
......
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