[webkit-reviews] review granted: [Bug 208365] UIProcess crash after using _prepareForMoveToWindow, then deallocating the WKWebView before moving to the window : [Attachment 392675] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Sihui Liu <sihui_liu at apple.com>'s
request for review:
Bug 208365: UIProcess crash after using _prepareForMoveToWindow, then
deallocating the WKWebView before moving to the window
https://bugs.webkit.org/show_bug.cgi?id=208365

Attachment 392675: Patch

https://bugs.webkit.org/attachment.cgi?id=392675&action=review




--- Comment #19 from Geoffrey Garen <ggaren at apple.com> ---
Comment on attachment 392675
  --> https://bugs.webkit.org/attachment.cgi?id=392675
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=392675&action=review

Looks right to me, and tests pass. Yay! I think this could use a round of style
fixup before landing.

> Source/WebKit/ChangeLog:10
> +	   not actually moves. Make WebView hold strong reference to the target
window so it knows to stop observing at

does not actually move.

> Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:2043
> +	   return m_targetWindowForMovePreparation.get().backingScaleFactor;

m_targetWindowForMovePreparation->backingScaleFactor

> Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:2337
> +	       m_colorSpace =
m_targetWindowForMovePreparation.get().colorSpace;

m_targetWindowForMovePreparation->colorSpace

> Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:2430
>      viewDidMoveToWindow();

Should this be viewDidMoveToWindowImpl? Or maybe you discovered that we only
need that treatment for viewWillMoveToWindow?