[webkit-reviews] review granted: [Bug 183787] ScrollViewInsetTests.RestoreInitialContentOffsetAfterCrash API test is failing with async delegates : [Attachment 336201] Patch

[bugzilla-daemon at webkit.org]

Wenson Hsieh <wenson_hsieh at apple.com> has granted  review:
Bug 183787: ScrollViewInsetTests.RestoreInitialContentOffsetAfterCrash API test
is failing with async delegates
https://bugs.webkit.org/show_bug.cgi?id=183787

Attachment 336201: Patch

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




--- Comment #11 from Wenson Hsieh <wenson_hsieh at apple.com> ---
Comment on attachment 336201
  --> https://bugs.webkit.org/attachment.cgi?id=336201
Patch

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

> Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:904
> +    if (!m_frame->isMainFrame())

This seems to be the case, but just wanted to make sure — it's not possible for
m_frame->isMainFrame() to be true in
WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction but false in
WebFrameLoaderClient::didDecidePolicyForNavigationAction, correct?

> Tools/TestWebKitAPI/Tests/ios/ScrollViewInsetTests.mm:36
> +static bool navigationComplete = false;

Nit - I think we now generally avoid using static state variables in API tests
if possible (though many older tests still do this). Could we fold this into
AsyncPolicyDelegateForInsetTest?