September 2022 - webkit-dev - lists.webkit.org

InjectedBundle deprecation
by Alex Christensen 01 Aug '25

01 Aug '25

Hello WebKit developers! We are ramping up to do some serious work on Site Isolation which includes putting cross-origin iframes in a different process than the parent frame. Similar efforts have been done in other browser engines and some related work has already been done in WebKit, but not enough. This will do strange things to the InjectedBundle APIs, which have fundamental assumptions that the whole DOM is in one process and that communication only happens with one process at a time. We have never exposed InjectedBundle APIs as public API, but some other distributors of WebKit have. As we make progress on this project, we anticipate that anything in Source/WebKit/WebProcess/InjectedBundle/API is subject to deprecation and removal. This will also allow us to tighten the sandbox on the web process, resolve security and performance issues, and have cleaner code. Would the maintainers of the GTK and WPE APIs be willing to assist in migrating from those APIs and designing replacements in the UI process? Alex Christensen

5 17

WebKit Documentation
by Brandon Stewart 16 Mar '23

16 Mar '23

Hi WebKit Developers, Documentation is an important part of any open source project, especially for a larger project like WebKit. Being able to ramp up during the onboarding process, reading up on architectural decisions, and learning how to perform common procedures are all features the documentation should tackle. WebKit has a large set of documentation already, but it is scattered around a wide range of platforms (Trac, GitHub Wiki, markdown files in the code, Git commits, etc...), and some of the information is out of date. A few months ago, I started working on a new documentation solution based on the DocC documentation framework. This provides an easy way to add and edit documentation through markdown files. I have already ported a large section of Trac, all of the GitHub Wiki, and all of the non third party markdown files in the code over to this platform. I have tested this on macOS and Linux and have found it works extremely well. (Windows should be able to use WSL2 at the moment, while a few remaining issues get sorted out). The only dependency for this project is a recent installation of Swift. You can already download the documentation and preview it locally, but we are looking to publish it online for easy viewing. We were looking to get your feedback if we would want to publish the documentation on GitHub Pages or webkit.org. The documentation source can be found at https://github.com/webkit/documentation. - Brandon

13 22

Smart Pointer Analysis Tool for WebKit
by Ryosuke Niwa 01 Mar '23

01 Mar '23

Hi all, I’ve been working with Geoff (ggaren) and Jan Korous from Apple's compiler team to create a static analyzer which detects dangerous use of ref counted objects, and we’re looking for folks who are interested in trying out this tool and helping us refine the tool. Please let me know if you’re interested in using this tool & try applying code changes to WebKit. Our goal is to eventually integrate this tool into buildbot and EWS so that we can catch dangerous code before they get committed. *What is Dangerous?* So *what is* a *dangerous* use of ref counted objects you may ask? It’s any use of which we can’t trivially conclude that it doesn’t lead to a use-after-free. For now, we don’t detect dangerous use of non-ref counted objects including ones that can vend WeakPtr. It’s on us, humans, to decide which objects need to be ref counted or need to be CanMakeWeakPtr. Consider the following example. This code most certainly will lead to a use-after-free of “parent” in the third line because the code doesn’t keep the parent alive. Because isContentEditable updates style in some cases, it could lead to arbitrary script execution which could remove the parent from the document. Node* parent = element->parentElement(); if (parent->isContentEditable()) parent->scrollIntoView(); In general, relying on a complex data structure such as DOM tree to keep RefCounted objects alive while we call a non-trivial function is not safe. All it takes for the code to have a use-after-free is for someone to start updating style, layout, etc… inside the function either directly or indirectly. And we don’t want to make WebKit development really hard by forcing anyone who modifies a function to check every caller of the function and their callers, etc… to make sure it’s safe to do so. For this reason, it’s *dangerous* to store a raw pointer or a reference to a ref counted object as a local variable and use it after calling a non-trivial function. We did a similar analysis of a number of other patterns and usage of ref counted objects in WebKit and came up with the following basic rules for using ref counted objects in a safe manner. We’re hoping that these rules would be eventually incorporated into our coding style guideline: https://webkit.org/code-style-guidelines/ *Rules for Using Ref Counted Objects* 1. Every data member to a ref counted object must use either Ref, RefPtr, or WeakPtr. webkit.NoUncountedMemberChecker <https://github.com/llvm/llvm-project/blob/master/clang/docs/analyzer/checke…> 2. Every ref counted base class, if it has a derived class, must define a virtual destructor. webkit.RefCntblBaseVirtualDtor <https://github.com/llvm/llvm-project/blob/master/clang/docs/analyzer/checke…> 3. Every ref counted object passed to a non-trivial function as an argument (including "this" pointer) should be stored as a Ref or RefPtr in the caller’s local scope unless it's an argument to the caller itself by transitive property [1]. alpha.webkit.UncountedCallArgsChecker <https://github.com/llvm/llvm-project/blob/master/clang/docs/analyzer/checke…> 4. Every ref counted object must be captured using Ref or RefPtr for lambda. webkit.UncountedLambdaCapturesChecker <https://github.com/llvm/llvm-project/blob/master/clang/docs/analyzer/checke…> 5. Local variables - we’re still working on this ( https://reviews.llvm.org/D83259) Below, I've dissected each one of these rules with the real warning emitted by the analysis tool in development. Please let me know if you have any comments / concerns. ------------------------------ (1) is pretty trivial. Every ref counted data member should be stored using Ref, RefPtr, or WeakPtr since it would be not trivially obvious that life cycles of two or more objects are correctly tied or managed together. ------------------------------ (2) is also pretty easy to understand. In the following example, if someone destroys an instance of B using Ref<A>, then it would result in an undefined behavior. struct A : public RefCounted<A> { Vector<int> someData; }; struct B : public A { Vector<int> otherData; }; ------------------------------ For (3), passing a ref counted object as a raw pointer or reference to a function as an argument, the tool may generate a warning like this: Source/WebCore/html/FormAssociatedElement.cpp:181:13: warning: [WebKit] call argument is a raw pointers to a ref-countable type [-Wfunction-arg-ptr-to-refcntbl] setForm(findAssociatedForm(&asHTMLElement(), originalForm.get())); ^ This warns that void setForm(HTMLFormElement*) is called with the result of findAssociatedForm, which returns HTMLFormElement* without storing it anywhere. If setForm can somehow cause HTMLFormElement to be deleted, then this can result in the use-after-free in setForm. void FormAssociatedElement::resetFormOwner() { RefPtr<HTMLFormElement> originalForm = m_form.get(); setForm(findAssociatedForm(&asHTMLElement(), originalForm.get())); HTMLElement& element = asHTMLElement(); auto* newForm = m_form.get(); if (newForm && newForm != originalForm && newForm->isConnected()) element.document().didAssociateFormControl(element); } Why, you may ask, we don't put HTMLFormElement* in a Ref or RefPtr in setForm instead? This is because a lot of code has calls to a function with a local variable. Because the callee doesn’t know whether the caller has already stored each argument in Ref / RefPtr or not, we need to be safe and store them again in Ref / RefPtr, resulting in an unnecessary ref-churn. Additionally, if we took the approach of the callee being responsible for keeping every argument alive, then every non-trivial member function of a ref counted object must have "protectedThis" at the beginning of the function, which would be particularly wasteful if those member functions are sometimes called by other member functions. Once this rule is applied everywhere, for example, we can get rid of “protectedThis” from our codebase because it would be redundant. Furthermore, this rule transitively allows a raw pointer or a reference passed in as an argument to be used as arguments to call another function without storing it in Ref or RefPtr. For example, the following function satisfies this rule because both oldDocument and newDocument are raw references passed to this function as arguments, and therefore the caller of this function must have already stored as Ref / RefPtr as local variables. void HTMLMediaElement::didMoveToNewDocument(Document& oldDocument, Document& newDocument) { ASSERT_WITH_SECURITY_IMPLICATION(&document() == &newDocument); if (m_shouldDelayLoadEvent) { oldDocument.decrementLoadEventDelayCount(); newDocument.incrementLoadEventDelayCount(); } unregisterWithDocument(oldDocument); registerWithDocument(newDocument); HTMLElement::didMoveToNewDocument(oldDocument, newDocument); updateShouldAutoplay(); } ------------------------------ For (4), capturing a ref counted object as a raw pointer or a reference, the tool may generate a warning like this: Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp:115:74: warning: [WebKit] captured a raw pointer to a ref-countable type [-Wlambda-capture-ptr-to-refcntbl] cookieManager->deleteCookie(m_owningDataStore->sessionID(), cookie, [pool = WTFMove(pool), completionHandler = WTFMove(completionHandler)]() mutable { ^ This is warning about webPage object getting captured using a raw reference in the following function: void HTTPCookieStore::deleteCookie(const WebCore::Cookie& cookie, CompletionHandler<void()>&& completionHandler) { auto* pool = m_owningDataStore->processPoolForCookieStorageOperations(); if (!pool) { if (m_owningDataStore->sessionID() == PAL::SessionID::defaultSessionID() && !cookie.session) deleteCookieFromDefaultUIProcessCookieStore(cookie); else m_owningDataStore->removePendingCookie(cookie); RunLoop::main().dispatch([completionHandler = WTFMove(completionHandler)] () mutable { completionHandler(); }); return; } auto* cookieManager = pool->supplement<WebKit::WebCookieManagerProxy>(); cookieManager->deleteCookie(m_owningDataStore->sessionID(), cookie, [pool = WTFMove(pool), completionHandler = WTFMove(completionHandler)]() mutable { completionHandler(); }); } Here, we should have stored WebProcessPool in Ref instead. - R. Niwa

9 17

Re: [webkit-dev] webkit-dev Digest, Vol 208, Issue 5
by Jonathan Bedard 30 Sep '22

30 Sep '22

>> On Sep 20, 2022, at 1:52 PM, Brent Fulgham <bfulgham(a)apple.com> wrote: >> >>> On Sep 20, 2022, at 1:16 AM, Ryosuke Niwa via webkit-dev <webkit-dev(a)lists.webkit.org> wrote: >>> >>>> On Sep 19, 2022, at 2:28 PM, Brandon Stewart via webkit-dev <webkit-dev(a)lists.webkit.org <mailto:webkit-dev@lists.webkit.org>> wrote: >>>> >>>> Documentation is an important part of any open source project, especially for a larger project like WebKit. Being able to ramp up during the onboarding process, reading up on architectural decisions, and learning how to perform common procedures are all features the documentation should tackle. WebKit has a large set of documentation already, but it is scattered around a wide range of platforms (Trac, GitHub Wiki, markdown files in the code, Git commits, etc...), and some of the information is out of date. >>> >>> This ultimately feels like this situation: >>> https://xkcd.com/927/ <https://xkcd.com/927/> >>> >>> I?ve been working on https://github.com/WebKit/WebKit/blob/main/Introduction.md <https://github.com/WebKit/WebKit/blob/main/Introduction.md> for the past couple of years, and I?d would have preferred to have a collaboration rather than a competition here. Right now we have: https://github.com/WebKit/WebKit/wiki https://trac.webkit.org/ https://webkit.org/getting-started/ https://github.com/WebKit/WebKit/blob/main/Introduction.md Brandon’s solution is designed to replace the first 2, and he has buy in from the maintainers of the first two, when his solution is deployed, our existing wikis will die. I think https://webkit.org/getting-started/ and https://github.com/WebKit/WebKit/blob/main/Introduction.md are both designed to be too narrow to be a dumping ground for all our documentation. >> >> Ryosuke: Your document is outstanding, and is a large part of the content we pulled into the current repo. I don?t think we view this as a competition; rather we are trying to host a collection of Markdown content in a common repo that does not have to be pulled and synced with WebKit source and tests. This provides a lower bar for people interested in contributing documentation as they do not have to download and sync Gigabytes of WebKit code to write documentation. > > Who are these people who want to contribute to WebKit?s documentation yet doesn?t already have a clone of WebKit repository? I just can?t think of people who would fit that description. Mixing together documentation commits with commits which change product code is not great, it makes tracking down regressions more difficult for the same reasons we like monotonically increasing commit numbers. I think this is a trade off well worth it for Introduction.md, but not for something like https://github.com/WebKit/WebKit/wiki/Source-Control#identifiers. It’s also worth calling out that Jon Davis and I have had a similar discussion about https://github.com/WebKit/WebKit/tree/main/Websites/webkit.org, that’s probably something we shouldn’t be committing to the same repository with all our product code. > > (2) is particularly important because many people who are new to WebKit often don?t know what they don?t know. This is why, for example, memory management section appears towards the beginning of the document and the information about adding IDL files is immediately followed by the discussion of JS wrapper lifecycles. With these information now split across multiple files, it?s easy for people to miss out on critical information. In the ideal world, people won?t be adding or editing IDL files before they understood how JS wrappers are managed because not knowing the latter is a sure way of introducing subtle GC bugs. > >>>> A few months ago, I started working on a new documentation solution based on the DocC documentation framework. >>> >>> Was there any discussion as to which documentation framework should be used? I?m personally not familiar with DooC documentation, and I?m surprised that such an important decision was made unilaterally without much discussion on webkit-dev. >> >> We discussed this internally, but today?s message is the first discussion on this repository that we?ve opened on the webkit-dev mailing list. We wanted to convince ourselves that the tool worked well, and was easy to use, and could produce documentation output that would be useful for Apple engineers. That is not the only intended audience for this work, but is the origin of this effort which we wanted to make available to others to use and contribute to. >> >> Those of us who have worked on WebKit for some time can easily remember the many discussions over the years about documentation efforts of various types. Lots of people have strong opinions, but few ever contribute despite their opinions of the right way for the work to be done. You are obviously part of the group that has contributed heavily, so I am sad that you do not seem to like our approach. > > Right, it?s why I wrote Introduction.md in the first place. We have many things which need to be documented that should not be in Introduction.md, though. An example I know I’ve dealt with recently is the management of EWS bots. This isn’t something that belong at the top-level of the WebKit repository, but it’s clearly important. Our historic answer to this has been Trac’s Wiki, but that solution isn’t compatible with GitHub, we clearly need something new. > >>>> I have tested this on macOS and Linux and have found it works extremely well. (Windows should be able to use WSL2 at the moment, while a few remaining issues get sorted out). The only dependency for this project is a recent installation of Swift. >>> >>> Previously, we?ve rejected Swift as a general purpose programming languages in WebKit: https://lists.webkit.org/pipermail/webkit-dev/2014-July/026722.html <https://lists.webkit.org/pipermail/webkit-dev/2014-July/026722.html> other than to allow existing C++ code to call into Swift API: https://lists.webkit.org/pipermail/webkit-dev/2021-June/031882.html <https://lists.webkit.org/pipermail/webkit-dev/2021-June/031882.html> >>> >>> As such, I don?t think we should require having a functional Swift toolchain as a requirement for contributing to WebKit or its documentations either. >> >> DocC is not a requirement to use or participate in this work. It?s simply a ?port? of the documentation that works for our needs. If others prefer to ?build? output documentation from Markdown using a different tool set, they are welcome to contribute those build commands and instructions. > Something else worth calling out here is that contributing DocC compatible markdown is similar to contributions to https://github.com/WebKit/WebKit/tree/main/Websites/webkit.org that eventually end up on webkit.org <http://webkit.org/>, except that we have a nice path to automating DocC deployment (either to GitHub pages or a webkit.org <http://webkit.org/> property). We pretty frequently use technologies in our services we don’t intend contributors to regularly run at desk. > >> Our goal is to accumulate all relevant and open source documentation related to WebKit in this repository so that we can generate searchable documentation. We would also like this to be accessible and searchable from the web. > > I suggest that we don?t migrate contents of https://trac.webkit.org/wiki <https://trac.webkit.org/wiki> to whatever new documentation we create. Most information on that wiki is extremely outdated or outright wrong. That very much depends on the section of Trac’s Wiki you’re looking at. https://trac.webkit.org/wiki/TestExpectations is ancient, but pretty accurate, for example. Same for https://trac.webkit.org/wiki/LayoutTestsSearchPath (despite the references to ancient MacOS versions). We need to do some serious clean up (which probably involves a lot of deletion), but blind deletion seems unwise.

3 4

Deployment of new EWS Non-Unified builder
by dpino 08 Sep '22

08 Sep '22

Hi, Last year we started a thread to discuss the possibility of deploying a new EWS bot that builds WebKit with Non-Unified sources [1]. This thread explains the technical reasons why a non-unified build bot is desirable. If you're aware of the problems introduced by unified sources compilation, skip the next paragraph. Unified sources compilation consists of merging several source files together into one big file to improve building times. Usually the same sources files are grouped together, but compilation flags or downstream changes can create different source file groups. When that happens, you may hit a build error. Since unified sources group files together, missing headers in one file can be satisfied by another file in the same group. What's happening in WebKit development currently is that source code that breaks non-unified compilation frequently lands without even being noticed. The breaks are usually related to missing headers. Embedders that need to support WebKit builds with different flags or maintain downstream changes are more likely to hit these compilation errors. Last year's attempt to deploy an EWS Non-Unified builder ended up in a deployment of a post-commit bot plus two workers running in the UAT. It was actually hard to get the Non-Unified builder working [2], something that we achieved at the beginning of this year (kudos to Adrián and Lauro). Since then we have been maintaining the post-commit bot very closely. There are periods of time where the bot has remained green for a long period of time. But lately maintaining the bot green has become harder and harder, specially due to the big refactorizations that have happened in WebKit source code lately. The bot very rarely stays green longer than 2 or 3 days without close maintenance. We believe that we all should share the responsibility of keeping the non-unified build working, and therefore we want to proceed with the deployment of a EWS Non-Unified builder. Initially, we'll provide two workers on a modern host with 8 cores assigned each. We have found this setup faster for most patches than many of the existing EWS queues, as it only runs a build in non-unified mode, without test or upload steps. If this were not fast enough, we would consider increasing the number of workers in the queue. We set up a page[3] with a rough comparison to the regular (unified) bot and the build times are pretty similar. Diego [1] https://www.mail-archive.com/webkit-dev@lists.webkit.org/msg30077.html [2] https://bugs.webkit.org/show_bug.cgi?id=226088 [3] https://people.igalia.com/lmoura/webkit-bots-dashboard/unified.html

14 34

Goal of Fixing non-Unified Build Failures (Was Deployment of new EWS Non-Unified builder)
by Ryosuke Niwa 08 Sep '22

08 Sep '22

> There is no such thing as "not maintaining the non-unified build"; there never has been. We have covered that this is an *inherent* problem in a unified build mechanism and that this would be an issue even if Mac were the only platform. It appears to me that we’re lacking the clarify in defining what problem(s) we’re trying to solve here. As far as I can tell, fixing non-unified builds is a potential solution to two problems: Adding a new cpp file to Sources.txt causes random other file to trigger a compiler error because the set of cpp files unified together have changed / shuffled across. Build failure may occur in an exotic port or configuration which unify a different set of cpp files than the supported configurations. (1) might be a problem worth solving but there are multiple solutions to this problem. For example, we could stop lexicologically sorting cpp files in Sources.txt, and always append new cpp files at the end of each directory instead. This will avoid shuffling cpp files that are unified together when adding new cpp files. Someone who feels like it could come in & sort files occasionally, or maybe we should just forget about this entirely and let the chaos ensue. Either solution seems less costly than forcing every contributor to maintain non-unified builds functional. While I’m sympathetic to the view that (2) is a problem or that it would be nice for the WebKit project to address the concerns of such exotic configurations or ports, I just don’t see how this differs from all other build failures that occur from WebKit code changes such as refactoring of platform abstraction. I grant that build failures which are effect of cpp files getting unified differently across different ports and configurations might be tricky to diagnose. Even if we were to agree that (2) is a problem worth addressing, again, there are multiple solutions to this problem. We could, for example, stabilize cpp files that are unified across configurations / ports so that if a set of cpp files get unified under one configuration, they will always get unified in all other configurations. In practice, this might mean moving every platform specific file into a sub directory and unifying them separately for example. - R. Niwa

1 0