Closed
Conversation
See web-platform-tests#7544. TBR: mkwst@chromium.org Change-Id: I620980cf9f969045fdcc1eecaffbdec6ec607e07 Reviewed-on: https://chromium-review.googlesource.com/696084 Reviewed-by: Philip Jägenstedt <foolip@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Commit-Queue: Philip Jägenstedt <foolip@chromium.org> Cr-Commit-Position: refs/heads/master@{#506375}
Note that the expectation is flipped to match the specification.
…tform-tests#7510) SpeechSynthesis-speak-twice.html is timeout in Firefox, written for this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1372325
Done by listing all files using setTimeout and manually makes reasonably-sized groups.
jdm
pushed a commit
that referenced
this pull request
Jan 31, 2018
…mPoint" This reverts commit dd944882a245a5117b50cb417138d92f32d931d6. Reason for revert: This causes WebKit Linux Trusty ASAN buildbot failure. https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Linux%20Trusty%20ASAN/builds/8618 23:46:29.565 3877 ==1==ERROR: AddressSanitizer: use-after-poison on address 0x7ead60c0dbf0 at pc 0x00000dca937a bp 0x7ffd86b90c10 sp 0x7ffd86b90c08 23:46:29.565 3877 READ of size 8 at 0x7ead60c0dbf0 thread T0 (content_shell) 23:46:29.565 3877 #0 0xdca9379 in operator==<const blink::TreeScope, const blink::TreeScope> third_party/WebKit/Source/platform/heap/Member.h:128:27 23:46:29.565 3877 #1 0xdca9379 in blink::TreeScope::Retarget(blink::Element const&) const third_party/WebKit/Source/core/dom/TreeScope.cpp:393:0 23:46:29.565 3877 #2 0xdca8894 in blink::TreeScope::HitTestPointInternal(blink::Node*) const third_party/WebKit/Source/core/dom/TreeScope.cpp:267:10 23:46:29.565 3877 #3 0xdca8325 in HitTestPoint third_party/WebKit/Source/core/dom/TreeScope.cpp:254:10 23:46:29.566 3877 #4 0xdca8325 in blink::TreeScope::ElementFromPoint(double, double) const third_party/WebKit/Source/core/dom/TreeScope.cpp:245:0 23:46:29.566 3877 #5 0xc9353a7 in elementFromPoint third_party/WebKit/Source/core/dom/DocumentOrShadowRoot.h:38:23 Original change's description: > Fix retargeting of result in elementFromPoint and elementsFromPoint > > Currently elementFromPoint and elementsFromPoint are not per spec, and it may > return null incorrectly. This change adds retargeting of the result with > respect to the context object, and adds some tests that are similar to > elementFromPoint tests in WebKit, but with some corrected cases: > https://git.webkit.org/?p=WebKit-https.git;a=blob;f=LayoutTests/fast/shadow-dom/DocumentOrShadowRoot-prototype-elementFromPoint.html;h=a8dc4da2430713521b9ba77c742db10397a8e638;hb=HEAD > > Spec: > https://w3c.github.io/webcomponents/spec/shadow/#extensions-to-the-documentorshadowroot-mixin > > Bug: 759947 > Change-Id: I6aece5e9cc826124772c6ce13c806865055b2b9b > Reviewed-on: https://chromium-review.googlesource.com/808446 > Commit-Queue: Rakina Zata Amni <rakina@chromium.org> > Reviewed-by: Dmitry Gozman <dgozman@chromium.org> > Reviewed-by: Hayato Ito <hayato@chromium.org> > Reviewed-by: Takayoshi Kochi <kochi@chromium.org> > Cr-Commit-Position: refs/heads/master@{#531139} TBR=kochi@chromium.org,dgozman@chromium.org,hayato@chromium.org,pfeldman@chromium.org,rakina@chromium.org Change-Id: Id62abd371d93627d3178b63ca189cecfe9ff44d4 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 759947 Reviewed-on: https://chromium-review.googlesource.com/880264 Reviewed-by: Takashi Sakamoto <tasak@google.com> Commit-Queue: Takashi Sakamoto <tasak@google.com> Cr-Commit-Position: refs/heads/master@{#531178}
jdm
pushed a commit
that referenced
this pull request
Mar 21, 2018
Prior to this change, response body methods such as response.body.arrayBuffer() would reject with a TypeError if the fetch was aborted. This change makes them correctly reject with an AbortError instead. This implements stage #3 of the "Body Cancellation" section of the design doc: https://docs.google.com/document/d/1OuoCG2uiijbAwbCw9jaS7tHEO0LBO_4gMNio1ox0qlY/edit#heading=h.fvc7d7q07oio Bug: 817687 Change-Id: Ifde322d9c22485a8ba9d14fd4ffca65c4fb4745a Reviewed-on: https://chromium-review.googlesource.com/954765 Commit-Queue: Adam Rice <ricea@chromium.org> Reviewed-by: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Cr-Commit-Position: refs/heads/master@{#544335}
jdm
pushed a commit
that referenced
this pull request
Mar 6, 2019
chromedriver doesn't allow changing Object.prototype to add enumerable
properties, but this test requires setting some values on
Object.prototype. When Object.prototype.a is set to:
{b: {c: 'on proto'}}
chromedriver fails with:
JavascriptErrorException: javascript error (500): Maximum call stack size exceeded
(Session info: chrome=72.0.3626.121)
Remote-end stacktrace:
#0 0x563ff3a32a59 <unknown>
#1 0x563ff39cb7f3 <unknown>
#2 0x563ff38fcd7c <unknown>
#3 0x563ff38ff78c <unknown>
#4 0x563ff38ff5f7 <unknown>
#5 0x563ff38ffbe7 <unknown>
#6 0x563ff38fff1b <unknown>
#7 0x563ff38a3f7a <unknown>
#8 0x563ff3899bf2 <unknown>
#9 0x563ff38a37b7 <unknown>
#10 0x563ff3899ac3 <unknown>
#11 0x563ff38782d2 <unknown>
#12 0x563ff3879112 <unknown>
#13 0x563ff39fe865 <unknown>
#14 0x563ff39ff32b <unknown>
#15 0x563ff39ff70c <unknown>
#16 0x563ff39d940a <unknown>
#17 0x563ff39ff997 <unknown>
#18 0x563ff39e9947 <unknown>
#19 0x563ff3a1a800 <unknown>
#20 0x563ff3a3c8be <unknown>
#21 0x7f3bf4545494 start_thread
#22 0x7f3bf2d58a8f clone
Ran 1 tests finished in 2.0 seconds.
• 0 ran as expected. 0 tests skipped.
• 1 tests had errors unexpectedly
Work around this problem by cleaning up the test environment so
Object.prototype no longer has the override by the time chromedriver
tries to inspect the test result.
While here, fix the other tests to use the t.add_cleanup() function
so they'll cleanup their test environment in case they exit in
some other way besides reaching t.done().
The underlying chromedriver issue is tracked upstream at
https://crbug.com/chromedriver/2555.
Bug: 934844
Change-Id: Id1b4ab2a908bfbc001e2a2d045eeec3ef01c24d9
jdm
pushed a commit
that referenced
this pull request
Jun 28, 2019
All tests pass, and crashes no longer happen. I believe that code will not longer crash, but there might be futher instances of incorrect positioning. Fix #1 LayoutDescendantCandidates did not sweep newly discovered candidates. This was done manually once inside NGOutOfFlowLayoutPart::Run, and sweep was not performed for LayoutDescendantCandidates found in Legacy. Fix is to make LayoutDescendantCandidates perform sweep instead. Fix #2 fix #1 exposed a bug where duplicate fragments were generated for a single layout object. This happened when NG was generating fragments not inside ContainingBlock. Fix one instance of this inside NGOutOfFlowLayoutPart::IsContainingBlockForDescendant by making sure that OOF with inline containers are only positioned inside its ContainingBlock() Fix #3 NGOutOfFlowLayoutPart::LayoutDescendant offset adjustment. Bug: 935805 Change-Id: I9f7ebbc7223f40fbbf6ba3739d9385bfd59e3641 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1517093 Commit-Queue: Aleks Totic <atotic@chromium.org> Reviewed-by: Morten Stenshorne <mstensho@chromium.org> Reviewed-by: Koji Ishii <kojii@chromium.org> Cr-Commit-Position: refs/heads/master@{#641628}
jdm
pushed a commit
that referenced
this pull request
Jun 28, 2019
This reverts commit 712c3cf3ed8201420acf23f760eaa34be20781cd. Reason for revert: This patch causes webkit-layout-tests failure on WebKit_Linux_Trusty_ASAN bot: https://ci.chromium.org/p/chromium/builders/ci/WebKit%20Linux%20Trusty%20ASAN/25720 Unexpected Failures: * external/wpt/css/css-scroll-snap/scroll-snap-type.html * virtual/threaded/external/wpt/css/css-scroll-snap/scroll-snap-type.html STDERR: ==1==ERROR: AddressSanitizer: heap-use-after-free on address 0x61200023f8d8 at pc 0x5620c924e56d bp 0x7ffde3c56830 sp 0x7ffde3c56828 STDERR: READ of size 8 at 0x61200023f8d8 thread T0 (content_shell) STDERR: #0 0x5620c924e56c in get ./../../base/memory/scoped_refptr.h:212:27 STDERR: #1 0x5620c924e56c in Style ./../../third_party/blink/renderer/core/layout/layout_object.h:1615:0 STDERR: #2 0x5620c924e56c in GetPhysicalSnapType ./../../third_party/blink/renderer/core/page/scrolling/snap_coordinator.cc:88:0 STDERR: #3 0x5620c924e56c in blink::SnapCoordinator::UpdateSnapContainerData(blink::LayoutBox&) ./../../third_party/blink/renderer/core/page/scrolling/snap_coordinator.cc:107:0 STDERR: #4 0x5620c924e74b in blink::SnapCoordinator::UpdateAllSnapContainerData() ./../../third_party/blink/renderer/core/page/scrolling/snap_coordinator.cc:76:5 Original change's description: > Correctly handle scroll-snap-type changes to 'none' > > > Previously when a scroll container's snap type is changed to 'none' its > data was discarded including all of its snap areas. However this is > incorrect. Because while the snap type is 'none', the element is still > a scroll container which per spec [1] means that is should continue to > captures the snap areas in its subtree for whom it is the nearest > ancestor scroll container . The only difference is that it no longer > snaps. > > The fix is that we no longer remove the snap container data just > because is has a 'none' snap type and instead keep it and its snap > areas. But we check the snap type before performing any snap. > > To ensure this does not introduce any performance regression, this CL > also includes an optimization where we avoid re-calculating > snap_container_data when the snap type is 'none'. So keeping these snap > data should not be cheap. > > Note that there is another problem where if the current snap container > is no longer a scroll container (e.g., overflow: scroll => overflow: > visible) we release its snap areas and they become "orphan". But if we > are to do this correctly, we should re-assign these areas to the next > stroller in the chain. Similarly when an element becomes a scroll > container, it can potentially take over snap areas from its parent snap > container. > > > This patch does not address that situation yet but fixes the easier > problem. > > [1] https://drafts.csswg.org/css-scroll-snap/#overview > > Bug: 953575 > Test: > - wpt/css/css-scroll-snap/scroll-snap-type-change.html => Changing snap-type should work correctly > - wpt/css/css-scroll-snap/scroll-snap-type.html => Add a specific test for type 'none' to ensure it does not snap > > Change-Id: Ie493ad68ecba818ed41c0ee103ccf44725ff6e3f > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1589899 > Reviewed-by: Majid Valipour <majidvp@chromium.org> > Reviewed-by: David Bokan <bokan@chromium.org> > Commit-Queue: Majid Valipour <majidvp@chromium.org> > Cr-Commit-Position: refs/heads/master@{#657460} TBR=bokan@chromium.org,majidvp@chromium.org Change-Id: I3a327f6e342e95d045194d24ceaf49de52b2b921 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 953575 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1600437 Reviewed-by: Takashi Sakamoto <tasak@google.com> Commit-Queue: Takashi Sakamoto <tasak@google.com> Cr-Commit-Position: refs/heads/master@{#657571}
This file contains hidden or 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
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.
Reviewed in https://github.com/servo/servo/pulls/18746.