Fixed possible NULL pointer crash. #2
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fData->frame->document() can return NULL in certain cases, causing a crash.
|
Looks like it might be worthwhile to add a null check for (fData->frame) too? (can just add it in as |
|
Is this confirmed to fix any bugs? |
|
BWebFrame is always tied to a WebKit frame, so there is no need to check for that to be NULL, I think. |
|
@waddlesplash it is related to a patch I will submit later today. It calls BWePage::MainFrameURL which calls this URL method and could sometimes trigger the bug. |
pulkomandy
pushed a commit
that referenced
this pull request
Jan 1, 2018
…broken <https://webkit.org/b/158743> Unreviewed build fix. * DumpRenderTree/PlatformMac.cmake: Fix silly typo. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@202065 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Mar 4, 2018
…when emitting a PutHint of a materialized object into a PromotedHeapLocation of a still sunken object https://bugs.webkit.org/show_bug.cgi?id=168140 <rdar://problem/30205880> Reviewed by Filip Pizlo. JSTests: * stress/allocation-sinking-puthint-control-flow.js: Added. (e): (bar): (let.y): (else.let.y): (baz): (foo): (catch): Source/JavaScriptCore: This patch fixes a bug in allocation sinking phase where we don't properly handle control flow when materializing an object and also PutHinting that materialization into a still sunken object. We were performing the PutHint for the materialization at the point of materialization, however, we may have materialized along both edges of a control flow diamond, in which case, we need to also PutHint at the join point. Consider this program: ``` bb#0: b: PhantomActivation() a: PhantomNewFunction() c: PutHint(@A, @b, ActivationLoc) Branch(#1, #2) bb#1: d: MaterializeActivation() e: PutHint(@A, @d, ActivationLoc) f: Upsilon(@d, ^p) Jump(#3) bb#2: g: MaterializeActivation() h: PutHint(@A, @g, ActivationLoc) i: Upsilon(@d, ^p) Jump(#3) bb#3: p: Phi() // What is PromotedHeapLocation(@A, ActivationLoc) here? // What would we do if we exited? ``` Before this patch, we didn't perform a PutHint of the Phi. However, we need to, otherwise when exit, we won't know the value of PromotedHeapLocation(@A, ActivationLoc) The program we need then, for correctness, is this: ``` bb#0: b: PhantomActivation() a: PhantomNewFunction() c: PutHint(@A, @b, ActivationLoc) Branch(#1, #2) bb#1: d: MaterializeActivation() e: PutHint(@A, @d, ActivationLoc) f: Upsilon(@d, ^p) Jump(#3) bb#2: g: MaterializeActivation() h: PutHint(@A, @g, ActivationLoc) i: Upsilon(@d, ^p) Jump(#3) bb#3: p: Phi() j: PutHint(@A, @p, ActivationLoc) ``` This patch makes it so that we emit the necessary PutHint at node `j`. I've also added more validation to the OSRAvailabilityAnalysisPhase to catch this problem during validation. * dfg/DFGOSRAvailabilityAnalysisPhase.cpp: (JSC::DFG::OSRAvailabilityAnalysisPhase::run): * dfg/DFGObjectAllocationSinkingPhase.cpp: * ftl/FTLOperations.cpp: (JSC::FTL::operationMaterializeObjectInOSR): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@212177 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Mar 5, 2018
https://bugs.webkit.org/show_bug.cgi?id=168629 Reviewed by Filip Pizlo. This will make dumping FTL disassembly dump Air intermixed with the assembly generated by each Air Inst. This is similar to how dumpDFGDisassembly dumps the generated assembly for each Node. Here is what the output will look like: Generated FTL JIT code for foo#CUaFiQ:[0x10b76c960->0x10b76c2d0->0x10b7b6da0, FTLFunctionCall, 40 (NeverInline)], instruction count = 40: BB#0: ; frequency = 1.000000 0x469004e02e00: push %rbp 0x469004e02e01: mov %rsp, %rbp 0x469004e02e04: add $0xffffffffffffffd0, %rsp Move $0x10b76c960, %rax, $4487301472(@16) 0x469004e02e08: mov $0x10b76c960, %rax Move %rax, 16(%rbp), @19 0x469004e02e12: mov %rax, 0x10(%rbp) Patch &Patchpoint2, %rbp, %rax, @20 0x469004e02e16: lea -0x50(%rbp), %rax 0x469004e02e1a: mov $0x1084081e0, %r11 0x469004e02e24: cmp %rax, (%r11) 0x469004e02e27: ja 0x469004e02e9a Move 56(%rbp), %rdx, @23 0x469004e02e2d: mov 0x38(%rbp), %rdx Move $0xffff000000000002, %rax, $-281474976710654(@15) 0x469004e02e31: mov $0xffff000000000002, %rax Patch &BranchTest64(3,SameAsRep)1, NonZero, %rdx, %rax, %rdx, @26 0x469004e02e3b: test %rdx, %rax 0x469004e02e3e: jnz 0x469004e02f08 Move 48(%rbp), %rax, @29 0x469004e02e44: mov 0x30(%rbp), %rax Move %rax, %rcx, @31 0x469004e02e48: mov %rax, %rcx Xor64 $6, %rcx, @31 0x469004e02e4b: xor $0x6, %rcx Patch &BranchTest64(3,SameAsRep)1, NonZero, %rcx, $-2, %rax, @35 0x469004e02e4f: test $0xfffffffffffffffe, %rcx 0x469004e02e56: jnz 0x469004e02f12 Patch &Branch32(3,SameAsRep)0, NotEqual, (%rdx), $266, %rdx, @45 0x469004e02e5c: cmp $0x10a, (%rdx) 0x469004e02e62: jnz 0x469004e02f1c BranchTest32 NonZero, %rax, $1, @49 0x469004e02e68: test $0x1, %al 0x469004e02e6a: jnz 0x469004e02e91 Successors: #3, #1 BB#1: ; frequency = 1.000000 Predecessors: #0 Move $0, %rcx, @65 0x469004e02e70: xor %rcx, %rcx Jump @66 Successors: #2 BB#2: ; frequency = 1.000000 Predecessors: #1, #3 Move 24(%rdx), %rax, @58 0x469004e02e73: mov 0x18(%rdx), %rax Patch &BranchAdd32(4,ForceLateUseUnlessRecoverable)3, Overflow, %rcx, %rax, %rcx, %rcx, %rax, @60 0x469004e02e77: add %eax, %ecx 0x469004e02e79: jo 0x469004e02f26 Move $0xffff000000000000, %rax, $-281474976710656(@14) 0x469004e02e7f: mov $0xffff000000000000, %rax Add64 %rcx, %rax, %rax, @62 0x469004e02e89: add %rcx, %rax Ret64 %rax, @63 0x469004e02e8c: mov %rbp, %rsp 0x469004e02e8f: pop %rbp 0x469004e02e90: ret BB#3: ; frequency = 1.000000 Predecessors: #0 Move 16(%rdx), %rcx, @52 0x469004e02e91: mov 0x10(%rdx), %rcx Jump @55 0x469004e02e95: jmp 0x469004e02e73 Successors: #2 * CMakeLists.txt: * JavaScriptCore.xcodeproj/project.pbxproj: * b3/air/AirCode.h: (JSC::B3::Air::Code::setDisassembler): (JSC::B3::Air::Code::disassembler): * b3/air/AirDisassembler.cpp: Added. (JSC::B3::Air::Disassembler::startEntrypoint): (JSC::B3::Air::Disassembler::endEntrypoint): (JSC::B3::Air::Disassembler::startLatePath): (JSC::B3::Air::Disassembler::endLatePath): (JSC::B3::Air::Disassembler::startBlock): (JSC::B3::Air::Disassembler::addInst): (JSC::B3::Air::Disassembler::dump): * b3/air/AirDisassembler.h: Added. * b3/air/AirGenerate.cpp: (JSC::B3::Air::generate): * ftl/FTLCompile.cpp: (JSC::FTL::compile): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@212775 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Apr 7, 2018
…cs should not be required to perform font loading correctly. https://bugs.webkit.org/show_bug.cgi?id=168487 Reviewed by Antti Koivisto. Source/WebCore: There are three ways a Web author can chain multiple font files together: 1. Multiple entries in the "src" descriptor in an @font-face rule 2. Multiple @font-face rules with the same "font-family" descriptor 3. Multiple entries in the "font-family" property on an element Before r212513, the code which iterated across #2 and #3 above could have triggered each item in the chain to download. r212513 tried to solve this by using LastResort as the interstitial font used during downloads, because LastResort supports every character and therefore solves #3 above. However, this change had a few problems: 1. Previously, our code would try to avoid using the interstitial font for layout or rendering whenever possible (because one of the chains above may have named a local font which would be better to use). In order to use the benefits of LastResort, I had to remove this avoidance logic and make WebKit try to use the interstitial font as often as possible. However, due to the large metrics of LastResort, this means that offsetWidth queries during font loading would be wildly inaccurate, causing Google Docs to break. 2. It also means that canvas drawing during font loading would actually draw LastResort, causing Bing maps to break. 3. LastResort is platform-specific, so only platforms which have it would actually be able to load fonts correctly. Instead, we should keep the older logic about avoiding using the interstitial font so that loading has a better experience for the user. We solve the unnecessary download problem by giving our loading code a downloading policy enum, which has two values: allow downloads or forbid downloads. Whenever our loading code returns the interstitial font, we continue our search, but we change the policy to forbid downloads. There is one piece of subtlety, though: It is more common for web authors to put good fallbacks in the "font-family" property than in the "src" descriptor inside @font-face. This means that we shouldn't exhaustively search through the @font-face src list first. Instead, we should look through the src list until we hit a non-local font, and then immediately start looking through the other other chains. Tests: fast/text/font-download-font-face-src-list.html fast/text/font-download-font-family-property.html fast/text/font-download-remote-fallback-all.html fast/text/font-interstitial-invisible-width-while-loading.html fast/text/font-weight-download-3.html fast/text/web-font-load-fallback-during-loading-2.html fast/text/web-font-load-invisible-during-loading.html * css/CSSFontFace.cpp: (WebCore::CSSFontFace::fontLoadEventOccurred): Implement support for the font download policy. (WebCore::CSSFontFace::setStatus): After 3 seconds of loading, we will start drawing the fallback font. However, for testing, we have an internal setting to make this switch happen immediately. This patch now requires that this internal switch happen synchronously. (WebCore::CSSFontFace::pump): Implement support for the font download policy. (WebCore::CSSFontFace::load): Ditto. (WebCore::CSSFontFace::font): Ditto. * css/CSSFontFace.h: Ditto. * css/CSSFontSelector.cpp: (WebCore::CSSFontSelector::beginLoadingFontSoon): Implement support for synchronous font download timeouts. * css/CSSSegmentedFontFace.cpp: (WebCore::CSSSegmentedFontFace::fontRanges): Implement support for the font download policy. * platform/graphics/Font.cpp: Add new flag which represents if the interstitial font was created after the 3 second timeout or before. Previously, we would distinguish between these two cases by knowing that one font was LastResort and the other font was a fallback. Now that we're using fallback fonts on both sides of the 3 second timeout, we now no longer know which one should be invisible. This new enum solves this problem. (WebCore::Font::Font): (WebCore::Font::verticalRightOrientationFont): (WebCore::Font::uprightOrientationFont): * platform/graphics/Font.h: Ditto. (WebCore::Font::create): (WebCore::Font::origin): (WebCore::Font::visibility): * platform/graphics/FontCache.h: * platform/graphics/FontCascade.cpp: We try to fall back to a local() font during downloads, but there might not be one that we can use. Therefore, we can't use the presence of the interstitial font to detect if we should paint invisibly. Instead, we can move this logic into the font-specific part of painting, and consult with the specific font to know if it was created from a timed-out @font-face rule or not. (WebCore::FontCascade::drawText): (WebCore::shouldDrawIfLoading): (WebCore::FontCascade::drawGlyphBuffer): (WebCore::FontCascade::drawEmphasisMarks): * platform/graphics/FontCascade.h: * platform/graphics/FontCascadeFonts.cpp: (WebCore::FontCascadeFonts::glyphDataForVariant): Implement the logic described above where we switch the policy if we encounter the intestitial font. (WebCore::FontCascadeFonts::glyphDataForNormalVariant): Ditto. (WebCore::glyphPageFromFontRanges): Ditto. * platform/graphics/FontRanges.cpp: Implement support for the font download policy. (WebCore::FontRanges::Range::font): (WebCore::FontRanges::glyphDataForCharacter): (WebCore::FontRanges::fontForCharacter): (WebCore::FontRanges::fontForFirstRange): * platform/graphics/FontRanges.h: * platform/graphics/FontSelector.h: * platform/graphics/freetype/FontCacheFreeType.cpp: (WebCore::FontCache::lastResortFallbackFontForEveryCharacter): Deleted. * platform/graphics/mac/FontCacheMac.mm: (WebCore::FontCache::lastResortFallbackFontForEveryCharacter): Deleted. * platform/graphics/win/FontCacheWin.cpp: (WebCore::FontCache::lastResortFallbackFontForEveryCharacter): Deleted. LayoutTests: * fast/text/font-download-font-face-src-list-expected.txt: Added. * fast/text/font-download-font-face-src-list.html: Copied from LayoutTests/fast/text/font-weight-download-2.html. * fast/text/font-download-font-family-property-expected.txt: Added. * fast/text/font-download-font-family-property.html: Copied from LayoutTests/fast/text/font-weight-download-2.html. * fast/text/font-download-remote-fallback-all-expected.txt: Added. * fast/text/font-download-remote-fallback-all.html: Copied from LayoutTests/fast/text/font-weight-download-2.html. * fast/text/font-interstitial-invisible-width-while-loading-expected.txt: Added. * fast/text/font-interstitial-invisible-width-while-loading.html: Added. * fast/text/font-weight-download-2.html: * fast/text/font-weight-download-3-expected.txt: Added. * fast/text/font-weight-download-3.html: Copied from LayoutTests/fast/text/font-weight-download-2.html. * fast/text/web-font-load-fallback-during-loading-2-expected.html: Added. * fast/text/web-font-load-fallback-during-loading-2.html: Added. * fast/text/web-font-load-fallback-during-loading-expected.html: * fast/text/web-font-load-fallback-during-loading.html: * fast/text/web-font-load-invisible-during-loading-expected.txt: Added. * fast/text/web-font-load-invisible-during-loading.html: Added. * http/tests/webfont/fallback-font-while-loading-expected.txt: * http/tests/webfont/fallback-font-while-loading.html: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@216944 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Apr 7, 2018
https://bugs.webkit.org/show_bug.cgi?id=172288 Patch by Romain Bellessort <romain.bellessort@crf.canon.fr> on 2017-05-23 Reviewed by Chris Dumez. Two changes are implemented in this patch: - Change #1: An issue was reported to GH [1] while working on respondInClosedState implementation. This issue has now been fixed, and this patch aligns implementation with spec [2]. - Change #2: In addition, this patch also fixes a bug that went unnoticed as code is not yet reachable (usage of controller.@reader is not valid and is therefore replaced by controller.@controlledReadableStream.@reader). [1] whatwg/streams#686 [2] https://streams.spec.whatwg.org/#readable-byte-stream-controller-respond-in-closed-state No added test as: - Change #1 does not change behavior; - Change #2 is not testable as the code is not yet reachable. * Modules/streams/ReadableByteStreamInternals.js: (readableByteStreamControllerRespondInClosedState): Aligned with spec. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@217279 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Apr 8, 2018
…64 bit platforms https://bugs.webkit.org/show_bug.cgi?id=172957 <rdar://problem/32602704> Reviewed by Filip Pizlo. JSTests: * stress/spec-empty-flows-through-cell-checks.js: Added. (A): (B): (i.catch): Source/JavaScriptCore: Consider this program: ``` block#1: n: GetClosureVar(..., |this|) // this will load empty JSValue() SetLocal(Cell:@n, locFoo) // Cell check succeeds because JSValue() looks like a cell Branch(#2, #3) Block#3: x: GetLocal(locFoo) y: CheckNotEmpty(@x) ``` If we claim that a cell check filters out the empty value, we will incorrectly eliminate the CheckNotEmpty node @y. This patch fixes AI, FTLLowerDFGToB3, and DFGSpeculativeJIT to no longer make this claim. On 64 bit platforms: - Cell use kind *now allows* the empty value to pass through. - CellOrOther use kind *now allows* for the empty value to pass through - NotCell use kind *no longer allows* the empty value to pass through. * assembler/CPU.h: (JSC::isARMv7IDIVSupported): (JSC::isARM64): (JSC::isX86): (JSC::isX86_64): (JSC::is64Bit): (JSC::is32Bit): (JSC::isMIPS): Make these functions constexpr so we can use them in static variable assignment. * bytecode/SpeculatedType.h: * dfg/DFGSpeculativeJIT.cpp: (JSC::DFG::SpeculativeJIT::compileValueToInt32): (JSC::DFG::SpeculativeJIT::compileDoubleRep): (JSC::DFG::SpeculativeJIT::compileLogicalNotStringOrOther): (JSC::DFG::SpeculativeJIT::emitStringOrOtherBranch): (JSC::DFG::SpeculativeJIT::speculateCell): (JSC::DFG::SpeculativeJIT::speculateCellOrOther): (JSC::DFG::SpeculativeJIT::speculateObjectOrOther): (JSC::DFG::SpeculativeJIT::speculateString): (JSC::DFG::SpeculativeJIT::speculateStringOrOther): (JSC::DFG::SpeculativeJIT::speculateSymbol): (JSC::DFG::SpeculativeJIT::speculateNotCell): * dfg/DFGSpeculativeJIT32_64.cpp: * dfg/DFGSpeculativeJIT64.cpp: (JSC::DFG::SpeculativeJIT::fillSpeculateCell): (JSC::DFG::SpeculativeJIT::compileObjectToObjectOrOtherEquality): (JSC::DFG::SpeculativeJIT::compilePeepHoleObjectToObjectOrOtherEquality): (JSC::DFG::SpeculativeJIT::compileObjectOrOtherLogicalNot): (JSC::DFG::SpeculativeJIT::emitObjectOrOtherBranch): * dfg/DFGUseKind.h: (JSC::DFG::typeFilterFor): * ftl/FTLLowerDFGToB3.cpp: (JSC::FTL::DFG::LowerDFGToB3::compileDoubleRep): (JSC::FTL::DFG::LowerDFGToB3::numberOrNotCellToInt32): (JSC::FTL::DFG::LowerDFGToB3::compareEqObjectOrOtherToObject): (JSC::FTL::DFG::LowerDFGToB3::boolify): (JSC::FTL::DFG::LowerDFGToB3::equalNullOrUndefined): (JSC::FTL::DFG::LowerDFGToB3::lowCell): (JSC::FTL::DFG::LowerDFGToB3::lowNotCell): (JSC::FTL::DFG::LowerDFGToB3::isCellOrMisc): (JSC::FTL::DFG::LowerDFGToB3::isNotCellOrMisc): (JSC::FTL::DFG::LowerDFGToB3::isNotCell): (JSC::FTL::DFG::LowerDFGToB3::isCell): (JSC::FTL::DFG::LowerDFGToB3::speculateCellOrOther): (JSC::FTL::DFG::LowerDFGToB3::speculateObjectOrOther): (JSC::FTL::DFG::LowerDFGToB3::speculateString): (JSC::FTL::DFG::LowerDFGToB3::speculateStringOrOther): (JSC::FTL::DFG::LowerDFGToB3::speculateSymbol): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@218137 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Apr 17, 2018
* WebCoreSupport/WebPlatformStrategies.cpp: (WebPlatformStrategies::cookieRequestHeaderFieldValue): Correct return type and arguments to match new API. * WebCoreSupport/WebPlatformStrategies.h: Update signatures. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@221289 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Jun 18, 2018
…bexpressions has a weird fixpoint requirement https://bugs.webkit.org/show_bug.cgi?id=180783 Reviewed by Saam Barati. This fixes the regression by fixpointing CSE. We need to fixpoint CSE because of this case: BB#1: a: Load(@x) b: Load(@x) c: Load(@b) BB#2: d: Load(@b) BB#3: e: Load(@b) Lets assume that #3 loops around to #2, so to eliminate @d, we need to prove that it's redundant with both @c and @e. The problem is that by the time we get to @d, the CSE state will look like this: BB#1: a: Load(@x) b: Load(@x) c: Load(@A) memoryAtTail: {@x=>@A, @A=>@c} BB#2: d: Load(@A) [sic] memoryAtTail: {@b=>@d} BB#3: e: Load(@b) memoryAtTail: {@b=>@e} [sic] Note that #3's atTail map is keyed on @b, which was the old (no longer canonical) version of @A. But @d's children were already substituted, so it refers to @A. Since @A is not in #3's atTail map, we don't find it and leave the redundancy. I think that the cleanest solution is to fixpoint. CSE is pretty cheap, so hopefully we can afford this. It fixes the richards regression, since richards is super dependent on B3 CSE. * b3/B3EliminateCommonSubexpressions.cpp: Logging. * b3/B3Generate.cpp: (JSC::B3::generateToAir): Fix the bug. * b3/air/AirReportUsedRegisters.cpp: (JSC::B3::Air::reportUsedRegisters): Logging. * dfg/DFGByteCodeParser.cpp: * dfg/DFGSSAConversionPhase.cpp: (JSC::DFG::SSAConversionPhase::run): Don't generate EntrySwitch if we don't need it (makes IR easier to read). * ftl/FTLLowerDFGToB3.cpp: (JSC::FTL::DFG::LowerDFGToB3::lower): Don't generate EntrySwitch if we don't need it (makes IR easier to read). git-svn-id: http://svn.webkit.org/repository/webkit/trunk@225893 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Jun 23, 2018
…y promise rejected. https://bugs.webkit.org/show_bug.cgi?id=180781 Reviewed by Eric Carlson. Source/WebCore: Test: media/video-pause-play-resolve.html When scheduling a rejection or resolution of existing play promises, move() the existing promises into the block. This ensures that valid promises aren't added to the play promise vector between when a rejection is scheduled and when it runs. Drive-by fix: Don't return false from playInternal() just so the newly created promise will get rejected. The pause() command will reject the promise, so just make sure it's added to the m_pendingPlayPromises before calling playInternal(). Drive-by fix #2: The spec referenced by playInternal() and pauseInternal() doesn't say to call the "Media Element Load Algorithm" (i.e., prepareForLoad()); it says to call the "Resource Selection Algorithm" (i.e., selectMediaResource()). But fixing this bug caused an assertion crash when the resource selection task was fired and m_player was null. This was because the algorithm is being run at stop() time due to stop() calling pause(). The solution to this ASSERT is to stop the m_resourceSelectionTaskQueue in stop(). * html/HTMLMediaElement.cpp: (WebCore::HTMLMediaElement::scheduleRejectPendingPlayPromises): (WebCore::HTMLMediaElement::rejectPendingPlayPromises): (WebCore::HTMLMediaElement::resolvePendingPlayPromises): (WebCore::HTMLMediaElement::scheduleNotifyAboutPlaying): (WebCore::HTMLMediaElement::notifyAboutPlaying): (WebCore::HTMLMediaElement::noneSupported): (WebCore::HTMLMediaElement::cancelPendingEventsAndCallbacks): (WebCore::HTMLMediaElement::play): (WebCore::HTMLMediaElement::playInternal): (WebCore::HTMLMediaElement::pauseInternal): (WebCore::HTMLMediaElement::stop): * html/HTMLMediaElement.h: LayoutTests: * media/audio-dealloc-crash.html: * media/video-pause-play-resolve-expected.txt: Added. * media/video-pause-play-resolve.html: Added. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@226059 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Jun 23, 2018
…bKit projects <https://webkit.org/b/181256> <rdar://problem/36281730> * Plugins/WebNetscapePluginEventHandlerCocoa.mm: (WebNetscapePluginEventHandlerCocoa::handleTSMEvent): - Use reinterpret_cast<NPNSString*>(const_cast<CFMutableStringRef>()) to avoid warnings from casting CFStringRef to NPNSString*. Note that CFMutableStringRef is the same as CFStringRef without the const modifier, hence its use in the const_cast<> above. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@226503 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Oct 20, 2018
https://bugs.webkit.org/show_bug.cgi?id=187731 <rdar://problem/42290703> Reviewed by Brady Eidson. Source/WebCore: Test: http/tests/security/canvas-remote-read-remote-video-hls.html Most media sources are single-resource; they are accessed from a single origin. HLS manifests can contain many subresources from arbitrary origins, and canvases should be tainted when painted from media elements whose subresources were retrieved from tainting origins. Add a new method to HTMLMediaElement, wouldTaintOrigin(), taking a SecurityOrigin, and returning whether the media element would taint that origin. This gets piped all the way down to MediaPlayerPrivateAVFoundationObjC which uses WebCoreNSURLSession to track all the origins of all the responses which resulted from the media element's load. Drive-by fix: also fix this issue for media elements which render to an AudioContext. Drive-by fix #2: CanvasRenderingContext2DBase::createPattern() needs to check the return value of ImageBuffer::create() before using it. * Modules/webaudio/MediaElementAudioSourceNode.cpp: (WebCore::MediaElementAudioSourceNode::wouldTaintOrigin): * html/HTMLMediaElement.cpp: (WebCore::HTMLMediaElement::didAttachRenderers): (WebCore::HTMLMediaElement::didDetachRenderers): (WebCore::HTMLMediaElement::scheduleUpdateShouldAutoplay): * html/HTMLMediaElement.h: (WebCore::HTMLMediaElement::wouldTaintOrigin const): * html/canvas/CanvasRenderingContext.cpp: (WebCore::CanvasRenderingContext::wouldTaintOrigin): * html/canvas/CanvasRenderingContext2DBase.cpp: (WebCore::CanvasRenderingContext2DBase::createPattern): * platform/graphics/MediaPlayer.cpp: (WebCore::MediaPlayer::wouldTaintOrigin const): * platform/graphics/MediaPlayer.h: * platform/graphics/MediaPlayerPrivate.h: (WebCore::MediaPlayerPrivateInterface::hasSingleSecurityOrigin const): (WebCore::MediaPlayerPrivateInterface::wouldTaintOrigin const): * platform/graphics/avfoundation/objc/CDMSessionAVContentKeySession.mm: (WebCore::CDMSessionAVContentKeySession::update): * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h: * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm: (WebCore::MediaPlayerPrivateAVFoundationObjC::wouldTaintOrigin const): * platform/network/cocoa/WebCoreNSURLSession.h: * platform/network/cocoa/WebCoreNSURLSession.mm: (-[WebCoreNSURLSession task:didReceiveResponseFromOrigin:]): (-[WebCoreNSURLSession wouldTaintOrigin:]): (-[WebCoreNSURLSessionDataTask resource:receivedResponse:]): LayoutTests: * http/tests/media/resources/hls/test-vod-localhost.m3u8: Added. * http/tests/security/canvas-remote-read-remote-video-hls-expected.txt: Added. * http/tests/security/canvas-remote-read-remote-video-hls.html: Added. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@234055 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Oct 20, 2018
…iled on Windows Python https://bugs.webkit.org/show_bug.cgi?id=187581 Reviewed by Daniel Bates. There are two failures in this test case: 1. proc.poll() doesn't return 0. 2. stderr is not output. For failure #1, this is expected. the process should not exit at the time. proc.poll() should return None because the process is still alive. This change added a new test to check proc.poll() becomes 0 after the process successfully exits. For failure #2, stderr is not flushed even though stdout is flushed. This change uses '-u' command switch to force stdin, stdout and stderr to be totally unbuffered. * Scripts/webkitpy/port/server_process_unittest.py: (TestServerProcess.test_basic): Added -u command switch. Do not flush stdout. Removed the special condition for Windows. Add a new test to check proc.poll() returns 0. (TestServerProcess.test_process_crashing): Added -u command switch. Do not flush stdout. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@234130 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Nov 2, 2018
https://bugs.webkit.org/show_bug.cgi?id=189939 <rdar://problem/44407030> Reviewed by Michael Saboff. JSTests: * stress/ftl-should-always-filter-for-low-type-check-functions.js: Added. (foo): (test): Source/JavaScriptCore: For example, the FTL may know more about data flow than AI in certain programs, and it needs to inform AI of these data flow properties to appease the assertion we have in AI that a node must perform type checks on its child nodes. For example, consider this program: ``` bb#1 a: Phi // Let's say it has an Int32 result, so it goes into the int32 hash table in FTLLower Branch(..., #2, #3) bb#2 ArrayifyToStructure(Cell:@A) // This modifies @A to have the its previous type union the type of some structure set. Jump(#3) bb#3 c: Add(Int32:@something, Int32:@A) ``` When the Add node does lowInt32() for @A, FTL lower used to just grab it from the int32 hash table without filtering the AbstractValue. However, the parent node is asking for a type check to happen, so we must inform AI of this "type check" if we want to appease the assertion that all nodes perform type checks for their edges that semantically perform type checks. This patch makes it so we filter the AbstractValue in the lowXYZ even if FTLLower proved the value must be XYZ. * ftl/FTLLowerDFGToB3.cpp: (JSC::FTL::DFG::LowerDFGToB3::compilePhi): (JSC::FTL::DFG::LowerDFGToB3::simulatedTypeCheck): (JSC::FTL::DFG::LowerDFGToB3::lowInt32): (JSC::FTL::DFG::LowerDFGToB3::lowCell): (JSC::FTL::DFG::LowerDFGToB3::lowBoolean): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@236824 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Dec 5, 2018
…llFrame. https://bugs.webkit.org/show_bug.cgi?id=191579 <rdar://problem/45942472> Reviewed by Saam Barati. JSTests: * stress/regress-191579.js: Added. Source/JavaScriptCore: Both of these functions do a lot of work. It would be good for the topCallFrame to be correct should we need to throw an exception. For example, we've observed the following crash trace: * frame #0: WTFCrash() at Assertions.cpp:253 frame #1: ... frame #2: JSC::StructureIDTable::get(this=0x00006040000162f0, structureID=1874583248) at StructureIDTable.h:129 frame #3: JSC::VM::getStructure(this=0x0000604000016210, id=4022066896) at VM.h:705 frame #4: JSC::JSCell::structure(this=0x00007ffeefbbde30, vm=0x0000604000016210) const at JSCellInlines.h:125 frame #5: JSC::JSCell::classInfo(this=0x00007ffeefbbde30, vm=0x0000604000016210) const at JSCellInlines.h:335 frame #6: JSC::JSCell::inherits(this=0x00007ffeefbbde30, vm=0x0000604000016210, info=0x0000000105eaf020) const at JSCellInlines.h:302 frame #7: JSC::JSObject* JSC::jsCast<JSC::JSObject*, JSC::JSCell>(from=0x00007ffeefbbde30) at JSCast.h:36 frame #8: JSC::asObject(cell=0x00007ffeefbbde30) at JSObject.h:1299 frame #9: JSC::asObject(value=JSValue @ 0x00007ffeefbba380) at JSObject.h:1304 frame #10: JSC::Register::object(this=0x00007ffeefbbdd58) const at JSObject.h:1514 frame #11: JSC::ExecState::jsCallee(this=0x00007ffeefbbdd40) const at CallFrame.h:107 frame #12: JSC::ExecState::isStackOverflowFrame(this=0x00007ffeefbbdd40) const at CallFrameInlines.h:36 frame #13: JSC::StackVisitor::StackVisitor(this=0x00007ffeefbba860, startFrame=0x00007ffeefbbdd40, vm=0x0000631000000800) at StackVisitor.cpp:52 frame #14: JSC::StackVisitor::StackVisitor(this=0x00007ffeefbba860, startFrame=0x00007ffeefbbdd40, vm=0x0000631000000800) at StackVisitor.cpp:41 frame #15: void JSC::StackVisitor::visit<(JSC::StackVisitor::EmptyEntryFrameAction)0, JSC::Interpreter::getStackTrace(JSC::JSCell*, WTF::Vector<JSC::StackFrame, 0ul, WTF::CrashOnOverflow, 16ul>&, unsigned long, unsigned long)::$_3>(startFrame=0x00007ffeefbbdd40, vm=0x0000631000000800, functor=0x00007ffeefbbaa60)::$_3 const&) at StackVisitor.h:147 frame #16: JSC::Interpreter::getStackTrace(this=0x0000602000005db0, owner=0x000062d00020cbe0, results=0x00006020000249d0, framesToSkip=0, maxStackSize=1) at Interpreter.cpp:437 frame #17: JSC::getStackTrace(exec=0x000062d00002c048, vm=0x0000631000000800, obj=0x000062d00020cbe0, useCurrentFrame=true) at Error.cpp:170 frame #18: JSC::ErrorInstance::finishCreation(this=0x000062d00020cbe0, exec=0x000062d00002c048, vm=0x0000631000000800, message=0x00007ffeefbbb800, useCurrentFrame=true) at ErrorInstance.cpp:119 frame #19: JSC::ErrorInstance::create(exec=0x000062d00002c048, vm=0x0000631000000800, structure=0x000062d0000f5730, message=0x00007ffeefbbb800, appender=0x0000000000000000, type=TypeNothing, useCurrentFrame=true)(WTF::String const&, WTF::String const&, JSC::RuntimeType, JSC::ErrorInstance::SourceTextWhereErrorOccurred), JSC::RuntimeType, bool) at ErrorInstance.h:49 frame #20: JSC::createRangeError(exec=0x000062d00002c048, globalObject=0x000062d00002c000, message=0x00007ffeefbbb800, appender=0x0000000000000000)(WTF::String const&, WTF::String const&, JSC::RuntimeType, JSC::ErrorInstance::SourceTextWhereErrorOccurred)) at Error.cpp:68 frame #21: JSC::createRangeError(exec=0x000062d00002c048, globalObject=0x000062d00002c000, message=0x00007ffeefbbb800) at Error.cpp:316 frame #22: JSC::createStackOverflowError(exec=0x000062d00002c048, globalObject=0x000062d00002c000) at ExceptionHelpers.cpp:77 frame #23: JSC::createStackOverflowError(exec=0x000062d00002c048) at ExceptionHelpers.cpp:72 frame #24: JSC::throwStackOverflowError(exec=0x000062d00002c048, scope=0x00007ffeefbbbaa0) at ExceptionHelpers.cpp:335 frame #25: JSC::ProxyObject::getOwnPropertySlotCommon(this=0x000062d000200e40, exec=0x000062d00002c048, propertyName=PropertyName @ 0x00007ffeefbbba80, slot=0x00007ffeefbbc720) at ProxyObject.cpp:372 frame #26: JSC::ProxyObject::getOwnPropertySlot(object=0x000062d000200e40, exec=0x000062d00002c048, propertyName=PropertyName @ 0x00007ffeefbbbd40, slot=0x00007ffeefbbc720) at ProxyObject.cpp:395 frame #27: JSC::JSObject::getNonIndexPropertySlot(this=0x000062d000200e40, exec=0x000062d00002c048, propertyName=PropertyName @ 0x00007ffeefbbbea0, slot=0x00007ffeefbbc720) at JSObjectInlines.h:150 frame #28: bool JSC::JSObject::getPropertySlot<false>(this=0x000062d000200e40, exec=0x000062d00002c048, propertyName=PropertyName @ 0x00007ffeefbbc320, slot=0x00007ffeefbbc720) at JSObject.h:1424 frame #29: JSC::JSObject::calculatedClassName(object=0x000062d000200e40) at JSObject.cpp:535 frame #30: JSC::Structure::toStructureShape(this=0x000062d000007410, value=JSValue @ 0x00007ffeefbbcae0, sawPolyProtoStructure=0x00007ffeefbbcf60) at Structure.cpp:1142 frame #31: JSC::TypeProfilerLog::processLogEntries(this=0x000060400000a950, reason=0x00007ffeefbbd5c0) at TypeProfilerLog.cpp:89 frame #32: JSC::JIT::doMainThreadPreparationBeforeCompile(this=0x0000619000034da0) at JIT.cpp:951 frame #33: JSC::JITWorklist::Plan::Plan(this=0x0000619000034d80, codeBlock=0x000062d0001d88c0, loopOSREntryBytecodeOffset=0) at JITWorklist.cpp:43 frame #34: JSC::JITWorklist::Plan::Plan(this=0x0000619000034d80, codeBlock=0x000062d0001d88c0, loopOSREntryBytecodeOffset=0) at JITWorklist.cpp:42 frame #35: JSC::JITWorklist::compileLater(this=0x0000616000001b80, codeBlock=0x000062d0001d88c0, loopOSREntryBytecodeOffset=0) at JITWorklist.cpp:256 frame #36: JSC::LLInt::jitCompileAndSetHeuristics(codeBlock=0x000062d0001d88c0, exec=0x00007ffeefbbde30, loopOSREntryBytecodeOffset=0) at LLIntSlowPaths.cpp:391 frame #37: llint_replace(exec=0x00007ffeefbbde30, pc=0x00006040000161ba) at LLIntSlowPaths.cpp:516 frame #38: llint_entry at LowLevelInterpreter64.asm:98 frame #39: vmEntryToJavaScript at LowLevelInterpreter64.asm:296 ... This crash occurred because StackVisitor was seeing an invalid topCallFrame while trying to capture the Error stack while throwing a StackOverflowError below llint_replace. While in this specific example, it is questionable whether we should be executing JS code below TypeProfilerLog::processLogEntries(), it is correct to have set the topCallFrame in llint_replace. We do this by calling LLINT_BEGIN_NO_SET_PC() at the top of llint_replace. We also do the same for llint_osr. Note: both of these LLInt slow path functions are called with a fully initialized CallFrame. Hence, there's no issue with setting topCallFrame to their CallFrames for these functions. * llint/LLIntSlowPaths.cpp: (JSC::LLInt::LLINT_SLOW_PATH_DECL): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@238141 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Dec 6, 2018
…amiliar architectures https://bugs.webkit.org/show_bug.cgi?id=186722 Reviewed by Žan Doberšek. Time for part #2! This change was defeated for GTK and WPE by the code that makes the options public. We have three options: (a) duplicate the architecture check currently in WebKitFeatures.cmake in both OptionsGTK.cmake and OptionsWPE.cmake, (b) rely on the result of that check in OptionsGTK.cmake and OptionsWPE.cmake by using ENABLE_JIT_DEFAULT and USE_SYSTEM_MALLOC_DEFAULT, a fragile encapsulation violation, or (c) just make the options private. They have been public up until now because they needed to be turned off on unsupported architectures. But now they are off by default and enabled only for particular whitelisted architectures, so they shouldn't be needed anymore. Note we have to hide ENABLE_SAMPLING_PROFILER as well, since it needs to match the value of ENABLE_JIT. Again, this is handled properly in WebKitFeatures.cmake, and defeated here in OptionsGTK.cmake. (This is not a problem for WPE.) * Source/cmake/OptionsGTK.cmake: * Source/cmake/OptionsWPE.cmake: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@238619 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Dec 6, 2018
https://bugs.webkit.org/show_bug.cgi?id=192236 <rdar://problem/45792118> Reviewed by Geoffrey Garen. Source/WebCore: This patch introduces asynchronous content change observation. 1. Start observing synchronous content change and timer install as the result of dispatching mouseMoved event. 2. Start observing synchronous content change and style recalc schedule as the result of a timer callback (installed at #1). 3. Start observing synchronous content change as the result of a style recalc (scheduled at #2). This patch also extends the timeout value from 100ms to 250ms. Certain content prefer longer timeouts (see http://briancherne.github.io/jquery-hoverIntent/ for details). Test: fast/events/touch/ios/hover-when-style-change-is-async.html * dom/Document.cpp: (WebCore::Document::scheduleStyleRecalc): (WebCore::Document::updateStyleIfNeeded): * page/DOMTimer.cpp: (WebCore::DOMTimer::install): (WebCore::DOMTimer::fired): * platform/ios/wak/WKContentObservation.cpp: (WKStartObservingStyleRecalcScheduling): (WKStopObservingStyleRecalcScheduling): (WKIsObservingStyleRecalcScheduling): (WKSetShouldObserveNextStyleRecalc): (WKShouldObserveNextStyleRecalc): (WKSetObservedContentChange): * platform/ios/wak/WKContentObservation.h: LayoutTests: * fast/events/touch/ios/hover-when-style-change-is-async-expected.txt: Added. * fast/events/touch/ios/hover-when-style-change-is-async.html: Added. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@238759 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Feb 3, 2019
…elements. https://bugs.webkit.org/show_bug.cgi?id=193431 Reviewed by Antti Koivisto. Source/WebCore: If the element has 'position: absolute', the containing block is established by the nearest ancestor with a 'position' of 'absolute', 'relative' or 'fixed', in the following way: 1. In the case that the ancestor is an inline element, the containing block is the bounding box around the padding boxes of the first and the last inline boxes generated for that element. In CSS 2.2, if the inline element is split across multiple lines, the containing block is undefined. 2. Otherwise, the containing block is formed by the padding edge of the ancestor. This patch covers #2. Test: fast/block/block-only/out-of-flow-with-containing-block-border-padding.html * layout/displaytree/DisplayBox.h: (WebCore::Display::Box::width const): (WebCore::Display::Box::height const): (WebCore::Display::Box::contentBoxTop const): (WebCore::Display::Box::contentBoxLeft const): (WebCore::Display::Box::paddingBoxTop const): (WebCore::Display::Box::paddingBoxLeft const): (WebCore::Display::Box::paddingBoxBottom const): (WebCore::Display::Box::paddingBoxRight const): (WebCore::Display::Box::paddingBoxHeight const): (WebCore::Display::Box::paddingBoxWidth const): * page/FrameViewLayoutContext.cpp: (WebCore::layoutUsingFormattingContext): Tools: * LayoutReloaded/misc/LFC-passing-tests.txt: LayoutTests: * fast/block/block-only/out-of-flow-with-containing-block-border-padding-expected.txt: Added. * fast/block/block-only/out-of-flow-with-containing-block-border-padding.html: Added. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@239983 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Feb 3, 2019
…ation https://bugs.webkit.org/show_bug.cgi?id=193499 Reviewed by Eric Carlson. MediaPlayerPrivateAVFoundation uses rate() as an indicator of whether the player is paused or not. This is incorrect when playback is stalled waiting for more data. For MPPAVFObjC, use the timeControlStatus as a more accurate indicator of whether the player is playing. Now that we have correct play state information, we can remove the handlePlaybackCommand() path when playing remotely for a more direct approach of notifying the HTMLMediaElement that the play state has changed. Drive-by fix: Before throwing away the AVPlayer, clear its output context. This keeps remote devices from keeping the AVPlayer alive. Drive-by fix #2: The NullMediaPlayer should always return "true" for paused(), not "false", since it can't possibly play anything. * platform/graphics/MediaPlayer.cpp: * platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp: (WebCore::MediaPlayerPrivateAVFoundation::paused const): * platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h: (WebCore::MediaPlayerPrivateAVFoundation::platformPaused const): * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h: * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm: (WebCore::MediaPlayerPrivateAVFoundationObjC::cancelLoad): (WebCore::MediaPlayerPrivateAVFoundationObjC::platformPaused const): (WebCore::MediaPlayerPrivateAVFoundationObjC::timeControlStatusDidChange): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@240123 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Feb 3, 2019
<https://bugs.webkit.org/show_bug.cgi?id=190925> <rdar://problem/45497382> * pal/spi/cf/CFNetworkSPI.h: Protect Objective-C code using defined(__OBJC__) macro. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@240430 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
May 4, 2019
https://bugs.webkit.org/show_bug.cgi?id=194648 Reviewed by Keith Miller. JSTests: * microbenchmarks/generate-multiple-llint-entrypoints.js: Added. Source/JavaScriptCore: 1. Making LLIntThunks singleton. Motivation: Former implementation has one LLIntThunk per type per VM. However, the generated code for every kind of thunk is essentially the same and we end up wasting memory (right now jitAllocationGranule = 32 bytes) when we have 2 or more VM instantiated. Turn these thunks into singleton will avoid such wasting. Tradeoff: This change comes with a price, because we will keep thunks allocated even when there is no VM instantiated. Considering WebCore use case, the situation of having no VM instantiated is uncommon, since once a VM is created through `commomVM()`, it will never be destroyed. Given that, this change does not impact the overall memory comsumption of WebCore/JSC. It also doesn't impact memory footprint, since thunks are generated lazily (see results below). Since we are keeping a static `MacroAssemblerCodeRef<JITThunkPtrTag>`, we have the assurance that JITed code will never be deallocated, given it is being pointed by `RefPtr<ExecutableMemoryHandle> m_executableMemory`. To understand why we decided to make LLIntThunks singleton instead of removing them, please see the comment on `llint/LLIntThunks.cpp`. 2. Making all LLIntEntrypoints singleton Motivation: With singleton LLIntThunks, we also can have singleton DirectJITCodes and NativeJITCodes for each LLIntEntrypoint type and avoid multiple allocations of objects with the same content. Tradeoff: As explained before, once we allocate an entrypoint, it will be alive until the program exits. However, the gains we can achieve in some use cases justifies such allocations. As DirectJITCode and NativeJITCode are ThreadSafeRefCounted and we are using `codeBlock->setJITCode(makeRef(*jitCode))`, their reference counter will never be less than 1. 3. Memory usage analysis This change reduces memory usage on stress/generate-multiple-llint-entrypoints.js by 2% and is neutral on JetStream 2. Following results were generated running each benchmark 6 times and using 95% Student's t distribution confidence interval. microbenchmarks/generate-multiple-llint-entrypoints.js (Changes uses less memory): Mean of memory peak on ToT: 122576896 bytes (confidence interval: 67747.2316) Mean of memory peak on Changes: 119248213.33 bytes (confidence interval: 50251.2718) JetStream2 (Neutral): Mean of memory peak on ToT: 5442742272 bytes (confidence interval: 134381565.9117) Mean of memory peak on Changes: 5384949760 bytes (confidence interval: 158413904.8352) 4. Performance Analysis This change is performance neutral on JetStream 2 and Speedometer 2. See results below.: JetStream 2 (Neutral): Mean of score on ToT: 139.58 (confidence interval: 2.44) Mean of score on Changes: 141.46 (confidence interval: 4.24) Speedometer run #1 ToT: 110 +- 2.9 Changes: 110 +- 1.8 Speedometer run #2 ToT: 110 +- 1.6 Changes: 108 +- 2.3 Speedometer run #3 ToT: 110 +- 3.0 Changes: 110 +- 1.4 * jit/JSInterfaceJIT.h: (JSC::JSInterfaceJIT::JSInterfaceJIT): * llint/LLIntEntrypoint.cpp: Here we are changing the usage or DirectJITCode by NativeJITCode on cases where there is no difference from address of calls with and without ArithCheck. (JSC::LLInt::setFunctionEntrypoint): (JSC::LLInt::setEvalEntrypoint): (JSC::LLInt::setProgramEntrypoint): (JSC::LLInt::setModuleProgramEntrypoint): (JSC::LLInt::setEntrypoint): * llint/LLIntEntrypoint.h: * llint/LLIntThunks.cpp: (JSC::LLInt::generateThunkWithJumpTo): (JSC::LLInt::functionForCallEntryThunk): (JSC::LLInt::functionForConstructEntryThunk): (JSC::LLInt::functionForCallArityCheckThunk): (JSC::LLInt::functionForConstructArityCheckThunk): (JSC::LLInt::evalEntryThunk): (JSC::LLInt::programEntryThunk): (JSC::LLInt::moduleProgramEntryThunk): (JSC::LLInt::functionForCallEntryThunkGenerator): Deleted. (JSC::LLInt::functionForConstructEntryThunkGenerator): Deleted. (JSC::LLInt::functionForCallArityCheckThunkGenerator): Deleted. (JSC::LLInt::functionForConstructArityCheckThunkGenerator): Deleted. (JSC::LLInt::evalEntryThunkGenerator): Deleted. (JSC::LLInt::programEntryThunkGenerator): Deleted. (JSC::LLInt::moduleProgramEntryThunkGenerator): Deleted. * llint/LLIntThunks.h: * runtime/ScriptExecutable.cpp: (JSC::setupLLInt): (JSC::ScriptExecutable::prepareForExecutionImpl): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@243136 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
May 18, 2019
…flaky failure on Mac Release builds https://bugs.webkit.org/show_bug.cgi?id=196905 <rdar://problem/49886096> Reviewed by Tim Horton. This flaky test exercises a race condition between when attachment insertion updates are dispatched from the web process to the UI process, and when script is executed via -[WKWebView evaluateJavaScript:completionHandler:]. Since attachment insertion and removal updates from the web process to the UI process are scheduled on a zero- delay timer, we end up with this sequence of events in the problematic (failure) case: (a) [UI] Run script #1 (which calls `HTMLAttachmentElement.getAttachmentIdentifier`) ...IPC from UI to web process... (b) [Web] Evaluate script #1 in the web process, which schedules attachment updates on a zero-delay timer ...IPC from web to UI process... (c) [UI] Invoke completion handler for script #1 (d) [UI] Run script #2 (which calls `document.querySelector('img').attachmentIdentifier`) ...IPC from UI to web process... (e) [Web] Evaluate script #2 in the web process (f) [Web] Zero-delay timer fires and dispatches attachment updates to the UI process ...which means that script #2 will complete before the UI process has received the attachment updates sent in step (f). However, in the case where the flaky test succeeds, the zero-delay timer in (f) fires *before* script #2 is run in step (e). This patch fixes the flaky test by waiting until attachment insertion updates are guaranteed to be received in the UI process by waiting on a script message posted by the web process, after attachment updates are dispatched. * TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm: (TestWebKitAPI::TEST): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@244251 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
May 18, 2019
https://bugs.webkit.org/show_bug.cgi?id=197118 <rdar://problem/49969960> Reviewed by Michael Saboff. JSTests: * stress/abstract-value-can-include-int52.js: Added. (foo): (index.index.8.index.60.index.65.index.1234.index.1234.parseInt.string_appeared_here.String.fromCharCode): Source/JavaScriptCore: Let's analyze this control flow diamond: #0 branch #1, #2 #1: PutStack(JSValue, loc42) Jump #3 #2: PutStack(Int52, loc42) Jump #3 #3: ... Our abstract value for loc42 at the head of #3 will contain an abstract value that us the union of Int52 with other things. Obviously in the above program, a GetStack for loc42 would be inavlid, since it might be loading either JSValue or Int52. However, the abstract interpreter just tracks what the value could be, and it could be Int52 or JSValue. When I did the Int52 refactoring, I expected such things to never happen, but it turns out it does. We should just allow for this instead of asserting against it since it's valid IR to do the above. * bytecode/SpeculatedType.cpp: (JSC::dumpSpeculation): * dfg/DFGAbstractValue.cpp: (JSC::DFG::AbstractValue::checkConsistency const): * dfg/DFGAbstractValue.h: (JSC::DFG::AbstractValue::validateTypeAcceptingBoxedInt52 const): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@244480 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Dec 8, 2019
…anches during interpretation https://bugs.webkit.org/show_bug.cgi?id=198650 Reviewed by Saam Barati. JSTests: * stress/object-allocation-sinking-interpretation-can-interpret-edges-that-can-be-proven-unreachable-in-ai.js: (main.v0): (main): Source/JavaScriptCore: Object Allocation Sinking phase has a lightweight abstract interpreter which interprets DFG nodes related to allocations and properties. This interpreter is lightweight since it does not track abstract values and conditions as deeply as AI does. It can happen that this interpreter interpret the control-flow edge that AI proved that is never taken. AI already knows some control-flow edges are never taken, and based on this information, AI can remove CheckStructure nodes. But ObjectAllocationSinking phase can trace this never-taken edges and propagate structure information that contradicts to the analysis done in ObjectAllocationSinking. Let's see the example. BB#0 35: NewObject([%AM:Object]) ... 47: Branch(ConstantTrue, T:#1, F:#2) BB#1 // This basic block is never taken due to @47's jump. ... 71: PutByOffset(@35, @66, id2{a}, 0, W:NamedProperties(2)) 72: PutStructure(@35, %AM:Object -> %Dx:Object, ID:60066) ... XX: Jump(#2) BB#2 ... 92: CheckStructure(@35, [%Dx:Object]) 93: PutByOffset(@35, @35, id2{a}, 0, W:NamedProperties(2)) ... AI removes @92 because AI knows BB#0 only takes BB#1 branch. @35's Structure is always %Dx so @92 is redundant. AI proved that @71 and @72 are always executed while BB#0 -> BB#2 edge is never taken so that @35 object's structure is proven at @92. After AI removes @92, ObjectAllocationSinking starts looking into this graph. BB#0 35: NewObject([%AM:Object]) ... 47: Branch(ConstantTrue, T:#1, F:#2) BB#1 // This basic block is never taken due to @47's jump. ... 71: PutByOffset(@35, @66, id2{a}, 0, W:NamedProperties(2)) 72: PutStructure(@35, %AM:Object -> %Dx:Object, ID:60066) ... XX: Jump(#2) BB#2 ... 93: PutByOffset(@35, @35, id2{a}, 0, W:NamedProperties(2)) ... YY: Jump(#3) BB#3 ... ZZ: <HERE> want to materialize @35's sunk object. Since AI does not change the @47 Branch to Jump (it is OK anyway), BB#0 -> BB#2 edge remains and ObjectAllocationSinking phase propagates information in BB#0's %AM structure information to BB#2. ObjectAllocationSinking phase converts @35 to PhantomNewObject, removes PutByOffset and PutStructure, and insert MaterializeNewObject in @zz. At this point, ObjectAllocationSinking lightweight interpreter gets two structures while AI gets one: @35's original one (%AM) and @72's replaced one (%Dx). Since AI already proved @zz only gets %Dx, AI removed @92 CheckStructure. But this is not known to ObjectAllocationSinking phase's interpretation. So when creating recovery data, MultiPutByOffset includes two structures, %AM and %Dx. This is OK since MultiPutByOffset takes conservative set of structures and performs switching. But the problem here is that %AM's id2{a} offset is -1 since %AM does not have such a property. So when creating MultiPutByOffset in ObjectAllocationSinking, we accidentally create MultiPutByOffset with -1 offset data, and lowering phase hits the debug assertion. 187: MultiPutByOffset(@138, @138, id2{a}, <Replace: [%AM:Object], offset = -1, >, <Replace: [%Dx:Object], offset = 0, >) This bug is harmless since %AM structure comparison never meets at runtime. But we are not considering the case including `-1` offset property in MultiPutByOffset data. In this patch, we just filter out apparently wrong structures when creating MultiPutByOffset in ObjectAllocationSinking. This is OK since it never comes at runtime. * dfg/DFGObjectAllocationSinkingPhase.cpp: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@249306 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Dec 8, 2019
https://bugs.webkit.org/show_bug.cgi?id=201529 <rdar://problem/53935772> Not reviewed. * stress/test-out-of-memory.js: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@249610 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Jan 26, 2020
Object allocation sinking is missing PutHint for allocations unreachable in the graph https://bugs.webkit.org/show_bug.cgi?id=203799 <rdar://problem/56852162> Reviewed by Saam Barati. * stress/allocation-sinking-puthint-control-flow-2.js: Added. (f.handler.construct): (f): Source/JavaScriptCore: Object allocation sinking is missing PutHint for sunken allocations https://bugs.webkit.org/show_bug.cgi?id=203799 <rdar://problem/56852162> Reviewed by Saam Barati. Consider the following graph: Block #0: 1: PhantomCreateActivation() 2: PhantomNewFunction() PutHint(@2, @1, FunctionActivationPLoc) Branch(#1, #2) Block #1: 3: MaterializeCreateActivation() PutHint(@2, @3, FunctionActivationPLoc) Upsilon(@3, ^5) Jump(#3) Block #2: 4: MaterializeCreateActivation() PutHint(@2, @4, FunctionActivationPLoc) Upsilon(@4, ^5) Jump(#3) Block #3: 5: Phi() ExitOK() On Block #3, we need to emit a PutHint after the Phi, since we might exit after it. However, object allocation sinking skipped this Phi because it was checking whether the base of the location that caused us to create this Phi (@2) was live, but it's dead in the graph (there are no pointers to it). The issue is that, even though there are no pointers to the base, the location `PromotedHeapLocation(@2, FunctionActivationPLoc)` is still live, so we should PutHint to it. We fix it by checking for liveness of the location rather than its base. * dfg/DFGObjectAllocationSinkingPhase.cpp: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@254725 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Feb 6, 2020
Object allocation sinking is missing PutHint for allocations unreachable in the graph https://bugs.webkit.org/show_bug.cgi?id=203799 <rdar://problem/56852162> Reviewed by Saam Barati. * stress/allocation-sinking-puthint-control-flow-2.js: Added. (f.handler.construct): (f): Source/JavaScriptCore: Object allocation sinking is missing PutHint for sunken allocations https://bugs.webkit.org/show_bug.cgi?id=203799 <rdar://problem/56852162> Reviewed by Saam Barati. Consider the following graph: Block #0: 1: PhantomCreateActivation() 2: PhantomNewFunction() PutHint(@2, @1, FunctionActivationPLoc) Branch(#1, #2) Block #1: 3: MaterializeCreateActivation() PutHint(@2, @3, FunctionActivationPLoc) Upsilon(@3, ^5) Jump(#3) Block #2: 4: MaterializeCreateActivation() PutHint(@2, @4, FunctionActivationPLoc) Upsilon(@4, ^5) Jump(#3) Block #3: 5: Phi() ExitOK() On Block #3, we need to emit a PutHint after the Phi, since we might exit after it. However, object allocation sinking skipped this Phi because it was checking whether the base of the location that caused us to create this Phi (@2) was live, but it's dead in the graph (there are no pointers to it). The issue is that, even though there are no pointers to the base, the location `PromotedHeapLocation(@2, FunctionActivationPLoc)` is still live, so we should PutHint to it. We fix it by checking for liveness of the location rather than its base. * dfg/DFGObjectAllocationSinkingPhase.cpp: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@254866 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Feb 6, 2020
…rchability. https://bugs.webkit.org/show_bug.cgi?id=207024 Reviewed by Saam Barati. This patch applies the following changes: 1. Prefix Air and B2 dumps with a tierName prefix. The tierName prefix strings are as follows: "FTL ", "DFG ", "b3 ", "Air ", "asm " The choice to use a lowercase "b3" and "asm" with upper case "Air" is deliberate because I found this combination to be easier to read and scan as prefixes of the dump lines. See dump samples below. 2. Make DFG node IDs consistently expressed as D@<node index> e.g. D@104. The definition of the node will be the id followed by a colon e.g. D@104: This makes it easy to search references to this node anywhere in the dump. Make B3 nodes expressed as b@<node index> e.g. b@542. This also makes it searchable since there's now no ambiguity between b@542 and D@542. The choice to use a lowercase "b" and an uppercase "D" is intentional because "b@542" and "d@542" looks too similar, and I prefer to not use too much uppercase. Plus this makes the node consistent in capitalization with the tierName prefixes above of "b3 " and "DFG " respectively. Here's a sample of what the dumps now look like: DFG graph dump: <code> ... 6 55: <-- foo#DFndCW:<0x62d0000b8140, bc#65, Call, known callee: Object: 0x62d000035920 with butterfly 0x0 (Structure %AN:Function), StructureID: 12711, numArgs+this = 1, numFixup = 0, stackOffset = -16 (loc0 maps to loc16)> 3 6 55: D@79:< 3:-> ArithAdd(Int32:Kill:D@95, Int32:D@42, Int32|PureNum|UseAsOther, Int32, CheckOverflow, Exits, bc#71, ExitValid) 4 6 55: D@3:<!0:-> KillStack(MustGen, loc7, W:Stack(loc7), ClobbersExit, bc#71, ExitInvalid) 5 6 55: D@85:<!0:-> MovHint(Check:Untyped:D@79, MustGen, loc7, W:SideState, ClobbersExit, bc#71, ExitInvalid) 6 6 55: D@102:< 1:-> CompareLess(Int32:D@79, Int32:D@89, Boolean|UseAsOther, Bool, Exits, bc#74, ExitValid) 7 6 55: D@104:<!0:-> Branch(KnownBoolean:Kill:D@102, MustGen, T:#1/w:10.000000, F:#7/w:1.000000, W:SideState, bc#74, ExitInvalid) ... </code> B3 graph dump: <code> ... b3 BB#14: ; frequency = 10.000000 b3 Predecessors: #13 b3 Int32 b@531 = CheckAdd(b@10:WarmAny, $1(b@1):WarmAny, b@64:ColdAny, b@10:ColdAny, generator = 0x606000022e80, earlyClobbered = [], lateClobbered = [], usedRegisters = [], ExitsSideways|Reads:Top, D@79) b3 Int32 b@539 = LessThan(b@531, $100(b@578), D@102) b3 Void b@542 = Branch(b@539, Terminal, D@104) b3 Successors: Then:#2, Else:#15 ... </code> Air graph dump: <code> ... Air BB#5: ; frequency = 10.000000 Air Predecessors: #4 Air Move -96(%rbp), %rax, b@531 Air Patch &BranchAdd32(3,ForceLateUseUnlessRecoverable)3, Overflow, $1, %rax, -104(%rbp), -96(%rbp), b@531 Air Branch32 LessThan, %rax, $100, b@542 Air Successors: #1, #6 ... </code> FTL disassembly dump: <code> ... Air BB#5: ; frequency = 10.000000 Air Predecessors: #4 DFG D@42:< 2:-> JSConstant(JS|PureInt, Int32, Int32: 1, bc#0, ExitInvalid) DFG D@79:< 3:-> ArithAdd(Int32:Kill:D@95, Int32:D@42, Int32|PureNum|UseAsOther, Int32, CheckOverflow, Exits, bc#71, ExitValid) b3 Int32 b@1 = Const32(1) b3 Int32 b@531 = CheckAdd(b@10:WarmAny, $1(b@1):WarmAny, b@64:ColdAny, b@10:ColdAny, generator = 0x606000022e80, earlyClobbered = [], lateClobbered = [], usedRegisters = [%rax, %rbx, %rbp, %r12], ExitsSideways|Reads:Top, D@79) Air Move -96(%rbp), %rax, b@531 asm 0x4576b9c04712: mov -0x60(%rbp), %rax Air Patch &BranchAdd32(3,ForceLateUseUnlessRecoverable)3, Overflow, $1, %rax, -104(%rbp), -96(%rbp), b@531 asm 0x4576b9c04716: inc %eax asm 0x4576b9c04718: jo 0x4576b9c04861 DFG D@89:< 1:-> JSConstant(JS|PureNum|UseAsOther, NonBoolInt32, Int32: 100, bc#0, ExitInvalid) DFG D@102:< 1:-> CompareLess(Int32:D@79, Int32:D@89, Boolean|UseAsOther, Bool, Exits, bc#74, ExitValid) DFG D@104:<!0:-> Branch(KnownBoolean:Kill:D@102, MustGen, T:#1/w:10.000000, F:#7/w:1.000000, W:SideState, bc#74, ExitInvalid) b3 Int32 b@578 = Const32(100, D@89) b3 Int32 b@539 = LessThan(b@531, $100(b@578), D@102) b3 Void b@542 = Branch(b@539, Terminal, D@104) Air Branch32 LessThan, %rax, $100, b@542 asm 0x4576b9c0471e: cmp $0x64, %eax asm 0x4576b9c04721: jl 0x4576b9c0462f Air Successors: #1, #6 ... </code> * b3/B3BasicBlock.cpp: (JSC::B3::BasicBlock::deepDump const): * b3/B3Common.cpp: * b3/B3Common.h: * b3/B3Generate.cpp: (JSC::B3::generateToAir): * b3/B3Procedure.cpp: (JSC::B3::Procedure::dump const): * b3/B3Value.cpp: * b3/air/AirBasicBlock.cpp: (JSC::B3::Air::BasicBlock::deepDump const): (JSC::B3::Air::BasicBlock::dumpHeader const): (JSC::B3::Air::BasicBlock::dumpFooter const): * b3/air/AirCode.cpp: (JSC::B3::Air::Code::dump const): * b3/air/AirCode.h: * b3/air/AirDisassembler.cpp: (JSC::B3::Air::Disassembler::dump): * b3/air/AirGenerate.cpp: (JSC::B3::Air::prepareForGeneration): * dfg/DFGCommon.cpp: * dfg/DFGCommon.h: * dfg/DFGGraph.cpp: (JSC::DFG::Graph::dump): (JSC::DFG::Graph::dumpBlockHeader): * dfg/DFGNode.cpp: (WTF::printInternal): * ftl/FTLCompile.cpp: (JSC::FTL::compile): * ftl/FTLCompile.h: * ftl/FTLState.cpp: (JSC::FTL::State::State): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@255482 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Aug 9, 2020
* UIProcess/ios/WKMouseGestureRecognizer.mm:
- Use ALLOW_DEPRECATED_IMPLEMENTATIONS_{BEGIN,END}.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@261454 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Aug 18, 2020
…id places for recovery https://bugs.webkit.org/show_bug.cgi?id=215434 Reviewed by Saam Barati. JSTests: * stress/for-of-post-sinking-osr-availability.js: Added. (foo): Source/JavaScriptCore: It's somewhat subtle why we cannot use the node for the GetStack itself in the Availability's node field. The reason is that if we did we would need to make any phase that converts nodes to GetStack availability aware. For instance, a place where this could come up is in constant folding if you had a graph like the following, which could arise from PutStack sinking: BB#1: @1: NewObject() @2: MovHint(@1, inline-arg1) @3: Jump(#2, #3) BB#2: @4: PutStack(@1, inline-arg1) @5: GetMyArgumentByVal(inline-arg1) @6: Jump(#3) BB#3: @7: InvalidationPoint() If constant folding converts @5 to a GetStack then at @7 inline-arg1 won't be available since at the end of BB#1 our availability is (@1, DeadFlush) and (@5, FlushedAt(inline-arg1)). When that gets merged at BB#3 then the availability will be (nullptr, ConflictingFlush). This patch also makes validation check that availability is sane at each pontential exit site if Options::validateFTLOSRExitLiveness() is set. Since this is actually a Phase we also need to make sure that we don't infinite loop, so there is now a m_isValidating field on m_graph. The validateOSRExitAvailability phase is also careful not to modify the Graph, in order to avoid masking bugs when validating. In a followup patch I intend to look into why MovHint elimination will convert: @2: MovHint(@0, loc1, bc#1, ExitInvalid) @3: KillStack(loc1, bc#2, ExitValid) @4: MovHint(@1 loc1, bc#2, ExitInvalid) into @2: ZombieHint(@0, loc1, bc#1, ExitInvalid) @3: KillStack(loc1, bc#2, ExitValid) @4: MovHint(@1 loc1, bc#2, ExitInvalid) when loc1 is live in the bytecode at bc#2. But for now, the validation rule works around this by only checking when mayExit and the nodes exitOK agree exiting is possible. * dfg/DFGGraph.h: * dfg/DFGOSRAvailabilityAnalysisPhase.cpp: (JSC::DFG::OSRAvailabilityAnalysisPhase::OSRAvailabilityAnalysisPhase): (JSC::DFG::OSRAvailabilityAnalysisPhase::run): (JSC::DFG::performOSRAvailabilityAnalysis): (JSC::DFG::validateOSRExitAvailability): (JSC::DFG::LocalOSRAvailabilityCalculator::executeNode): * dfg/DFGOSRAvailabilityAnalysisPhase.h: * dfg/DFGPhase.h: (JSC::DFG::runPhase): * dfg/DFGValidate.cpp: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@265685 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy
pushed a commit
that referenced
this pull request
Nov 28, 2020
https://bugs.webkit.org/show_bug.cgi?id=218398 <rdar://problem/67613836> Reviewed by Chris Dumez. Make the MainThreadGenericEventQueue protect the target as well as the owner of the queue. Drive-by fix: Create the scoped `eventFiringScope` object after the `protect` object, to ensure that the member variable set by the first scope will safely occur. Drive-by fix #2: Also null-check the result of document().page() within HTMLMediaElement::dispatchEvent(). * dom/GenericEventQueue.cpp: (WebCore::MainThreadGenericEventQueue::dispatchOneEvent): * html/HTMLMediaElement.cpp: (WebCore::HTMLMediaElement::dispatchEvent): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@269321 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fData->frame->document() can return NULL in certain cases, causing a crash.