Skip to content

Preliminary work on the switch to BUrlSession #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

pulkomandy
Copy link
Member

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.

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.
@pulkomandy
Copy link
Member Author

We have decided to not include the BUrlSession changes in libnetservices for now. I am still not clear why we need it, since it is exactly the same as BUrlContext. Why can't we keep and improve BUrlContext instead?

Keeping this merge request for tracing, archival, and as a place to discuss things.

pulkomandy pushed a commit that referenced this pull request Aug 1, 2021
https://bugs.webkit.org/show_bug.cgi?id=228047

Reviewed by Phil Pizlo.

Pre-indexed addressing means that the address is the sum of the value in the 64-bit base register
and an offset, and the address is then written back to the base register. And post-indexed
addressing means that the address is the value in the 64-bit base register, and the sum of the
address and the offset is then written back to the base register. They are relatively common for
loops to iterate over an array by increasing/decreasing a pointer into the array at each iteration.
With such an addressing mode, the instruction selector can merge the increment and access the array.

#####################################
## Pre-Index Address Mode For Load ##
#####################################

LDR Wt, [Xn, #imm]!

In B3 Reduction Strength, since we have this reduction rule:
    Turn this: Load(Add(address, offset1), offset = offset2)
    Into this: Load(address, offset = offset1 + offset2)

Then, the equivalent pattern is:
    address = Add(base, offset)
    ...
    memory = Load(base, offset)

First, we convert it to the canonical form:
    address = Add(base, offset)
    newMemory = Load(base, offset) // move the memory to just after the address
    ...
    memory = Identity(newMemory)

Next, lower to Air:
    Move %base, %address
    Move (%address, prefix(offset)), %newMemory

######################################
## Post-Index Address Mode For Load ##
######################################

LDR Wt, [Xn], #imm

Then, the equivalent pattern is:
    memory = Load(base, 0)
    ...
    address = Add(base, offset)

First, we convert it to the canonical form:
    newOffset = Constant
    newAddress = Add(base, offset)
    memory = Load(base, 0) // move the offset and address to just before the memory
    ...
    offset = Identity(newOffset)
    address = Identity(newAddress)

Next, lower to Air:
    Move %base, %newAddress
    Move (%newAddress, postfix(offset)), %memory

#############################
## Pattern Match Algorithm ##
#############################

To detect the pattern for prefix/postfix increment address is tricky due to the structure in B3 IR. The
algorithm used in this patch is to collect the first valid values (add/load), then search for any
paired value (load/add) to match all of them. In worst case, the runtime complexity is O(n^2)
when n is the number of all values.

After collecting two sets of candidates, we match the prefix incremental address first since it seems
more beneficial to the compiler (shown in the next section). And then, go for the postfix one.

##############################################
## Test for Pre/Post-Increment Address Mode ##
##############################################

Given Loop with Pre-Increment:
int64_t ldr_pre(int64_t *p) {
    int64_t res = 0;
    while (res < 10)
        res += *++p;
    return res;
}

B3 IR:
------------------------------------------------------
BB#0: ; frequency = 1.000000
    Int64 b@0 = Const64(0)
    Int64 b@2 = ArgumentReg(%x0)
    Void b@20 = Upsilon($0(b@0), ^18, WritesLocalState)
    Void b@21 = Upsilon(b@2, ^19, WritesLocalState)
    Void b@4 = Jump(Terminal)
Successors: #1
BB#1: ; frequency = 1.000000
Predecessors: #0, #2
    Int64 b@18 = Phi(ReadsLocalState)
    Int64 b@19 = Phi(ReadsLocalState)
    Int64 b@7 = Const64(10)
    Int32 b@8 = AboveEqual(b@18, $10(b@7))
    Void b@9 = Branch(b@8, Terminal)
Successors: Then:#3, Else:#2
BB#2: ; frequency = 1.000000
Predecessors: #1
    Int64 b@10 = Const64(8)
    Int64 b@11 = Add(b@19, $8(b@10))
    Int64 b@13 = Load(b@11, ControlDependent|Reads:Top)
    Int64 b@14 = Add(b@18, b@13)
    Void b@22 = Upsilon(b@14, ^18, WritesLocalState)
    Void b@23 = Upsilon(b@11, ^19, WritesLocalState)
    Void b@16 = Jump(Terminal)
Successors: #1
BB#3: ; frequency = 1.000000
Predecessors: #1
    Void b@17 = Return(b@18, Terminal)
Variables:
    Int64 var0
    Int64 var1
------------------------------------------------------

W/O Pre-Increment Address Mode:
------------------------------------------------------
...
BB#2: ; frequency = 1.000000
Predecessors: #1
    Move $8, %x3, $8(b@12)
    Add64 $8, %x0, %x1, b@11
    Move (%x0,%x3), %x0, b@13
    Add64 %x0, %x2, %x2, b@14
    Move %x1, %x0, b@23
    Jump b@16
Successors: #1
...
------------------------------------------------------

W/ Pre-Increment Address Mode:
------------------------------------------------------
...
BB#2: ; frequency = 1.000000
Predecessors: #1
    MoveWithIncrement64 (%x0,Pre($8)), %x2, b@13
    Add64 %x2, %x1, %x1, b@14
    Jump b@16
Successors: #1
...
------------------------------------------------------

Given Loop with Post-Increment:
int64_t ldr_pre(int64_t *p) {
    int64_t res = 0;
    while (res < 10)
        res += *p++;
    return res;
}

B3 IR:
------------------------------------------------------
BB#0: ; frequency = 1.000000
    Int64 b@0 = Const64(0)
    Int64 b@2 = ArgumentReg(%x0)
    Void b@20 = Upsilon($0(b@0), ^18, WritesLocalState)
    Void b@21 = Upsilon(b@2, ^19, WritesLocalState)
    Void b@4 = Jump(Terminal)
Successors: #1
BB#1: ; frequency = 1.000000
Predecessors: #0, #2
    Int64 b@18 = Phi(ReadsLocalState)
    Int64 b@19 = Phi(ReadsLocalState)
    Int64 b@7 = Const64(10)
    Int32 b@8 = AboveEqual(b@18, $10(b@7))
    Void b@9 = Branch(b@8, Terminal)
Successors: Then:#3, Else:#2
BB#2: ; frequency = 1.000000
Predecessors: #1
    Int64 b@10 = Load(b@19, ControlDependent|Reads:Top)
    Int64 b@11 = Add(b@18, b@10)
    Int64 b@12 = Const64(8)
    Int64 b@13 = Add(b@19, $8(b@12))
    Void b@22 = Upsilon(b@11, ^18, WritesLocalState)
    Void b@23 = Upsilon(b@13, ^19, WritesLocalState)
    Void b@16 = Jump(Terminal)
Successors: #1
BB#3: ; frequency = 1.000000
Predecessors: #1
    Void b@17 = Return(b@18, Terminal)
Variables:
    Int64 var0
    Int64 var1
------------------------------------------------------

W/O Post-Increment Address Mode:
------------------------------------------------------
...
BB#2: ; frequency = 1.000000
Predecessors: #1
    Move (%x0), %x2, b@10
    Add64 %x2, %x1, %x1, b@11
    Add64 $8, %x0, %x0, b@13
    Jump b@16
Successors: #1
...
------------------------------------------------------

W/ Post-Increment Address Mode:
------------------------------------------------------
...
BB#2: ; frequency = 1.000000
Predecessors: #1
    MoveWithIncrement64 (%x0,Post($8)), %x2, b@10
    Add64 %x2, %x1, %x1, b@11
    Jump b@16
Successors: #1
...
------------------------------------------------------

* Sources.txt:
* assembler/AbstractMacroAssembler.h:
(JSC::AbstractMacroAssembler::PreIndexAddress::PreIndexAddress):
(JSC::AbstractMacroAssembler::PostIndexAddress::PostIndexAddress):
* assembler/MacroAssemblerARM64.h:
(JSC::MacroAssemblerARM64::load64):
(JSC::MacroAssemblerARM64::load32):
(JSC::MacroAssemblerARM64::store64):
(JSC::MacroAssemblerARM64::store32):
* assembler/testmasm.cpp:
(JSC::testStorePrePostIndex32):
(JSC::testStorePrePostIndex64):
(JSC::testLoadPrePostIndex32):
(JSC::testLoadPrePostIndex64):
* b3/B3CanonicalizePrePostIncrements.cpp: Added.
(JSC::B3::canonicalizePrePostIncrements):
* b3/B3CanonicalizePrePostIncrements.h: Copied from Source/JavaScriptCore/b3/B3ValueKeyInlines.h.
* b3/B3Generate.cpp:
(JSC::B3::generateToAir):
* b3/B3LowerToAir.cpp:
* b3/B3ValueKey.h:
* b3/B3ValueKeyInlines.h:
(JSC::B3::ValueKey::ValueKey):
* b3/air/AirArg.cpp:
(JSC::B3::Air::Arg::jsHash const):
(JSC::B3::Air::Arg::dump const):
(WTF::printInternal):
* b3/air/AirArg.h:
(JSC::B3::Air::Arg::preIndex):
(JSC::B3::Air::Arg::postIndex):
(JSC::B3::Air::Arg::isPreIndex const):
(JSC::B3::Air::Arg::isPostIndex const):
(JSC::B3::Air::Arg::isMemory const):
(JSC::B3::Air::Arg::base const):
(JSC::B3::Air::Arg::offset const):
(JSC::B3::Air::Arg::isGP const):
(JSC::B3::Air::Arg::isFP const):
(JSC::B3::Air::Arg::isValidPreIndexForm):
(JSC::B3::Air::Arg::isValidPostIndexForm):
(JSC::B3::Air::Arg::isValidForm const):
(JSC::B3::Air::Arg::forEachTmpFast):
(JSC::B3::Air::Arg::forEachTmp):
(JSC::B3::Air::Arg::asPreIndexAddress const):
(JSC::B3::Air::Arg::asPostIndexAddress const):
* b3/air/AirOpcode.opcodes:
* b3/air/opcode_generator.rb:
* b3/testb3.h:
* b3/testb3_3.cpp:
(testLoadPreIndex32):
(testLoadPreIndex64):
(testLoadPostIndex32):
(testLoadPostIndex64):
(addShrTests):
* jit/ExecutableAllocator.cpp:
(JSC::jitWriteThunkGenerator):


Canonical link: https://commits.webkit.org/240125@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@280493 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy pushed a commit that referenced this pull request Oct 31, 2021
https://bugs.webkit.org/show_bug.cgi?id=232303

Patch by Xan López <[email protected]> on 2021-10-26
Reviewed by Mark Lam.

This patch does two things:

Add the .size and .type directives to every llint "function"
(global, llint opcode, 'glue'). This allows a debugger to tell you
in what logical function you are inside the giant chunk of code
that is the llint interpreter. So instead of something like this:

(gdb) x/5i $pc
  => 0xf5f8af60 <wasmLLIntPCRangeStart+3856>:  b.n     0xf5f8af6c <wasmLLIntPCRangeStart+3868>
     0xf5f8af62 <wasmLLIntPCRangeStart+3858>:  ldr     r2, [r7, #8]
     0xf5f8af64 <wasmLLIntPCRangeStart+3860>:  ldr     r2, [r2, #28]
     0xf5f8af66 <wasmLLIntPCRangeStart+3862>:  subs    r0, #16
     0xf5f8af68 <wasmLLIntPCRangeStart+3864>:  ldr.w   r0, [r2, r0, lsl #3]

you get something like this:

(gdb) x/5i $pc
  => 0xf5f8c770 <wasm_f32_add+12>:      bge.n   0xf5f8c77c <wasm_f32_add+24>
     0xf5f8c772 <wasm_f32_add+14>:      add.w   r6, r7, r9, lsl #3
     0xf5f8c776 <wasm_f32_add+18>:      vldr    d0, [r6]
     0xf5f8c77a <wasm_f32_add+22>:      b.n     0xf5f8c78c <wasm_f32_add+40>
     0xf5f8c77c <wasm_f32_add+24>:      ldr     r2, [r7, #8]

The other change adds a local symbol (in addition to an internal
label) to all the "glue" labels. That allows wasm opcodes to be
seen by the debugger (and the user to break on them), among other
things.

* CMakeLists.txt: tell offlineasm we use the ELF binary format on Linux.
* llint/LowLevelInterpreter.cpp: emit a non-local label for "glue" labels.
* offlineasm/asm.rb: emit the .size and .type directives for every
llint "function" on ELF systems.

Canonical link: https://commits.webkit.org/243545@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@284868 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy pushed a commit that referenced this pull request Dec 19, 2021
…nstead of _relevant_

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

Patch by Alexey Shvayka <[email protected]> on 2021-12-10
Reviewed by Sam Weinig.

LayoutTests/imported/w3c:

Import WPT tests from web-platform-tests/wpt#32012.

* web-platform-tests/dom/events/Event-timestamp-cross-realm-getter-expected.txt: Added.
* web-platform-tests/dom/events/Event-timestamp-cross-realm-getter.html: Added.
* web-platform-tests/html/browsers/history/the-history-interface/history_back_cross_realm_method-expected.txt: Added.
* web-platform-tests/html/browsers/history/the-history-interface/history_back_cross_realm_method.html: Added.
* web-platform-tests/html/browsers/history/the-history-interface/history_forward_cross_realm_method-expected.txt: Added.
* web-platform-tests/html/browsers/history/the-history-interface/history_forward_cross_realm_method.html: Added.
* web-platform-tests/html/browsers/history/the-history-interface/history_go_cross_realm_method-expected.txt: Added.
* web-platform-tests/html/browsers/history/the-history-interface/history_go_cross_realm_method.html: Added.
* web-platform-tests/html/webappapis/scripting/reporterror-cross-realm-method-expected.txt: Added.
* web-platform-tests/html/webappapis/scripting/reporterror-cross-realm-method.html: Added.
* web-platform-tests/html/webappapis/structured-clone/structured-clone-cross-realm-method-expected.txt: Added.
* web-platform-tests/html/webappapis/structured-clone/structured-clone-cross-realm-method.html: Added.
* web-platform-tests/requestidlecallback/callback-timeRemaining-cross-realm-method-expected.txt: Added.
* web-platform-tests/requestidlecallback/callback-timeRemaining-cross-realm-method.html: Added.

Source/WebCore:

This patch replaces _current_ global object with _relevant_, as per recommendation
for spec authors [1], for select WebIDL operations / attributes that satisfy all
the following conditions:

  1) it's an instance member: static ones and constructors can't use _relevant_;
  2) it's on standards track (not deprecated / WebKit-only / internal);
  3) the change is directly observable: global object is used for something
     beyond lifecycle / event loop / parsing CSS etc;
  4) the change either aligns WebKit with both Blink and Gecko,
     or the spec explicitly requires _relevant_ realm / settings object.

Most of the remaining [CallWith=GlobalObject] instances are correctly used for
converting JS arguments to WebIDL values; the rest, along with _current_ Document
and ScriptExecutionContext, either match the spec or replacing them with _relevant_
global object is not directly observable (see condition #3).

This change is aimed at fixing web-exposed APIs rather than performing a global cleanup.

[1] https://html.spec.whatwg.org/multipage/webappapis.html#concept-current-everything

Tests: imported/w3c/web-platform-tests/dom/events/Event-timestamp-cross-realm-getter.html
       imported/w3c/web-platform-tests/html/browsers/history/the-history-interface/history_back_cross_realm_method.html
       imported/w3c/web-platform-tests/html/browsers/history/the-history-interface/history_forward_cross_realm_method.html
       imported/w3c/web-platform-tests/html/browsers/history/the-history-interface/history_go_cross_realm_method.html
       imported/w3c/web-platform-tests/html/webappapis/scripting/reporterror-cross-realm-method.html
       imported/w3c/web-platform-tests/html/webappapis/structured-clone/structured-clone-cross-realm-method.html
       imported/w3c/web-platform-tests/requestidlecallback/callback-timeRemaining-cross-realm-method.html

* Modules/indexeddb/IDBFactory.idl:
https://www.w3.org/TR/IndexedDB/#dom-idbfactory-open (step 2)
https://www.w3.org/TR/IndexedDB/#dom-idbfactory-deletedatabase (step 1)
https://www.w3.org/TR/IndexedDB/#dom-idbfactory-databases (step 1)

* Modules/paymentrequest/PaymentRequest.idl:
https://www.w3.org/TR/payment-request/#show-method (steps 2-4)
https://www.w3.org/TR/payment-request/#can-make-payment-algorithm (before step 1)

* bindings/scripts/CodeGeneratorJS.pm:
(GenerateCallWith):
* bindings/scripts/IDLAttributes.json:
* bindings/scripts/test/JS/JSTestObj.cpp:
* bindings/scripts/test/TestObj.idl:
* dom/Event.idl:
https://dom.spec.whatwg.org/#inner-event-creation-steps (step 3)

* dom/IdleDeadline.idl:
https://w3c.github.io/requestidlecallback/#the-requestidlecallback-method (step 1)

* page/History.idl:
https://html.spec.whatwg.org/multipage/history.html#dom-history-go (step 1)
https://html.spec.whatwg.org/multipage/history.html#dom-history-back (step 1)
https://html.spec.whatwg.org/multipage/history.html#dom-history-forward (step 1)

* page/DOMWindow.cpp:
(WebCore::DOMWindow::setTimeout):
(WebCore::DOMWindow::setInterval):
* page/DOMWindow.h:
* workers/WorkerGlobalScope.cpp:
(WebCore::WorkerGlobalScope::setTimeout):
(WebCore::WorkerGlobalScope::setInterval):
* workers/WorkerGlobalScope.h:
Although condition #4 isn't satisfied for setTimeout() / setInterval() because
_current_ global object is used only for logging, replacing it with _relevant_
nicely cleans up method signatures.

* page/WindowOrWorkerGlobalScope.cpp:
(WebCore::WindowOrWorkerGlobalScope::structuredClone):
* page/WindowOrWorkerGlobalScope.h:
* page/WindowOrWorkerGlobalScope.idl:
https://html.spec.whatwg.org/multipage/webappapis.html#report-the-exception
https://html.spec.whatwg.org/multipage/structured-data.html#structured-cloning (step 2)


Canonical link: https://commits.webkit.org/245123@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@286895 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy pushed a commit that referenced this pull request May 26, 2022
…LInt asm.

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

Reviewed by Geoffrey Garen.

With this, debugging LLInt code will be easier.  LLInt code will no longer all be at an
offset from vmEntryToJavaScript.  They will instead be broken up into different sections
under human readable labels.

Secondly, crash traces of LLInt crashes will now be able to give us the nearest label
for a crash site, as opposed to everything being an offset from vmEntryToJavaScript.

For example, instead of this:

    JavaScriptCore`vmEntryToJavaScript:
        0x1026525b8 <+0>:   pacibsp
        0x1026525bc <+4>:   stp    x29, x30, [sp, #-0x10]!
        0x1026525c0 <+8>:   mov    x29, sp
        0x1026525c4 <+12>:  sub    sp, x29, #0xb0
        0x1026525c8 <+16>:  mov    x13, #0xc800
        0x1026525cc <+20>:  add    x17, x1, x13
        0x1026525d0 <+24>:  ldr    w4, [x17]
        0x1026525d4 <+28>:  cbnz   w4, 0x10265275c           ; vmEntryToJavaScriptGateAfter + 120
        0x1026525d8 <+32>:  str    x1, [sp]
        0x1026525dc <+36>:  mov    x17, #0x9e78
        0x1026525e0 <+40>:  add    x13, x1, x17
        0x1026525e4 <+44>:  ldr    x4, [x13]
        0x1026525e8 <+48>:  str    x4, [sp, #0x8]
        0x1026525ec <+52>:  mov    x13, #0x9e70
        0x1026525f0 <+56>:  add    x17, x1, x13
        0x1026525f4 <+60>:  ldr    x4, [x17]
        0x1026525f8 <+64>:  str    x4, [sp, #0x10]
        0x1026525fc <+68>:  ldr    x4, [x2, #0x8]
        0x102652600 <+72>:  str    x4, [sp, #0x18]
        0x102652604 <+76>:  ldr    w4, [x2, #0x20]
        0x102652608 <+80>:  add    x4, x4, #0x5
        0x10265260c <+84>:  lsl    x4, x4, #3
        0x102652610 <+88>:  sub    x3, sp, x4
        0x102652614 <+92>:  cmp    sp, x3
        0x102652618 <+96>:  b.ls   0x10265271c               ; vmEntryToJavaScriptGateAfter + 56
        0x10265261c <+100>: mov    x17, #0xca00
        0x102652620 <+104>: add    x13, x1, x17
        0x102652624 <+108>: ldr    x17, [x13]
        0x102652628 <+112>: cmp    x3, x17
        0x10265262c <+116>: b.lo   0x10265271c               ; vmEntryToJavaScriptGateAfter + 56
        0x102652630 <+120>: mov    sp, x3
        0x102652634 <+124>: mov    x3, #0x4
        0x102652638 <+128>: sub    w3, w3, #0x1
        0x10265263c <+132>: add    x17, x2, x3, lsl #3
        0x102652640 <+136>: ldr    x5, [x17]

We now get this:

    JavaScriptCore`vmEntryToJavaScript:
        0x1028b5d90 <+0>:   pacibsp
        0x1028b5d94 <+4>:   stp    x29, x30, [sp, #-0x10]!
        0x1028b5d98 <+8>:   mov    x29, sp
        0x1028b5d9c <+12>:  sub    sp, x29, #0xb0
        0x1028b5da0 <+16>:  mov    x13, #0xc800
        0x1028b5da4 <+20>:  add    x17, x1, x13
        0x1028b5da8 <+24>:  ldr    w4, [x17]
        0x1028b5dac <+28>:  cbnz   w4, 0x1028b5f34           ; _offlineasm_doVMEntry__checkVMEntryPermission
        0x1028b5db0 <+32>:  str    x1, [sp]
        0x1028b5db4 <+36>:  mov    x17, #0x9e78
        0x1028b5db8 <+40>:  add    x13, x1, x17
        0x1028b5dbc <+44>:  ldr    x4, [x13]
        0x1028b5dc0 <+48>:  str    x4, [sp, #0x8]
        0x1028b5dc4 <+52>:  mov    x13, #0x9e70
        0x1028b5dc8 <+56>:  add    x17, x1, x13
        0x1028b5dcc <+60>:  ldr    x4, [x17]
        0x1028b5dd0 <+64>:  str    x4, [sp, #0x10]
        0x1028b5dd4 <+68>:  ldr    x4, [x2, #0x8]
        0x1028b5dd8 <+72>:  str    x4, [sp, #0x18]
        0x1028b5ddc <+76>:  ldr    w4, [x2, #0x20]
        0x1028b5de0 <+80>:  add    x4, x4, #0x5
        0x1028b5de4 <+84>:  lsl    x4, x4, #3
        0x1028b5de8 <+88>:  sub    x3, sp, x4
        0x1028b5dec <+92>:  cmp    sp, x3
        0x1028b5df0 <+96>:  b.ls   0x1028b5ef4               ; _offlineasm_doVMEntry__throwStackOverflow
        0x1028b5df4 <+100>: mov    x17, #0xca00
        0x1028b5df8 <+104>: add    x13, x1, x17
        0x1028b5dfc <+108>: ldr    x17, [x13]
        0x1028b5e00 <+112>: cmp    x3, x17
        0x1028b5e04 <+116>: b.lo   0x1028b5ef4               ; _offlineasm_doVMEntry__throwStackOverflow

    JavaScriptCore`_offlineasm_doVMEntry__stackHeightOK:
        0x1028b5e08 <+0>:   mov    sp, x3
        0x1028b5e0c <+4>:   mov    x3, #0x4

    JavaScriptCore`_offlineasm_doVMEntry__copyHeaderLoop:
        0x1028b5e10 <+0>:   sub    w3, w3, #0x1
        0x1028b5e14 <+4>:   add    x17, x2, x3, lsl #3
        0x1028b5e18 <+8>:   ldr    x5, [x17]

This feature is only available when COMPILER(CLANG) is true.

* Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:

Canonical link: https://commits.webkit.org/250933@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294768 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy pushed a commit that referenced this pull request May 26, 2022
https://bugs.webkit.org/show_bug.cgi?id=240885

Reviewed by Geoffrey Garen.

Code generated by the offlineasm is tightly coupled with JSC anyway.  Might as well
make the local labels more informative about where the code came from in crash traces.

So, instead of this:

    JavaScriptCore`vmEntryToJavaScript:
        0x1028b5d90 <+0 >:   pacibsp
        0x1028b5d94 <+4 >:   stp    x29, x30, [sp, #-0x10]!
        0x1028b5d98 <+8 >:   mov    x29, sp
        0x1028b5d9c <+12 >:  sub    sp, x29, #0xb0
        0x1028b5da0 <+16 >:  mov    x13, #0xc800
        0x1028b5da4 <+20 >:  add    x17, x1, x13
        0x1028b5da8 <+24 >:  ldr    w4, [x17]
        0x1028b5dac <+28 >:  cbnz   w4, 0x1028b5f34           ; _offlineasm_doVMEntry__checkVMEntryPermission
        0x1028b5db0 <+32 >:  str    x1, [sp]
        0x1028b5db4 <+36 >:  mov    x17, #0x9e78
        ...
        0x1028b5de8 <+88 >:  sub    x3, sp, x4
        0x1028b5dec <+92 >:  cmp    sp, x3
        0x1028b5df0 <+96 >:  b.ls   0x1028b5ef4               ; _offlineasm_doVMEntry__throwStackOverflow
        0x1028b5df4 <+100 >: mov    x17, #0xca00
        0x1028b5df8 <+104 >: add    x13, x1, x17
        0x1028b5dfc <+108 >: ldr    x17, [x13]
        0x1028b5e00 <+112 >: cmp    x3, x17
        0x1028b5e04 <+116 >: b.lo   0x1028b5ef4               ; _offlineasm_doVMEntry__throwStackOverflow

    JavaScriptCore`_offlineasm_doVMEntry__stackHeightOK:
        0x1028b5e08 <+0 >:   mov    sp, x3
        0x1028b5e0c <+4 >:   mov    x3, #0x4

    JavaScriptCore`_offlineasm_doVMEntry__copyHeaderLoop:
        0x1028b5e10 <+0 >:   sub    w3, w3, #0x1
        0x1028b5e14 <+4 >:   add    x17, x2, x3, lsl #3
        0x1028b5e18 <+8 >:   ldr    x5, [x17]

We now get this:

    JavaScriptCore`vmEntryToJavaScript:
        0x1028cdd90 <+0>:   pacibsp
        0x1028cdd94 <+4>:   stp    x29, x30, [sp, #-0x10]!
        0x1028cdd98 <+8>:   mov    x29, sp
        0x1028cdd9c <+12>:  sub    sp, x29, #0xb0
        0x1028cdda0 <+16>:  mov    x13, #0xc800
        0x1028cdda4 <+20>:  add    x17, x1, x13
        0x1028cdda8 <+24>:  ldr    w4, [x17]
        0x1028cddac <+28>:  cbnz   w4, 0x1028cdf34           ; jsc_llint_doVMEntry__checkVMEntryPermission
        0x1028cddb0 <+32>:  str    x1, [sp]
        0x1028cddb4 <+36>:  mov    x17, #0x9e78
        ...
        0x1028cdde8 <+88>:  sub    x3, sp, x4
        0x1028cddec <+92>:  cmp    sp, x3
        0x1028cddf0 <+96>:  b.ls   0x1028cdef4               ; jsc_llint_doVMEntry__throwStackOverflow
        0x1028cddf4 <+100>: mov    x17, #0xca00
        0x1028cddf8 <+104>: add    x13, x1, x17
        0x1028cddfc <+108>: ldr    x17, [x13]
        0x1028cde00 <+112>: cmp    x3, x17
        0x1028cde04 <+116>: b.lo   0x1028cdef4               ; jsc_llint_doVMEntry__throwStackOverflow

    JavaScriptCore`jsc_llint_doVMEntry__stackHeightOK:
        0x1028cde08 <+0>:   mov    sp, x3
        0x1028cde0c <+4>:   mov    x3, #0x4

    JavaScriptCore`jsc_llint_doVMEntry__copyHeaderLoop:
        0x1028cde10 <+0>:   sub    w3, w3, #0x1
        0x1028cde14 <+4>:   add    x17, x2, x3, lsl #3
        0x1028cde18 <+8>:   ldr    x5, [x17]

* Source/JavaScriptCore/offlineasm/arm.rb:
* Source/JavaScriptCore/offlineasm/arm64.rb:
* Source/JavaScriptCore/offlineasm/backends.rb:
* Source/JavaScriptCore/offlineasm/config.rb:

Canonical link: https://commits.webkit.org/250946@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294787 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy pushed a commit that referenced this pull request Jul 8, 2022
https://bugs.webkit.org/show_bug.cgi?id=241905

Reviewed by Yusuke Suzuki.

offlineasm used to emit this LLInt code:
    ".loc 1 996\n"        "ldr x19, [x0] \n"                   // LowLevelInterpreter.asm:996
    ".loc 1 997\n"        "ldr x20, [x0, #8] \n"               // LowLevelInterpreter.asm:997
    ".loc 1 998\n"        "ldr x21, [x0, #16] \n"              // LowLevelInterpreter.asm:998
    ".loc 1 999\n"        "ldr x22, [x0, #24] \n"              // LowLevelInterpreter.asm:999
    ...
    ".loc 1 1006\n"       "ldr d8, [x0, WebKit#80] \n"               // LowLevelInterpreter.asm:1006
    ".loc 1 1007\n"       "ldr d9, [x0, WebKit#88] \n"               // LowLevelInterpreter.asm:1007
    ".loc 1 1008\n"       "ldr d10, [x0, WebKit#96] \n"              // LowLevelInterpreter.asm:1008
    ".loc 1 1009\n"       "ldr d11, [x0, WebKit#104] \n"             // LowLevelInterpreter.asm:1009
    ...

Now, it can emit this instead:
    ".loc 1 996\n"        "ldp x19, x20, [x0, #0] \n"          // LowLevelInterpreter.asm:996
    ".loc 1 997\n"        "ldp x21, x22, [x0, #16] \n"         // LowLevelInterpreter.asm:997
    ...
    ".loc 1 1001\n"       "ldp d8, d9, [x0, WebKit#80] \n"           // LowLevelInterpreter.asm:1001
    ".loc 1 1002\n"       "ldp d10, d11, [x0, WebKit#96] \n"         // LowLevelInterpreter.asm:1002
    ...

Also, there was some code that kept recomputing the base address of a sequence of load/store
instructions.  For example,
    ".loc 6 902\n"        "add x13, sp, x10, lsl #3 \n"        // WebAssembly.asm:902
                          "ldr x0, [x13, #48] \n"
                          "add x13, sp, x10, lsl #3 \n"
                          "ldr x1, [x13, #56] \n"
                          "add x13, sp, x10, lsl #3 \n"
                          "ldr x2, [x13, #64] \n"
                          "add x13, sp, x10, lsl #3 \n"
                          "ldr x3, [x13, WebKit#72] \n"
    ...

For such places, we observe that the base address is the same for every load/store instruction
in the sequence, and precompute it in the LLInt asm code to help out the offline asm.  This
allows the offlineasm to now emit this more efficient code instead:
    ".loc 6 896\n"        "add x10, sp, x10, lsl #3 \n"        // WebAssembly.asm:896
    ".loc 6 898\n"        "ldp x0, x1, [x10, #48] \n"          // WebAssembly.asm:898
                          "ldp x2, x3, [x10, #64] \n"
    ...

* Source/JavaScriptCore/llint/LowLevelInterpreter.asm:
* Source/JavaScriptCore/llint/WebAssembly.asm:
* Source/JavaScriptCore/offlineasm/arm64.rb:
* Source/JavaScriptCore/offlineasm/instructions.rb:

Canonical link: https://commits.webkit.org/251799@main
pulkomandy pushed a commit that referenced this pull request Jul 16, 2022
…us wrapper

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

Reviewed by Antti Koivisto.

When the anonymous block wrapper for an inline level child is not needed anymore (sibling block is removed or became non-inflow), we
1. detach the inline level child (and its subtree)
2. destroy the anonymous wrapper
3. re-attach the inline level child under the new parent (most likely the parent of the destroyed anonymous wrapper)

We call this re-parenting activity an "internal move".
Certain properties (e.g fragmentation state) are not supposed to change during this type of move (we simply stop calling some "reset" functions when RenderObject::IsInternalMove::Yes)

This patch ensures that the internal move flag is set for both #1 and #3.

* Source/WebCore/rendering/RenderBlockFlow.cpp: drive-by fix to ensure no ruby content gets multi-column context.
(WebCore::RenderBlockFlow::willCreateColumns const):
* Source/WebCore/rendering/updating/RenderTreeBuilder.cpp:
(WebCore::RenderTreeBuilder::removeAnonymousWrappersForInlineChildrenIfNeeded): Make sure both detach and attach are covered with the "internal move" flag as currently only the attach is covered. It means that whatever flags we reset at detach (not an internal move) we don't set back on attach (internal move).

Canonical link: https://commits.webkit.org/252456@main
pulkomandy pushed a commit that referenced this pull request Mar 5, 2023
https://bugs.webkit.org/show_bug.cgi?id=251063
rdar://104585575

Reviewed by Mark Lam and Justin Michaud.

This patch enhances CallFrame::dump to support wasm frames in btjs stacktrace.
The example is as follows.

    frame #0: 0x00000001035fca78 JavaScriptCore`JSC::functionBreakpoint(globalObject=0x000000012f410068, callFrame=0x000000016fdfa9d0) at JSDollarVM.cpp:2273:9 [opt]
    frame #1: 0x000000010ec44204 0x10eccc5dc
    frame #2: 0x000000010eccc5dc callback#Dwaxn6 [Baseline bc#50](Undefined)
    frame #3: 0x000000010ec4ca84 wasm-stub [WasmToJS](Wasm::Instance: 0x10d29da40)
    frame #4: 0x000000010ed0c060 <?>.wasm-function[1] [OMG](Wasm::Instance: 0x10d29da40)
    frame #5: 0x000000010ed100d0 jsToWasm#CWTx6k [FTL bc#22](Cell[JSModuleEnvironment]: 0x12f524540, Cell[WebAssemblyFunction]: 0x10d06a3a8, 1, 2, 3)
    frame #6: 0x000000010ec881b0 #D5ymZE [Baseline bc#733](Undefined, Cell[Generator]: 0x12f55c180, 1, Cell[Object]: 0x12f69dfc0, 0, Cell[JSLexicalEnvironment]: 0x12f52cee0)
    frame #7: 0x000000010ec3c008 asyncFunctionResume#A4ayYg [LLInt bc#49](Undefined, Cell[Generator]: 0x12f55c180, Cell[Object]: 0x12f69dfc0, 0)
    frame #8: 0x000000010ec3c008 promiseReactionJobWithoutPromise#D0yDF1 [LLInt bc#25](Undefined, Cell[Function]: 0x12f44f3c0, Cell[Object]: 0x12f69dfc0, Cell[Generator]: 0x12f55c180)
    frame #9: 0x000000010ec80ec0 promiseReactionJob#EdShZz [Baseline bc#74](Undefined, Undefined, Cell[Function]: 0x12f44f3c0, Cell[Object]: 0x12f69dfc0, Cell[Generator]: 0x12f55c180)
    frame #10: 0x000000010ec3c728
    frame #11: 0x0000000103137560 JavaScriptCore`JSC::Interpreter::executeCall(JSC::JSGlobalObject*, JSC::JSObject*, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) [inlined] JSC::JITCode::execute(this=<unavailable>, vm=<unavailable>, protoCallFrame=<unavailable>) at JITCodeInlines.h:42:38 [opt]
    frame #12: 0x0000000103137524 JavaScriptCore`JSC::Interpreter::executeCall(this=<unavailable>, lexicalGlobalObject=<unavailable>, function=<unavailable>, callData=<unavailable>, thisValue=<unavailable>, args=<unavailable>) at Interpreter.cpp:1093:27 [opt]
    frame #13: 0x000000010349d6d0 JavaScriptCore`JSC::runJSMicrotask(globalObject=0x000000012f410068, identifier=(m_identifier = 81), job=JSValue @ x22, argument0=JSValue @ x26, argument1=JSValue @ x25, argument2=<unavailable>, argument3=<unavailable>) at JSMicrotask.cpp:98:9 [opt]
    frame #14: 0x00000001039dfc54 JavaScriptCore`JSC::VM::drainMicrotasks() (.cold.1) at VM.cpp:0:9 [opt]
    frame #15: 0x00000001035e58a4 JavaScriptCore`JSC::VM::drainMicrotasks() [inlined] JSC::MicrotaskQueue::dequeue(this=<unavailable>) at VM.cpp:0:9 [opt]
    frame #16: 0x00000001035e5894 JavaScriptCore`JSC::VM::drainMicrotasks(this=0x000000012f000000) at VM.cpp:1255:46 [opt]
    ...

* Source/JavaScriptCore/interpreter/CallFrame.cpp:
(JSC::CallFrame::dump const):

Canonical link: https://commits.webkit.org/259262@main
pulkomandy pushed a commit that referenced this pull request Sep 2, 2024
…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
pulkomandy pushed a commit that referenced this pull request Sep 4, 2024
…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
pulkomandy pushed a commit that referenced this pull request Sep 28, 2024
…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, #4)

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

    BB#4
    PutByOffset(@0, 0, @z)
    Jump #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
pulkomandy pushed a commit that referenced this pull request Feb 16, 2025
…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
pulkomandy pushed a commit that referenced this pull request Mar 15, 2025
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 #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
pulkomandy pushed a commit that referenced this pull request Apr 3, 2025
…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
pulkomandy pushed a commit that referenced this pull request Apr 29, 2025
…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
kenmays referenced this pull request in kenmays/haikuwebkit May 18, 2025
…ngs'

rdar://151332172

Unreviewed build fix.

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

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

Successfully merging this pull request may close these issues.

2 participants