Skip to content

Commit

Permalink
Workaround crash in WebContentsNSViewBridge::SetParentNSView
Browse files Browse the repository at this point in the history
It has been observed that we are hitting the CHECK(parent_ns_view) in
WebContentsNSViewBridge::SetParentNSView.

Partially revert r625255, the culprit patch, by moving the functionality
of WebContentsNSViewBridge::SetParentNSView in to the caller,
NativeViewHostMac::AttachNativeView.

This is not a reasonable long-term solution, but is the most reasonable
merge candidate. Follow-on patches will move the CHECK up into
NativeViewHostMac::AttachNativeView to see exactly why this is
happening.

Bug: 933679
Change-Id: I08b19f3123bbe96c49376a41f28a879a855bee62
Reviewed-on: https://chromium-review.googlesource.com/c/1487888
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635705}
  • Loading branch information
ccameron-chromium authored and Commit Bot committed Feb 26, 2019
1 parent 118cd02 commit f129e62
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 8 deletions.
7 changes: 6 additions & 1 deletion content/browser/web_contents/web_contents_view_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,12 @@
} else if (factory_host_id != NSViewBridgeFactoryHost::kLocalDirectHostId) {
LOG(ERROR) << "Failed to look up NSViewBridgeFactoryHost!";
}
ns_view_bridge_local_->SetParentNSView(views_host_->GetNSViewId(), token);

// TODO(https://crbug.com/933679): WebContentsNSViewBridge::SetParentView
// will look up the parent NSView by its id, but this has been observed to
// fail in the field, so assume that the caller handles updating the NSView
// hierarchy.
// ns_view_bridge_local_->SetParentNSView(views_host_->GetNSViewId(), token);

for (auto* rwhv_mac : GetChildViews()) {
rwhv_mac->MigrateNSViewBridge(factory_host, ns_view_id_);
Expand Down
16 changes: 9 additions & 7 deletions ui/views/controls/native/native_view_host_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -122,15 +122,17 @@ void EnsureNativeViewHasNoChildWidgets(NSView* native_view) {
EnsureNativeViewHasNoChildWidgets(native_view_);

auto* bridge_host = GetBridgedNativeWidgetHost();
DCHECK(bridge_host);
CHECK(bridge_host);

if (native_view_hostable_) {
// TODO(https://crbug.com/933679): This is lifted out the ViewsHostableAttach
// call below because of crashes being observed in the field.
NSView* superview =
bridge_host->native_widget_mac()->GetNativeView().GetNativeNSView();
[superview addSubview:native_view_];

if (native_view_hostable_)
native_view_hostable_->ViewsHostableAttach(this);
} else {
NSView* superview =
bridge_host->native_widget_mac()->GetNativeView().GetNativeNSView();
[superview addSubview:native_view_];
}

bridge_host->SetAssociationForView(host_, native_view_);
}

Expand Down

0 comments on commit f129e62

Please sign in to comment.