Bug 189190 – REGRESSION(r235398) ASSERTION failure !m_client.didFinishDocumentLoadForFrame

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Frédéric Wang (:fredw)]

There are assertions in the deprecated WKPageSetPageLoaderClient class after bug 188997 ; it seems they are currently hit when running build with the external SDK.

[Frédéric Wang (:fredw)]

Created attachment 348636 [details]
Patch

[Alex Christensen]

I assume this is when you run system safari with your build of WebKit.  Does Safari work well enough with this WebKit?

[Alex Christensen]

This would be more correct if it were a version check to see if you're on Mojave or before.

[Alex Christensen]

Comment on attachment 348636 [details]
Patch

I mean, we would want to not assert if you're on Mojave or before just to work with system safari.

[Frédéric Wang (:fredw)]

(In reply to Alex Christensen from comment #2)
> I assume this is when you run system safari with your build of WebKit.  Does
> Safari work well enough with this WebKit?

I'm just doing run-safari [--ios-simulator] using the public SDK, so AFAIK yes it is the system one.

(In reply to Alex Christensen from comment #3)
> This would be more correct if it were a version check to see if you're on
> Mojave or before.

Right, I'll try to see how to do that.

[Alex Christensen]

Also, if you could get it to use Safari Tech Preview instead of Safari, that would probably fix it.

[Alexey Proskuryakov]

Is "Mojave or before" the right check, or is it actually about Safari 11.x?

[Frédéric Wang (:fredw)]

(In reply to Alex Christensen from comment #6)
> Also, if you could get it to use Safari Tech Preview instead of Safari, that
> would probably fix it.

I think I've never been able to make run-webkit-app work ; I just tried again and Safari Tech Preview does not seem to use the local build. Also, in any case that's not an option for running Safari on the iOS simulator.

(In reply to Alexey Proskuryakov from comment #7)
> Is "Mojave or before" the right check, or is it actually about Safari 11.x?

Good question. Comment 6 seems to suggest it's about the Safari version, not the OS so probably we should try that.

[Frédéric Wang (:fredw)]

*** Bug 189233 has been marked as a duplicate of this bug. ***

[Frédéric Wang (:fredw)]

Created attachment 348762 [details]
Patch

[Frédéric Wang (:fredw)]

I've updated the patch to use a OS version check (I'm not sure how to check Safari's version). I hope it's good enough.

[Alex Christensen]

Created attachment 348822 [details]
patch

[Alex Christensen]

I made the check for Mojave or before rather than add a bundle check for Safari.  That should be good enough for the open source developers checking their work with Safari until the next release.

[WebKit Commit Bot]

Comment on attachment 348822 [details]
patch

Clearing flags on attachment: 348822

Committed r235618: <https://trac.webkit.org/changeset/235618>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/44102173>

[Frédéric Wang (:fredw)]

(In reply to WebKit Commit Bot from comment #14)
> Comment on attachment 348822 [details]
> patch
> 
> Clearing flags on attachment: 348822
> 
> Committed r235618: <https://trac.webkit.org/changeset/235618>

@Alex: Any reason why you removed the condition "PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 120000"? It seems the landed commit will still fail to make "run-safari --ios-simulator" work for open source developers...

[Alex Christensen]

iOS Safari never used WKPageSetPageLoaderClient :)

[Frédéric Wang (:fredw)]

OK, maybe I mixed up the other day because bug 189188 happened at the same time.