Bug 195435 – Make sure an empty host matches the internal representation "nullOrigin" in WebCore::RegistrableDomain::matches()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[John Wilander]

WebCore::RegistrableDomain::matches() is a quick way to compare a RegistrableDomain with a URL. Since RegistrableDomain represents the empty host as "nullOrigin," a URL with an empty host needs to match that representation in WebCore::RegistrableDomain::matches(). This omission caused debug assertions in fast/ layout tests after https://trac.webkit.org/changeset/242603/webkit.

[John Wilander]

Created attachment 363929 [details]
Patch

[Brent Fulgham]

Comment on attachment 363929 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 363929 [details]
Patch

Clearing flags on attachment: 363929

Committed r242620: <https://trac.webkit.org/changeset/242620>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/48695077>

[Darin Adler]

Comment on attachment 363929 [details]
Patch

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

> Source/WebCore/platform/RegistrableDomain.h:62
> +        if (host.isEmpty() && m_registrableDomain == "nullOrigin"_s)

Using _s with == doesn’t optimize things; I think in fact it just makes them slower. This _s should be omitted.

[Darin Adler]

Comment on attachment 363929 [details]
Patch

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

>> Source/WebCore/platform/RegistrableDomain.h:62
>> +        if (host.isEmpty() && m_registrableDomain == "nullOrigin"_s)
> 
> Using _s with == doesn’t optimize things; I think in fact it just makes them slower. This _s should be omitted.

Or we could fix WTF::String so it at least doesn’t make things slower and then leave this alone.

[John Wilander]

(In reply to Darin Adler from comment #6)
> Comment on attachment 363929 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=363929&action=review
> 
> > Source/WebCore/platform/RegistrableDomain.h:62
> > +        if (host.isEmpty() && m_registrableDomain == "nullOrigin"_s)
> 
> Using _s with == doesn’t optimize things; I think in fact it just makes them
> slower. This _s should be omitted.

Thanks! Did not know that.

[Simon Fraser (smfr)]

Still lots of asserts in iOS sim:
https://build.webkit.org/builders/Apple%20iOS%2012%20Simulator%20Debug%20WK2%20%28Tests%29?numbuilds=100

[John Wilander]

(In reply to Simon Fraser (smfr) from comment #9)
> Still lots of asserts in iOS sim:
> https://build.webkit.org/builders/
> Apple%20iOS%2012%20Simulator%20Debug%20WK2%20%28Tests%29?numbuilds=100

Sorry about that. I will have a look tomorrow morning. It’s probably best to restructure the check before the IPC call. The reason for all of this is that I don’t want to do IPC for useless cases, only to drop them on the other side.

[John Wilander]

Looking at this now.

[John Wilander]

These assertion failures seem unrelated. The problem seems to be in WebKit::RemoteLayerBackingStoreCollection::backingStoreWillBeDisplayed(). Or did you see my code causing this, Simon?

ASSERTION FAILED: m_inLayerFlush
/Volumes/Data/slave/ios-simulator-12-debug/build/Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStoreCollection.mm(92) : bool WebKit::RemoteLayerBackingStoreCollection::backingStoreWillBeDisplayed(WebKit::RemoteLayerBackingStore &)

[John Wilander]

I was not able to repro with ToT on my machine, executing:
run-webkit-tests --ios-simulator animations/animation-controller-drt-api.html --no-retry-failures