Adaption of WebKit to the changes to Services Kit#35
Adaption of WebKit to the changes to Services Kit#35alaviss wants to merge 4 commits intohaiku:rebasedfrom
Conversation
Main changes in this commit:
- BUrlProtocolHandler is splitted into two classes:
- BUrlProtocolHandler for managing the request lifetime.
- BUrlRequestWrapper for handling events from requests spawned by
BUrlProtocolHandler.
The separation allow the events handling code to be greatly
simplified, and code for handling events and managing request
are now properly separated.
In the future this enables BUrlProtocolHandler to be the
synchronization/serialization point, allowing BUrlRequestWrapper to
interface with BUrlRequest directly instead of going through
BUrlProtocolAsynchronousRequest, which should allow for better
performance.
- Redirection and authentication are now handled manually by the backend
instead of delegating to BUrlRequest.
- Code style has been adjusted to match WebKit official style guideline.
This is a basic adaption of the current WebKit to make use of BUrlSession. It's done enough for HaikuLauncher to compile, however I've not managed to throughly test it due as my VM died.
|
Does it work (i.e. generally not crash)? We can work out the locking better later. |
|
I haven't thoroughly tested the |
pulkomandy
left a comment
pulkomandy
left a comment
There was a problem hiding this comment.
Thanks for cleaning the code :)
I have some questions, mostly to make sure I understand the design and the way things are supposed to work. Please see the inline comments.
|
|
||
| BHttpRequest* httpRequest = dynamic_cast<BHttpRequest*>(m_request); | ||
| if(httpRequest) { | ||
| // TODO maybe we have data to send in other cases ? |
There was a problem hiding this comment.
I think this TODO is still valid. It is possible (at least at the HTTP level) to do a GET request with a body. So maybe this if() needs to be removed.
| // the main dispatcher. | ||
| ref(); | ||
|
|
||
| // Block the receiving thread until headers are parsed. |
There was a problem hiding this comment.
why is that needed? The asycnhronous mode should allow lock-less operation?
Doesn't this risk stalling the whole browser for example if trying to connect to a very slow server or if the server decides to send a few gigabytes of headers?
There was a problem hiding this comment.
I don't remember why I added the lock... Upon a quick inspection it might not be needed, as the lambda produced by Write() actually check for m_handler validity before acting on the data.
I might have added it because earlier revisions of the patch have BUrlRequestWrapper::Write() accessing m_handler.
|
|
||
| ssize_t BUrlRequestWrapper::Write(const void* data, size_t size) | ||
| { | ||
| auto locker = holdLock(m_receiveMutex); |
There was a problem hiding this comment.
why does this need a lock? It only accesses the data passed as parameters, which shouldn't go away as long as the function doesn't return?
| bool m_didReceiveData { false }; | ||
| bool m_didUnblockReceive { false }; | ||
|
|
||
| // This lock is in charge of two things: |
There was a problem hiding this comment.
Please document it a bit more. Which threads will be locking it? When?
https://bugs.webkit.org/show_bug.cgi?id=216224 Reviewed by Ross Kirsling. JSTests: * stress/intl-collator-co-extension.js: (explicitTrueBeforeICU67): Deleted. * stress/intl-collator.js: (shouldBe.testCollator.Intl.Collator): (explicitTrueBeforeICU67): Deleted. * stress/intl-datetimeformat.js: * stress/intl-locale.js: * stress/intl-numberformat.js: * stress/intl-object.js: * stress/intl-pluralrules.js: * stress/intl-relativetimeformat.js: * test262/expectations.yaml: Source/JavaScriptCore: Unicode Technical Standard #35 defines that unicode extension type's "true" should be converged to "". This patch implements it by extracting unicode extension subtags and replacing "true" to "". * runtime/IntlLocale.cpp: (JSC::LocaleIDBuilder::toCanonical): (JSC::IntlLocale::keywordValue const): (JSC::IntlLocale::calendar): (JSC::IntlLocale::caseFirst): (JSC::IntlLocale::collation): (JSC::IntlLocale::hourCycle): (JSC::IntlLocale::numberingSystem): (JSC::IntlLocale::numeric): * runtime/IntlLocale.h: * runtime/IntlLocalePrototype.cpp: (JSC::IntlLocalePrototypeGetterCalendar): (JSC::IntlLocalePrototypeGetterCaseFirst): (JSC::IntlLocalePrototypeGetterCollation): (JSC::IntlLocalePrototypeGetterHourCycle): (JSC::IntlLocalePrototypeGetterNumberingSystem): * runtime/IntlObject.cpp: (JSC::unicodeExtensionSubTags): (JSC::canonicalizeUnicodeExtensionsAfterICULocaleCanonicalization): (JSC::languageTagForLocaleID): (JSC::resolveLocale): * runtime/IntlObject.h: * runtime/IntlObjectInlines.h: (JSC::computeTwoCharacters16Code): * runtime/StringPrototype.cpp: (JSC::computeTwoCharacters16Code): Deleted. Source/WTF: * wtf/text/StringView.h: (WTF::StringView::characterAt const): (WTF::StringView::operator[] const): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@266973 268f45cc-cd09-0410-ab3c-d52691b4dbfc
3 participants
This PR contains the work done to adapt haikuwebkit for use with my changes to Services Kit.
This PR contains the following major changes: