Skip to content

Take photo#3

Open
jyavenard wants to merge 3 commits into
eric-carlson:image-capture-take-photofrom
jyavenard:takePhoto
Open

Take photo#3
jyavenard wants to merge 3 commits into
eric-carlson:image-capture-take-photofrom
jyavenard:takePhoto

Conversation

@jyavenard
Copy link
Copy Markdown

@jyavenard jyavenard commented Oct 25, 2023

minor changes

@eric-carlson eric-carlson force-pushed the image-capture-take-photo branch 7 times, most recently from 92f46a0 to 3019a74 Compare October 27, 2023 19:01
@eric-carlson
[MediaStream] Add support for ImageCapture.takePhoto
cb5a25d 
https://bugs.webkit.org/show_bug.cgi?id=262467
rdar://116322637

Reviewed by NOBODY (OOPS!).

Implement ImageCapture.takePhoto in AVVideoCaptureSource and MockRealtimeVideoSource.
We reconfigure the capture source when asked for a photo larger than the current preset,
so it always returns an image at least as large as the requested size, but it doesn't
yet resize the image if the requested size doesn't exactly match the preset.

Taking a photo is an asynchronous operation that can take an arbitrary amount of time, so
we don't do it on the main thread. Because we may need to reconfigure the capture device
to take a photo with the requested settings, we need to prevent the capture device from
being reconfigured by `applyConstraints` while a photo is being generated. To do this we
serialize calls to `takePhoto` and `applyConstraints` so they don't interfere with one
another.

* LayoutTests/fast/mediastream/image-capture-take-photo-expected.txt: Added.
* LayoutTests/fast/mediastream/image-capture-take-photo.html: Added.
* Source/WebCore/Modules/mediastream/ImageCapture.cpp:
(WebCore::ImageCapture::takePhoto):
* Source/WebCore/Modules/mediastream/ImageCapture.h:
* Source/WebCore/Modules/mediastream/ImageCapture.idl:
* Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:
(WebCore::MediaStreamTrack::stopTrack):
(WebCore::MediaStreamTrack::queueAndProcessSerialAction):
(WebCore::MediaStreamTrack::processNextSerialAction):
(WebCore::MediaStreamTrack::takePhoto):
(WebCore::MediaStreamTrack::applyConstraints):
* Source/WebCore/Modules/mediastream/MediaStreamTrack.h:
* Source/WebCore/PAL/pal/cocoa/AVFoundationSoftLink.h:
* Source/WebCore/PAL/pal/cocoa/AVFoundationSoftLink.mm:
* Source/WebCore/platform/mediastream/MediaStreamTrackPrivate.cpp:
(WebCore::MediaStreamTrackPrivate::takePhoto):
* Source/WebCore/platform/mediastream/MediaStreamTrackPrivate.h:
* Source/WebCore/platform/mediastream/PhotoSettings.h:
(WebCore::operator==):
* Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp:
(WebCore::RealtimeMediaSource::takePhoto):
* Source/WebCore/platform/mediastream/RealtimeMediaSource.h:
* Source/WebCore/platform/mediastream/RealtimeVideoCaptureSource.cpp:
(WebCore::RealtimeVideoCaptureSource::bestSupportedSizeFrameRateAndZoomConsideringObservers):
(WebCore::RealtimeVideoCaptureSource::setSizeFrameRateAndZoom):
(WebCore::RealtimeVideoCaptureSource::takePhotoInternal):
(WebCore::RealtimeVideoCaptureSource::takePhoto):
* Source/WebCore/platform/mediastream/RealtimeVideoCaptureSource.h:
* Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.h:
* Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:
(WebCore::toFillLightMode):
(WebCore::toAVCaptureFlashMode):
(WebCore::photoQueue):
(WebCore::AVVideoCaptureSource::~AVVideoCaptureSource):
(WebCore::AVVideoCaptureSource::stopProducingData):
(WebCore::AVVideoCaptureSource::stopSession):
(WebCore::AVVideoCaptureSource::photoOutput):
(WebCore::AVVideoCaptureSource::resolvePendingPhotoRequest):
(WebCore::AVVideoCaptureSource::rejectPendingPhotoRequest):
(WebCore::AVVideoCaptureSource::maxPhotoSizeForCurrentPreset const):
(WebCore::AVVideoCaptureSource::photoConfiguration):
(WebCore::AVVideoCaptureSource::takePhotoInternal):
(WebCore::AVVideoCaptureSource::getPhotoSettings):
(WebCore::AVVideoCaptureSource::captureOutputDidFinishProcessingPhoto):
(-[WebCoreAVVideoCaptureSourceObserver captureOutput:didFinishProcessingPhoto:error:]):
* Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm:
(WebCore::MockRealtimeVideoSourceMac::updateSampleBuffer):
* Source/WebCore/platform/mock/MockRealtimeVideoSource.cpp:
(WebCore::takePhotoRunLoop):
(WebCore::MockRealtimeVideoSource::DrawingState::fontDescription):
(WebCore::MockRealtimeVideoSource::DrawingState::timeFont):
(WebCore::MockRealtimeVideoSource::DrawingState::bipBopFont):
(WebCore::MockRealtimeVideoSource::DrawingState::statsFont):
(WebCore::MockRealtimeVideoSource::takePhotoInternal):
(WebCore::MockRealtimeVideoSource::invalidateDrawingState):
(WebCore::MockRealtimeVideoSource::drawingState):
(WebCore::MockRealtimeVideoSource::settingsDidChange):
(WebCore::MockRealtimeVideoSource::drawText):
(WebCore::MockRealtimeVideoSource::generatePhoto):
(WebCore::MockRealtimeVideoSource::generateFrameInternal):
(WebCore::MockRealtimeVideoSource::generateFrame):
(WebCore::MockRealtimeVideoSource::imageBuffer):
(WebCore::MockRealtimeVideoSource::imageBufferInternal):
(WebCore::MockRealtimeVideoSource::imageBuffer const): Deleted.
* Source/WebCore/platform/mock/MockRealtimeVideoSource.h:
(WebCore::MockRealtimeVideoSource::DrawingState::DrawingState):
(WebCore::MockRealtimeVideoSource::DrawingState::baseFontSize const):
(WebCore::MockRealtimeVideoSource::DrawingState::statsFontSize const):
* Source/WebKit/GPUProcess/webrtc/RemoteSampleBufferDisplayLayer.h:
* Source/WebKit/UIProcess/Cocoa/UserMediaCaptureManagerProxy.cpp:
(WebKit::UserMediaCaptureManagerProxy::SourceProxy::takePhoto):
(WebKit::UserMediaCaptureManagerProxy::takePhoto):
* Source/WebKit/UIProcess/Cocoa/UserMediaCaptureManagerProxy.h:
* Source/WebKit/UIProcess/Cocoa/UserMediaCaptureManagerProxy.messages.in:
* Source/WebKit/WebProcess/cocoa/RemoteRealtimeMediaSource.cpp:
(WebKit::RemoteRealtimeMediaSource::takePhoto):
* Source/WebKit/WebProcess/cocoa/RemoteRealtimeMediaSource.h:
* Source/WebKit/WebProcess/cocoa/RemoteRealtimeMediaSourceProxy.cpp:
(WebKit::RemoteRealtimeMediaSourceProxy::takePhoto):
* Source/WebKit/WebProcess/cocoa/RemoteRealtimeMediaSourceProxy.h:
@eric-carlson eric-carlson force-pushed the image-capture-take-photo branch from 3019a74 to cb5a25d Compare October 27, 2023 20:44
@eric-carlson eric-carlson force-pushed the image-capture-take-photo branch 11 times, most recently from 4961334 to cb20161 Compare November 12, 2023 01:36
@eric-carlson eric-carlson force-pushed the image-capture-take-photo branch from cb20161 to dc92712 Compare November 13, 2023 00:11
eric-carlson pushed a commit that referenced this pull request May 13, 2024
@robert-jenner
[Gardening] [site-isolation] Attempt #3 of enable more tests to run i…
79f1358 
…n site-isolation rdar://127515199 https://bugs.webkit.org/show_bug.cgi?id=273715

https://bugs.webkit.org/show_bug.cgi?id=273715
rdar://127515199

Unreviewed test gardening.

* LayoutTests/platform/mac-site-isolation/TestExpectations:

Canonical link: https://commits.webkit.org/278466@main
eric-carlson pushed a commit that referenced this pull request Jul 11, 2024
@graouts
[web-animations] animation with a non-invertible matrix should not in…
df87a56 
…terpolate

https://bugs.webkit.org/show_bug.cgi?id=275993
rdar://130704075

Reviewed by Matt Woodrow.

We had three separate issues that would lead us to visually animate when one of the values in a given interval
is a non-invertible matrix:

1. The method that determines whether it's possible to interpolate between two `transform` values would only
account for `matrix()` values and not `matrix3d()`.

2. The `transform` property animation wrapper would not implement the `canInterpolate()` method and would thus
always indicate that two `transform` values could be interpolated. This caused CSS Transitions to run even when
the values would not a discrete interpolation.

3. Even if we correctly determined that two `transform` values should yield discrete interpolation, we would
delegate an accelerated animation to Core Animation and that animation's behavior would differ an visibly
interpolate.

In this patch, we fill all three issues.

First, we introduce a new `TransformOperations::containsNonInvertibleMatrix()` method which will check whether
a `matrix()` or `matrix3d()` value that is not invertible is contained in the list of transform operations. We
now use this function in `TransformOperations::shouldFallBackToDiscreteAnimation()` to address issue #1.

Then, we add a `canInterpolate()` implementation to `AcceleratedTransformOperationsPropertyWrapper` which calls
in the now-correct `TransformOperations::shouldFallBackToDiscreteAnimation()` to address issue #2.

Finally, we add a new flag on `BlendingKeyframes` to determine whether a keyframe contains a `transform` value
with a non-invertible matrix and we consult that flag in `KeyframeEffect::canBeAccelerated()` to determine whether
an animation should be delegated to Core Animation, addressing issue #3.

We add new WPT tests to check the correct interpolation behavior of `transform` when a non-invertible `matrix3d()`
value is used, that no CSS Transition can be started with such a value, and finally that no animation is visibly
run to catch the Core Animation case.

* LayoutTests/imported/w3c/web-platform-tests/css/css-transforms/animation/transform-interpolation-007-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-transforms/animation/transform-interpolation-007.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-transforms/animation/transform-non-invertible-discrete-interpolation-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-transforms/animation/transform-non-invertible-discrete-interpolation-ref.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-transforms/animation/transform-non-invertible-discrete-interpolation.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-transforms/animation/transform-non-invertible-no-transition-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-transforms/animation/transform-non-invertible-no-transition.html: Added.
* Source/WebCore/animation/BlendingKeyframes.cpp:
(WebCore::BlendingKeyframes::analyzeKeyframe):
* Source/WebCore/animation/BlendingKeyframes.h:
(WebCore::BlendingKeyframes::hasDiscreteTransformInterval const):
* Source/WebCore/animation/CSSPropertyAnimation.cpp:
* Source/WebCore/animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::canBeAccelerated const):
* Source/WebCore/platform/graphics/transforms/TransformOperations.cpp:
(WebCore::TransformOperations::containsNonInvertibleMatrix const):
(WebCore::TransformOperations::shouldFallBackToDiscreteAnimation const):
* Source/WebCore/platform/graphics/transforms/TransformOperations.h:

Canonical link: https://commits.webkit.org/280466@main
eric-carlson pushed a commit that referenced this pull request Sep 27, 2024
@Constellation
[JSC] ObjectAllocationSinking should not omit phi insertion when poin…
ba44420 
…ter follows to the same value

https://bugs.webkit.org/show_bug.cgi?id=279570
rdar://135851156

Reviewed by Keith Miller.

Let's consider the following FTL graph.

    BB#0
    @0 = NewObject()
    Jump #1

    BB#1
    PutByOffset(@0, 0, @x)
    Jump #2

    BB#2
    ...
    @z = ...
    @1 = GetByOffset(@x, 0)
    Branch(@1, #3, WebKit#4)

    BB#3
    PutByOffset(@0, 0, @z)
    Jump WebKit#5

    BB#4
    PutByOffset(@0, 0, @z)
    Jump WebKit#5

    BB#5
    Jump #2

Now, we would like to eliminate @0 object allocation. And we are
computing SSA for pointers of fields of the that object which gets
eliminated. Consider about @x's fields' SSA. PutByOffset becomes Def
and GetByOffset becomes Use. And the same field will get the same SSA
variable. So we first puts Defs and compute Phis based on that.

In ObjectAllocationSinking phase, we had a fast path when the both SSA
variable is following to the same value. Let's see BB#5. Because BB#3
and BB#4 defines Defs, dominance frontier BB#5 will need to introduce
Phi. But interestingly, both SSA variable is following to the same @z.
As a result, we were not inserting Phi for this case.

But this is wrong. Inserted Phi is a Def, and based on that, we will
further introduce Phis with that. If we omit inserting Phi in BB#5,
we will not insert Phi into BB#2 while BB#2 will merge BB#1's Def And
BB#5's Phi's Def. As a result, in BB#2, we think this variable is
following to BB#1's Def. But that's wrong and BB#5's Phi exists.

This patch removes this fast path to fix the issue.

* JSTests/stress/object-allocation-sinking-phi-insertion-for-pointers.js: Added.
(Queue):
(Queue.prototype.enqueue):
(Queue.prototype.dequeue):
(i.queue.dequeue):
* Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp:

Canonical link: https://commits.webkit.org/283558@main
eric-carlson pushed a commit that referenced this pull request Feb 13, 2025
@the-chenergy @JonWBedard
A compromised Web Content process should not be able to start Web Ins…
301bff4 
…pector

rdar://98891055
https://bugs.webkit.org/show_bug.cgi?id=283092

Reviewed by Ryosuke Niwa and BJ Burg.

There currently exists a message
WebInspectorUIProxy::OpenLocalInspectorFrontend, which the web process
sends to the UI process to show Web Inspector for the current web page.
This introduces security risks as a compromised website may find its way
to send arbitrary messages to the UI process, opening Web Inspector and
weakening the web content sandbox.

The reason this message exists is because there are useful ways the web
process needs to open Web Inspector with initiative. Normally, Web
Inspector is opened via one of the Develop menu's items, which is
controlled by the UI process. However, Web Inspector can also be opened
without being prompted by the UI process first, in these places:
   1. In a web page's context menu, the "Inspect Element" option
   2. Inside Web Inspector, if the Debug UI is enabled, on the top right
      corner, a button to open inspector^2
   3. In WebKitTestRunner, via the TestRunner::showWebInspector function

This patch makes it so that web process can no longer send a message to
a UI process to open Web Inspector. This means web process cannot open
Web Inspector at will -- it must be either due to the UI process's
demand, or it's in one of the above three cases. More details below.

I have tested that this change preserves the above three special cases
and does prevent the web page from opening Web Inspector at will.
   - Cases #1 and #2 can be tested from the UI.
   - Case #3 can be tested with a WebKit test involving Web Inspector.
     I ran the test LayoutTests/inspector/console/js-completions.html,
     where I saw the test crashing without special treatment for this
     case.
   - To verify that the web page can't open Web Inspector, I followed
     the reproduction steps from the Radar and saw Web Inspector no
     longer opens, and opening the external URL also failed as expected.

* Source/WebKit/UIProcess/Inspector/WebInspectorUIProxy.messages.in:
* Source/WebKit/UIProcess/Inspector/WebInspectorUIProxy.h:
* Source/WebKit/UIProcess/Inspector/WebInspectorUIProxy.cpp:
(WebKit::WebInspectorUIProxy::connect):
   - If the UI process wants to open Web Inspector, it sends a
     WebInspector::Show command to the web process. This patch makes
     that command take an async reply, so that the anticipated
     WebInspectorUIProxy::OpenLocalInspectorFrontend message from the
     web process can now be delivered through that async reply instead.
     This ensures that OpenLocalInspectorFrontend can only be done
     when initiated from the UI process (due to user interaction).

(WebKit::WebInspectorUIProxy::markAsUnderTest):
(WebKit::WebInspectorUIProxy::openLocalInspectorFrontend):
(WebKit::WebInspectorUIProxy::closeFrontendPageAndWindow):
   - To avoid relying on the web process for potentially sensitive
     parameters, I reworked and removed the canAttach and underTest
     arguments from openLocalInspectorFrontend. These two values
     are now stored and managed in the UI process instead, instead of
     being passed from the web process all the time.

      - For canAttach, I noticed that the
        WebInspectorUIProxyMac::platformCanAttach method already
        implements the same logic as the web process's
        WebInspector::canAttachWindow. I filed https://webkit.org/b/283435
        as a follow-up to clean up the webProcessCanAttach parameter,
        the canAttachWindow function in the web process, and potentially
        the m_attached field too, which all become obsolete due to
        this change.
           - I couldn't figure out what the `if (m_attached)` in
             canAttachWindow check does, and to me it had no effect, as
             this function is not called while inspector is open.

      - For underTest, I'm now letting the test runner directly set
        the flag on the WebInspectorUIProxy, as part of my fix to
        address case #3 from above.

(WebKit::WebInspectorUIProxy::showConsole):
(WebKit::WebInspectorUIProxy::showResources):
(WebKit::WebInspectorUIProxy::showMainResourceForFrame):
(WebKit::WebInspectorUIProxy::togglePageProfiling):
   - As the web process can longer call OpenLocalInspectorFrontend,
     call show/connect/openLocalInspectorFrontend here in the UI process
     instead.

(WebKit::WebInspectorUIProxy::requestOpenLocalInspectorFrontend):
   - To preserve the open inspector^2 button (case #2 from above), we
     still maintain this message, but we ignore it unless it's for
     opening inspector^2, thus renaming the message as a request.
     This is all assuming that the Web Inspector is not a compromised
     web process, so we allow that message from it to come through.

* Source/WebKit/WebProcess/Inspector/WebInspector.messages.in:
* Source/WebKit/WebProcess/Inspector/WebInspector.h:
* Source/WebKit/WebProcess/Inspector/WebInspector.cpp:
(WebKit::WebInspector::show):
   - The Show message now takes an async reply, which is used to replace
     sending WebInspectorUIProxy::OpenLocalInspectorFrontend later.

(WebKit::WebInspector::showConsole):
(WebKit::WebInspector::showResources):
(WebKit::WebInspector::showMainResourceForFrame):
(WebKit::WebInspector::startPageProfiling):
(WebKit::WebInspector::stopPageProfiling):
   - Calling inspectorController()->show() no longer does anything,
     since it's now the UI process's job to show Web Inspector first,
     for these functions to merely switch to the appropriate tabs.

* Source/WebKit/WebProcess/Inspector/WebInspector.cpp:
(WebKit::WebInspector::openLocalInspectorFrontend):
* Source/WebKit/WebProcess/Inspector/WebInspectorClient.cpp:
(WebKit::WebInspectorClient::openLocalFrontend):
   - Adapt to the command's reworked version.
   - This is maintained to allow the opening of inspector^2 from the web
     process (case #2 from above). For opening inspector^1, this message
     will be ignored by the UI process.

* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::contextMenuItemSelected):
   - When the "Inspect Element" context menu item is selected (case #1
     from above), since the web process may not be privileged to open
     Web Inspector, handle the showing of inspector here in UI process.

* Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:
(WTR::TestRunner::showWebInspector):
* Tools/WebKitTestRunner/TestInvocation.cpp:
(WTR::TestInvocation::didReceiveMessageFromInjectedBundle):
* Source/WebKit/UIProcess/API/C/WKPagePrivate.h:
* Source/WebKit/UIProcess/API/C/WKPage.cpp:
(WKPageShowWebInspectorForTesting):
   - Preserve letting the WebKitTestRunner open Web Inspector (case #3
     from above).
   - Adapt to the change that we now also let the UI process know about
     the underTest flag for case #3, rather than letting UI process
     rely on the value reported by the web process.

* Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.h:
* Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:
(WKBundlePageShowInspectorForTest): Deleted.
   - No longer used due to my special fix for case #3.

Originally-landed-as: 283286.537@safari-7620-branch (694a9b5). rdar://144667626
Canonical link: https://commits.webkit.org/290260@main
eric-carlson pushed a commit that referenced this pull request Mar 3, 2025
@the-chenergy
Web Inspector: Automatically connect Web Inspector to ServiceWorker
a85f489 
https://bugs.webkit.org/show_bug.cgi?id=264576
rdar://114997939

Reviewed by BJ Burg.

(This work was done in collaboration with Razvan and was based on his
draft at WebKit@377f3e1.)

This commit enables automatically inspecting and pausing the
ServiceWorkerDebuggable. The idea is similar to the same functionalities
with the JSContext/JSGlobalObjectDebuggable. The general flow is:
1. When the debuggable is first created, we optionally mark it as
   inspectable.
2. As soon as the debuggable is marked inspectable, its main thread
   (the thread that it was created on) gets blocked.
3. When the auto-launched Web Inspector frontend finishes initializing,
   it notifies the backend.
   - It's important for the debuggable to wait for this signal because
     a genuine auto-inspection must appear attached to the debuggable
     before it begins execution, respecting any breakpoints set early on
     in its script (where auto-pausing is basically a breakpoint
     before line 1).
4. The backend unpauses the blocked debuggable. If auto-pausing was
   requested, tell the debugger agent to pause.

The service worker begins executing script unless its worker thread was
specified to start in the WorkerThreadStartMode::WaitForInspector.
During that waiting period, the worker thread can perform tasks sent
into its debugging run loop, until it's signaled to stop waiting and
continue to execute the script like normal. This commit makes use of
that interface to make the service worker pause (when justified, i.e.
developerExtrasEnabled) before running the above flow resembling
auto-inspecting a JSContext.

* Source/WebCore/workers/service/context/ServiceWorkerThread.cpp:
(WebCore::threadStartModeFromSettings):
(WebCore::ServiceWorkerThread::ServiceWorkerThread):
   - When there is potentially a remote inspector that would like to
     auto-inspect, make it so that the thread waits on start before
     executing its script.

* Source/WebCore/workers/service/context/ServiceWorkerThreadProxy.h:
* Source/WebCore/workers/service/context/ServiceWorkerThreadProxy.cpp:
(WebCore::ServiceWorkerThreadProxy::ServiceWorkerThreadProxy):
(WebCore::ServiceWorkerThreadProxy::threadStartedRunningDebuggerTasks):
   - Setting inspectability is step #1 in the above flow.
   - In step #2, calling `debuggable->setInspectable(true)` might block
     already, but we don't want that until the worker thread is setup
     and have the run loop be in debug mode, so we do that in a callback
     instead.
   - In step WebKit#4, when connection to the inspector completes or fails,
     the setInspectable call only returns then, so we unblock
     the worker thread to resume code execution.

* Source/WebCore/inspector/agents/worker/WorkerDebuggerAgent.h:
* Source/WebCore/inspector/WorkerInspectorController.h:
* Source/WebCore/inspector/WorkerInspectorController.cpp:
(WebCore::WorkerInspectorController::frontendInitialized):
(WebCore::WorkerInspectorController::connectFrontend):
(WebCore::WorkerInspectorController::disconnectFrontend):
(WebCore::WorkerInspectorController::createLazyAgents):
(WebCore::WorkerInspectorController::ensureDebuggerAgent):
* Source/WebCore/workers/service/context/ServiceWorkerDebuggable.cpp:
(WebCore::ServiceWorkerDebuggable::connect):
* Source/WebCore/workers/service/context/ServiceWorkerInspectorProxy.h:
* Source/WebCore/workers/service/context/ServiceWorkerInspectorProxy.cpp:
(WebCore::ServiceWorkerInspectorProxy::connectToWorker):
   - Mimic the logic for auto-inspecting a JSContext/JSGlobalObjectDebuggable.

* Source/JavaScriptCore/inspector/protocol/Inspector.json:
   - Step #3 in the above flow, notify the backend when frontend
     completes setting up.

* Source/WebCore/workers/service/context/ServiceWorkerDebuggable.h:
  - Allow service workers to be auto-inspected. (This is checked at https://github.com/rcaliman-apple/WebKit/blob/eng/Web-Inspector-Automatically-connect-Web-Inspector-to-ServiceWorker/Source/JavaScriptCore/inspector/remote/RemoteInspectionTarget.cpp#L95)

* Source/WTF/wtf/PlatformEnableCocoa.h:
   - Add feature flag just in case.

Canonical link: https://commits.webkit.org/291167@main
webkit-commit-queue pushed a commit that referenced this pull request Apr 3, 2025
@alanbaradlay
Stale float state caused by 'content visibility' may lead to ASSERT i…
cab087e 
…n addFloatsToNewParent

https://bugs.webkit.org/show_bug.cgi?id=290898
<rdar://143296265>

Reviewed by Antti Koivisto.

In this patch
1. we let m_floatingObjects go stale on the skipped root (we already do that for the skipped subtree by not running layout)
2. we descend into skipped subtrees while cleaning up floats even when m_floatingObjects is stale/empty

Having up-to-date m_floatingObjects on the skipped root, while stale m_floatingObjects on the skipped subtree can lead to issues when
(#1) a previously intrusive float
(#2) becomes non-intrusive and
(#3) eventually gets deleted
prevents us from being able to cleanup m_floatingObjects in skipped subtree(s).

at #1 m_floatingObjects is populated with the intrusive float (both skipped root and renderers in skipped subtree)
and at #2 since we only run layout on the skipped root, m_floatingObjects gets updated by removing this previously intrusive float (skipped subtree becomes stale)
and at #3 we don't descend into the skipped subtree to cleanup m_floatingObjects since the skipped root does not have this float anymore (removed at #2).

* Source/WebCore/rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::markSiblingsWithFloatsForLayout):

Canonical link: https://commits.webkit.org/293119@main
eric-carlson pushed a commit that referenced this pull request Apr 28, 2025
@alanbaradlay
Stale float state caused by 'content visibility' may lead to ASSERT i…
68742b8 
…n addFloatsToNewParent

https://bugs.webkit.org/show_bug.cgi?id=290898
<rdar://149579273>

Reviewed by Antti Koivisto.

In this patch
1. we let m_floatingObjects go stale on the skipped root (we already do that for the skipped subtree by not running layout)
2. we descend into skipped subtrees while cleaning up floats even when m_floatingObjects is stale/empty

Having up-to-date m_floatingObjects on the skipped root, while stale m_floatingObjects on the skipped subtree can lead to issues when
(#1) a previously intrusive float
(#2) becomes non-intrusive and
(#3) eventually gets deleted
prevents us from being able to cleanup m_floatingObjects in skipped subtree(s).

at #1 m_floatingObjects is populated with the intrusive float (both skipped root and renderers in skipped subtree)
and at #2 since we only run layout on the skipped root, m_floatingObjects gets updated by removing this previously intrusive float (skipped subtree becomes stale)
and at #3 we don't descend into the skipped subtree to cleanup m_floatingObjects since the skipped root does not have this float anymore (removed at #2).

* Source/WebCore/rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::markSiblingsWithFloatsForLayout):

Canonical link: https://commits.webkit.org/293889@main
eric-carlson pushed a commit that referenced this pull request May 28, 2025
@JonWBedard
Build Fix attempt #3 for note: forward declaration of 'WebCore::Setti…
b7213b9 
…ngs'

rdar://151332172

Unreviewed build fix.

* Source/WebKit/WebProcess/Model/ModelProcessModelPlayer.cpp:

Canonical link: https://commits.webkit.org/294933@main
eric-carlson pushed a commit that referenced this pull request Jun 30, 2025
@alanbaradlay
Inputs within inline-block containers shift vertically when text is d…
22d6d63 
…eleted and re-entered into an input

https://bugs.webkit.org/show_bug.cgi?id=294558
<rdar://problem/154094432>

Reviewed by Antti Koivisto.

1. RenderTextControlSingleLine's inner renderer is vertically centered.
2. RenderTextControlSingleLine's inner renderer provides the baseline position for the input box.
3. The baseline position is passed to inline layout and is used to align other content on the line (when baseline alignment applies).

As text is appended to the input box:

1. An empty RenderText is constructed and inserted into the tree.
2. InsertIntoTextNodeCommand::doApply() is called.
3. RenderText is populated by calling setText.

In step #2, layout is prematurely run on the content (see passwordEchoEnabled) before it is populated in step #3.
This layout generates an empty inline display content with a 0px-tall line box, which gets vertically centered.
This "centered line" then becomes the baseline for the rest of the content.
In step #3, another layout is run on the input box, this time with populated content, but the layout is limited to the input only, so adjacent content does not get alignment treatment.
This is how the offset occurs (result of the premature layout).

There are a few issues here:

1. We should not run layout on the empty content before RenderText is populated (webkit.org/b/294880).
2. RenderTextControlSingleLine should not center the inner renderer when it has no content (webkit.org/b/294881).
3. IFC should not report valid line boxes when there’s no content at all (i.e., when no display boxes are generated).

This patch addresses issue #3.

* LayoutTests/fast/editing/incorrect-baseline-for-input-adjacent-content-expected.html: Added.
* LayoutTests/fast/editing/incorrect-baseline-for-input-adjacent-content.html: Added.
* Source/WebCore/layout/integration/inline/LayoutIntegrationLineLayout.cpp:
(WebCore::LayoutIntegration::LineLayout::firstLineBox const):
(WebCore::LayoutIntegration::LineLayout::lastLineBox const):

Canonical link: https://commits.webkit.org/296569@main
webkit-commit-queue pushed a commit that referenced this pull request Sep 26, 2025
@Constellation
[JSC] Immutable Load should look for the CSE target in dominators
62300f8 
https://bugs.webkit.org/show_bug.cgi?id=299504
rdar://161294228

Reviewed by Yijia Huang.

Previous one 300327@main worked for the same basic block's load,
but it didn't work for the load in dominators. This patch updates CSE
rules to make immutable load elimination work with dominators' load.

Like,

    BB#0
    @0: Load(@x, immutable)
    @1: CCall(...) # potentially clobber everything
    Branch ... #1, #2

    BB#1
    @2: CCall(...) # potentially clobber everything
    Jump #3

    BB#2
    @3: CCall(...) # potentially clobber everything
    Jump #3

    BB#3
    @4: Load(@x, immutable)
    ...

Then @4 should be replaced with Identity(@0) as dominator BB#0 is having
immutable load @0 matching to @4.

Tests: Source/JavaScriptCore/b3/testb3_1.cpp
       Source/JavaScriptCore/b3/testb3_8.cpp
* Source/JavaScriptCore/b3/B3EliminateCommonSubexpressions.cpp:
* Source/JavaScriptCore/b3/testb3.h:
* Source/JavaScriptCore/b3/testb3_1.cpp:
(run):
* Source/JavaScriptCore/b3/testb3_8.cpp:
(testLoadImmutableDominated):
(testLoadImmutableNonDominated):

Canonical link: https://commits.webkit.org/300562@main
eric-carlson pushed a commit that referenced this pull request Oct 15, 2025
@RupinMittal
[Navigation API] navigation-api/per-entry-events/dispose-for-navigati…
204956f 
…on-in-child.html is failing

https://bugs.webkit.org/show_bug.cgi?id=299628
rdar://161203486

Reviewed by Basuke Suzuki.

Suppose we have a mainframe and an iframe and this series of navigations happens:
1. iframe fragment navigates to "/#a"
2. main frame fragment navigates to "/#1"
3. main frame fragment navigates to "/#2"
4. main frame fragment navigates to "/#3"
5. iframe goes back
6. iframe fragment navigates to "/#b"

After Step 5, the UI Process b/f list should be:

A) mainframe - URL -    ItemID A
   ** iframe - URL -    ItemID A
B) mainframe - URL -    ItemID B
   ** iframe - URL/#a - ItemID B
C) mainframe - URL/#1 - ItemID C
   ** iframe - URL/#a - ItemID C
D) mainframe - URL/#2 - ItemID D
   ** iframe - URL/#a - ItemID D
E) mainframe - URL/#3 - ItemID E
   ** iframe - URL/#a - ItemID E

The mainframe's Navigation object's m_entries should be:

A) mainframe - URL -    ItemID A
C) mainframe - URL/#1 - ItemID C
D) mainframe - URL/#2 - ItemID D
E) mainframe - URL/#3 - ItemID E  <--- current index

The iframe's Navigation object's m_entries should be:

A) ** iframe - URL -    ItemID A  <--- current index
E) ** iframe - URL/#a - ItemID E

According to this layout test, after Step 6:

The mainframe's Navigation object's m_entries should be:

A) mainframe - URL -    ItemID A  <--- current index

The iframe's Navigation object's m_entries should be:

A) ** iframe - URL -    ItemID A
F) ** iframe - URL/#b - ItemID F  <--- current index

So when a subframe has a PUSH same-document navigation and disposes of any
forward entries, any parent frame must do the same.

This test was failing because the parent frame was not disposing of its
forward entries. To fix this, we add a new function to recusively dispose
of all forward entries in any parent frames when a subframe has a PUSH
same-document navigation. We use the ItemID to determine what entry must
stay and then dispose of any entries that come after that one.

* LayoutTests/imported/w3c/web-platform-tests/navigation-api/per-entry-events/dispose-for-navigation-in-child-expected.txt:
* Source/WebCore/page/LocalDOMWindowProperty.cpp:
(WebCore::LocalDOMWindowProperty::protectedFrame const):
* Source/WebCore/page/LocalDOMWindowProperty.h:
* Source/WebCore/page/Navigation.cpp:
(WebCore::Navigation::updateNavigationEntry):
(WebCore::Navigation::disposeOfForwardEntriesInParents):

Call recursivelyDisposeOfForwardEntriesInParents on the main frame,
which will traverse down the frame tree, and for each frame until we reach the
subframe that actually navigated, dispose of any forward entries.

(WebCore::Navigation::recursivelyDisposeOfForwardEntriesInParents):
(WebCore::Navigation::updateForNavigation):

This is called for same-document navigations. If it's a PUSH navigation, call
disposeOfForwardEntriesInParents. The ItemID that we keep in these parent frames
is the current ItemID right before this PUSH operation happens.

* Source/WebCore/page/Navigation.h:

Canonical link: https://commits.webkit.org/300721@main
eric-carlson pushed a commit that referenced this pull request Feb 27, 2026
@alanbaradlay
[IFC] Do not create display boxes on constrained lines for empty span…
dbf7847 
…ning inline boxes

https://bugs.webkit.org/show_bug.cgi?id=308696

Reviewed by Antti Koivisto.

1. 303091@main: we started creating empty (inline box) display boxes to be able to
   provided offsetTop/offsetHeight values for cases like
   <div>
     content<br>
     <span id=empty-inline-box></span>
   </div>
   where before the fix, the trailing <span> did not initiate any display boxes (empty line).

2. However after 303091@main, we also started creating empty display boxes for _line_spanning_
   inline boxes when we couldn't fit any content on the line due to intrusive floats.
   <div>
     content
      <float><span> this does not fit the line due to the float here</span>
   </div>
  and this turned out to be not web-compatible.

3. 307916@main and 308175@main addressed this issue by filtering the display box rects
   as we collect them in RenderInline's generateLineBoxRects (this is called by functions like
   offsetHeight etc.

Let's fix #2 and undo #3.

* LayoutTests/fast/inline/empty-inline-box-bounding-rect.html:
* Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayContentBuilder.cpp:
(WebCore::Layout::InlineDisplayContentBuilder::processNonBidiContent):
* Source/WebCore/rendering/RenderInline.cpp:
(WebCore::RenderInline::generateLineBoxRects const):

Canonical link: https://commits.webkit.org/308289@main
webkit-commit-queue pushed a commit that referenced this pull request Apr 28, 2026
@jernoble
[Build Speed] Group together source files requiring RenderStyle+Gette…
466ee71 
…rsInlines.h into separate UnifiedBuild sources

https://bugs.webkit.org/show_bug.cgi?id=313363
rdar://175635833

Reviewed by Geoffrey Garen.

Source files that include expensive headers are now grouped together in
unified source bundles, reducing the number of translation units that
pay the parsing cost for those headers.

Previously, files needing RenderStyle+GettersInlines.h (a 3.7s parse
cost) were scattered across 190 unified source bundles. Most of those
bundles also contained files that didn't need the header, but paid
the cost anyway since unified sources share a single preprocessing
context. By tagging these files and grouping them together, the header
is now parsed in only 91 bundles instead of 190.

Measured impact on WebCore clean build:
    Frontend parsing: 8018s -> 7351s (-667s, -8.3%)
    Backend/Codegen: 558s -> 542s (-16s, -2.8%)
    RenderStyle+GettersInlines.h: 730s -> 339s (-391s, -54%)

RenderStyle+GettersInlines.h drops from the #2 most expensive WebCore
header to... #3, even while it's parsing cost dropped by more than half.

The implementation adds a general-purpose @Header:<name> attribute
to the unified source generator. Files tagged with the same header
group name are placed into their own unified source bundles, separate
from untagged files. This prevents expensive headers from "leaking"
into bundles that don't need them.

The mechanism is extensible; other expensive headers can be targeted
the same way by tagging their includers in Sources.txt with a new
@Header:<name> group. No header refactoring or code changes are
required; only the Sources.txt tagging and a bundle count parameter
in generate-unified-sources.sh.

Compensating #includes were added to several .cpp files that
previously relied on transitive includes from other files in their
unified source bundle. Regrouping exposed these missing direct
includes.

* Source/WTF/Scripts/generate-unified-source-bundles.py:
(SourceFile.__init__):
(parse_args):
(main):
* Source/WebCore/Scripts/generate-unified-sources.sh:
* Source/WebCore/Sources.txt:
* Source/WebCore/SourcesCocoa.txt:
* Source/WebCore/UnifiedSources-output.xcfilelist:
* Source/WebCore/WebCore.xcodeproj/project.pbxproj:
* Source/WebCore/accessibility/AXCrossProcessSearch.cpp:
* Source/WebCore/accessibility/AXLogger.cpp:
* Source/WebCore/accessibility/AccessibilityScrollView.cpp:
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:
* Source/WebCore/dom/DeviceMotionEvent.cpp:
* Source/WebCore/dom/InternalObserverInspect.cpp:
* Source/WebCore/editing/cocoa/AutofillElements.cpp:
* Source/WebCore/history/CachedFrame.cpp:
* Source/WebCore/html/ModelDocument.cpp:
* Source/WebCore/inspector/agents/InspectorDOMAgent.cpp:
* Source/WebCore/layout/integration/grid/LayoutIntegrationGridCoverage.cpp:
(WebCore::LayoutIntegration::printLegacyGridReasons):
(WebCore::LayoutIntegration::printTextForSubtree): Deleted.
* Source/WebCore/page/FrameTree.cpp:
* Source/WebCore/page/ImageAnalysisQueue.cpp:
* Source/WebCore/page/IntelligenceTextEffectsSupport.cpp:
* Source/WebCore/page/LocalFrame.cpp:
* Source/WebCore/page/scrolling/ScrollLatchingController.cpp:
* Source/WebCore/platform/animation/values/paths/AcceleratedEffectBoxPath.cpp:
* Source/WebCore/platform/animation/values/paths/AcceleratedEffectBoxPath.h:
* Source/WebCore/platform/graphics/cg/GradientRendererCG.cpp:
* Source/WebCore/platform/graphics/cg/GradientRendererCG.h:
(WebCore::GradientRendererCG::createGradientBySampling):
* Source/WebCore/rendering/RenderModel.cpp:
* Source/WebCore/rendering/SubtreeScrollbarChangesState.cpp:
* Source/WebCore/style/StyleLocalPropertyRegistry.cpp:

Canonical link: https://commits.webkit.org/312184@main
eric-carlson pushed a commit that referenced this pull request Apr 29, 2026
@jernoble
[Build Speed] Group together source files requiring RenderStyle+Gette…
21ce77b 
…rsInlines.h into separate UnifiedBuild sources

https://bugs.webkit.org/show_bug.cgi?id=313363
rdar://175635833

Reviewed by Geoffrey Garen.

Source files that include expensive headers are now grouped together in
unified source bundles, reducing the number of translation units that
pay the parsing cost for those headers.

Previously, files needing RenderStyle+GettersInlines.h (a 3.7s parse
cost) were scattered across 190 unified source bundles. Most of those
bundles also contained files that didn't need the header, but paid
the cost anyway since unified sources share a single preprocessing
context. By tagging these files and grouping them together, the header
is now parsed in only 91 bundles instead of 190.

Measured impact on WebCore clean build:
    Frontend parsing: 8018s -> 7351s (-667s, -8.3%)
    Backend/Codegen: 558s -> 542s (-16s, -2.8%)
    RenderStyle+GettersInlines.h: 730s -> 339s (-391s, -54%)

RenderStyle+GettersInlines.h drops from the #2 most expensive WebCore
header to... #3, even while it's parsing cost dropped by more than half.

The implementation adds a general-purpose @Header:<name> attribute
to the unified source generator. Files tagged with the same header
group name are placed into their own unified source bundles, separate
from untagged files. This prevents expensive headers from "leaking"
into bundles that don't need them.

The mechanism is extensible; other expensive headers can be targeted
the same way by tagging their includers in Sources.txt with a new
@Header:<name> group. No header refactoring or code changes are
required; only the Sources.txt tagging and a bundle count parameter
in generate-unified-sources.sh.

Compensating #includes were added to several .cpp files that
previously relied on transitive includes from other files in their
unified source bundle. Regrouping exposed these missing direct
includes.

* Source/WTF/Scripts/generate-unified-source-bundles.py:
(SourceFile.__init__):
(parse_args):
(main):
* Source/WebCore/Scripts/generate-unified-sources.sh:
* Source/WebCore/Sources.txt:
* Source/WebCore/SourcesCocoa.txt:
* Source/WebCore/UnifiedSources-output.xcfilelist:
* Source/WebCore/WebCore.xcodeproj/project.pbxproj:
* Source/WebCore/accessibility/AXCrossProcessSearch.cpp:
* Source/WebCore/accessibility/AXLogger.cpp:
* Source/WebCore/accessibility/AccessibilityScrollView.cpp:
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:
* Source/WebCore/dom/DeviceMotionEvent.cpp:
* Source/WebCore/dom/InternalObserverInspect.cpp:
* Source/WebCore/editing/cocoa/AutofillElements.cpp:
* Source/WebCore/history/CachedFrame.cpp:
* Source/WebCore/html/ModelDocument.cpp:
* Source/WebCore/inspector/agents/InspectorDOMAgent.cpp:
* Source/WebCore/layout/integration/grid/LayoutIntegrationGridCoverage.cpp:
(WebCore::LayoutIntegration::printLegacyGridReasons):
(WebCore::LayoutIntegration::printTextForSubtree): Deleted.
* Source/WebCore/page/FrameTree.cpp:
* Source/WebCore/page/ImageAnalysisQueue.cpp:
* Source/WebCore/page/IntelligenceTextEffectsSupport.cpp:
* Source/WebCore/page/LocalFrame.cpp:
* Source/WebCore/page/scrolling/ScrollLatchingController.cpp:
* Source/WebCore/platform/animation/values/paths/AcceleratedEffectBoxPath.cpp:
* Source/WebCore/platform/animation/values/paths/AcceleratedEffectBoxPath.h:
* Source/WebCore/platform/graphics/cg/GradientRendererCG.cpp:
* Source/WebCore/platform/graphics/cg/GradientRendererCG.h:
(WebCore::GradientRendererCG::createGradientBySampling):
* Source/WebCore/rendering/RenderModel.cpp:
* Source/WebCore/rendering/SubtreeScrollbarChangesState.cpp:
* Source/WebCore/style/StyleLocalPropertyRegistry.cpp:

Canonical link: https://commits.webkit.org/312240@main
eric-carlson pushed a commit that referenced this pull request May 1, 2026
@sosukesuzuki
[JSC] Unroll zero-fill for NewTypedArray with small constant length
d54aaf2 
https://bugs.webkit.org/show_bug.cgi?id=312225

Reviewed by Yusuke Suzuki.

When the length of `new TypedArray(N)` is a small Int32 constant, DFG/FTL
still emitted a runtime zero-fill loop and fastSizeLimit branch even
though the byte size is known at compile time.

This patch derives the constant byte size in emitNewTypedArrayWithSizeInRegister
and, when set, skips the fastSizeLimit branch, materializes the byte size as
an immediate, and zero-fills via emitFillStorageWithJSEmpty (unrolled
`stp xzr, xzr` on ARM64) for up to 16 words. Guarded by USE(JSVALUE64) since
JSEmpty is non-zero on JSVALUE32_64. In FTL, a constInt32 word count is passed
to splatWords so its existing unroll path is taken and Air fuses the stores
into stp.

For `new Float64Array(4)` on ARM64, the NewTypedArray fast path goes from
74 to 47 dynamic instructions in DFG (-36%) and 52 to 36 in FTL (-31%):

    DFG before: cmp x0,#0x3e8; b.hi; lsl w4,w0,#3; ...
                cbz w0; mov; lsl; (sub; str wzr; cbnz) x8
    DFG after:  orr w4,wzr,#0x20; ...
                stp xzr,xzr,[x3]; stp xzr,xzr,[x3,#0x10]

Test: JSTests/stress/new-typed-array-constant-size-zero-fill.js

* JSTests/stress/new-typed-array-constant-size-zero-fill.js: Added.
(shouldBeZeroFilled):
(f64_0):
(i8_0):
(f64_1):
(u8_1):
(i8_3):
(i8_7):
(i8_9):
(u16_5):
(f32_3):
(f64_16):
(f64_17):
(u32_32):
(u32_33):
(i8_128):
(i8_129):
(f64_10):
(f64_11):
(f64_1000):
(f64_1001):
(bi64_2):
(bu64_16):
(i32_neg):
(dirty):
(fresh):
(i.catch):
* Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:
* Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::emitNewTypedArrayWithSize):

Canonical link: https://commits.webkit.org/312432@main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jyavenard @eric-carlson