Generalized Test #1

mariestaver · 2018-06-13T19:41:50Z

@jugglinmike -- ready for review!

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> web-platform-tests#14 0x563ff39ff32b <unknown> web-platform-tests#15 0x563ff39ff70c <unknown> web-platform-tests#16 0x563ff39d940a <unknown> web-platform-tests#17 0x563ff39ff997 <unknown> web-platform-tests#18 0x563ff39e9947 <unknown> web-platform-tests#19 0x563ff3a1a800 <unknown> web-platform-tests#20 0x563ff3a3c8be <unknown> web-platform-tests#21 0x7f3bf4545494 start_thread web-platform-tests#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

We used to commit navigation after receiving the first byte of document response. This CL moves commit earlier, synchronously done from CommitNavigation call. The change should not be web-observable, but some internal assumptions may have been affected. Test changes: - ReplacingDocumentLoaderFiresLoadEvent was testing the old behavior, which is not applicable anymore. - MultiChunkWithReentrancy now uses a different method to trigger reentrancy (pdf plugin), since we no longer commit after first byte. - backdrop-object.html and anchor-change-href.svg relied on test finishing late enough, now they wait for onload to eliminate a race. - use-property-synchronization-crash.html now reports an error message synchronously and therefore has JS stack and a line number. - setting-allowpaymentrequest-timing.https.sub.html has a race as explained here [1], and now fails even without site isolation. This corresponds to the step 8.b from the doc linked to the bug. Difference from attempt #1 (https://chromium-review.googlesource.com/c/1399447): - PluginDocumentParser and MediaDocumentParser early return if not parsing before accessing GetDocument. This is because DocumentLoader calls Finish() even after parser was stopped/detached. For example in Document::Abort we cancel parsing, but committed DocumentLoader might be still receiving data. We should ideally clean up all calls into parser, there are numerous TODOs for that. - pageload-image-in-popup.html relies on small image being parsed in the same task as navigation commit. Using onload seems to fix the issue. - touch-handler-iframe-plugin-assert.js hopes that onload for about:blank happens after test has finished, which is racy now. [1] https://bugs.chromium.org/p/chromium/issues/detail?id=819800#c6 Bug: 855189, 937639, 836242, 937358 Change-Id: I65048a27e6d249a608d4eb61e5c882292386026e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1506663 Commit-Queue: Dmitry Gozman <dgozman@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#639992}

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}

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}

Issue #1: The "SecureContext" IDL attribute considers localhost to be secure, but the Cookie Store API assumed that it would only be exposed on cryptographically secure origins, so a DCHECK caused attempts to set/delete cookies on http://localhost to crash. This CL replaces the DCHECK with an exception that is thrown on attempts to set/delete secure cookies on cryptographically insecure origins. Issue #2: The "secure" cookie attribute defaulted to true. Setting/deleting a secure cookie on a cryptographically insecure origin is prohibited. The cookieStore.delete() API excluded the option to set the "secure" attribute, however, so there was no way to delete an insecure cookie. This CL defaults the "secure" attribute to true on cryptographically secure origins, and false otherwise. Bug: 956641 Change-Id: Iff7c22713e8604d60b68d42199a74b2d08235712 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1700357 Commit-Queue: Staphany Park <staphany@chromium.org> Reviewed-by: Victor Costan <pwnall@chromium.org> Cr-Commit-Position: refs/heads/master@{#681054}

Bug is: <container overflow:scroll> <target transform:scale(1)> Initially, container's scrollbars are disabled. When target changes its scale and grows outside of container, scrollbars were not updated. Fix #1 is to call UpdateScrollbarEnabledState. This resulted in scrollbars being painted, but not clickable. Fix #2, calling UpdateScrollableAreaSet makes scrollbars clickable too. Fix #2 was an educated guess. Bug: 926167 Change-Id: I02454047c87aaecede9c56db1c02bbd1b21b15c5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1704218 Commit-Queue: Aleks Totic <atotic@chromium.org> Reviewed-by: Stefan Zager <szager@chromium.org> Cr-Commit-Position: refs/heads/master@{#681091}

…iner height See stack trace below. We set the override container logical height to -1 for the initial layout of a flex item so that we compute the correct size for min-height. However, that messes with our cache for definite heights because we would always set it to indefinite in such a case. Instead, just don't cache these values. That way we will later compute the right thing for resolving flex-basis, etc. (FlexNG can't come soon enough...) #0 blink::LayoutBox::ContainingBlockLogicalHeightForPercentageResolution (this=0x3dda8d434198, out_cb=0x7f6e7d42d8c0, out_skipped_auto_height_containing_block=0x0) at ../../third_party/blink/renderer/core/layout/layout_box.cc:3833 #1 0x00007f6ee84ad0a1 in blink::LayoutFlexibleBox::MainAxisLengthIsDefinite (this=0x3dda8d434010, child=..., flex_basis=Length(0%, Percent), add_to_cb=false) at ../../third_party/blink/renderer/core/layout/layout_flexible_box.cc:762 #2 0x00007f6ee84af930 in blink::LayoutFlexibleBox::MainSizeIsDefiniteForPercentageResolution ( this=0x3dda8d434010, child=...) at ../../third_party/blink/renderer/core/layout/layout_flexible_box.cc:1125 #3 0x00007f6ee84ad7f5 in blink::LayoutFlexibleBox::UseOverrideLogicalHeightForPerentageResolution ( this=0x3dda8d434010, child=...) at ../../third_party/blink/renderer/core/layout/layout_flexible_box.cc:1137 #4 0x00007f6ee83f2b9d in blink::LayoutBlock::AvailableLogicalHeightForPercentageComputation ( this=0x3dda8d434198) at ../../third_party/blink/renderer/core/layout/layout_block.cc:2333 #5 0x00007f6ee845e745 in blink::LayoutBox::ContainingBlockLogicalHeightForPercentageResolution ( this=0x3dda8d4243d0, out_cb=0x0, out_skipped_auto_height_containing_block=0x0) at ../../third_party/blink/renderer/core/layout/layout_box.cc:3830 #6 0x00007f6ee86dcc5c in blink::LayoutBoxUtils::AvailableLogicalHeight (box=..., cb=0x3dda8d434198) at ../../third_party/blink/renderer/core/layout/ng/layout_box_utils.cc:64 #7 0x00007f6ee86eafea in blink::LayoutNGMixin<blink::LayoutBlockFlow>::ComputeIntrinsicLogicalWidths ( this=0x3dda8d4243d0, min_logical_width=0px, max_logical_width=0px) at ../../third_party/blink/renderer/core/layout/ng/layout_ng_mixin.cc:48 #8 0x00007f6ee83ef53a in blink::LayoutBlock::ComputePreferredLogicalWidths (this=0x3dda8d4243d0) at ../../third_party/blink/renderer/core/layout/layout_block.cc:1509 #9 0x00007f6ee8451f01 in blink::LayoutBox::MaxPreferredLogicalWidth (this=0x3dda8d4243d0) at ../../third_party/blink/renderer/core/layout/layout_box.cc:1395 #10 0x00007f6ee84adba2 in blink::LayoutFlexibleBox::ComputeInnerFlexBaseSizeForChild (this=0x3dda8d434198, child=..., main_axis_border_and_padding=0px, child_layout_type=blink::LayoutFlexibleBox::kForceLayout) at ../../third_party/blink/renderer/core/layout/layout_flexible_box.cc:890 #11 0x00007f6ee84ae5d1 in blink::LayoutFlexibleBox::ConstructAndAppendFlexItem (this=0x3dda8d434198, algorithm=0x7f6e7d42ed70, child=..., layout_type=blink::LayoutFlexibleBox::kForceLayout) at ../../third_party/blink/renderer/core/layout/layout_flexible_box.cc:1203 #12 0x00007f6ee84aa27b in blink::LayoutFlexibleBox::LayoutFlexItems (this=0x3dda8d434198, relayout_children=true, layout_scope=...) at ../../third_party/blink/renderer/core/layout/layout_flexible_box.cc:934 #13 0x00007f6ee84a9cff in blink::LayoutFlexibleBox::UpdateBlockLayout (this=0x3dda8d434198, relayout_children=true) at ../../third_party/blink/renderer/core/layout/layout_flexible_box.cc:369 Bug: 1019138 Change-Id: Ie94e69a5f3fe6accc3623d358315b174088d5597 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1902514 Commit-Queue: David Grogan <dgrogan@chromium.org> Auto-Submit: Christian Biesinger <cbiesinger@chromium.org> Reviewed-by: David Grogan <dgrogan@chromium.org> Cr-Commit-Position: refs/heads/master@{#713296}

Intent-to-ship approval: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/4DC56FN3paU Changes are: 1. The background of the root element paints into the property tree state that includes the local border box effect state of the root element. 2. If #1 is happening, paint a separate backdrop in the ContentsProperties state of the LayoutView, to allow effects, clips and transforms of the root element to apply via PaintChunksToCcLayer. Bug: 898293,988446,1029170,711955 Change-Id: I751e3f9c23d0b06a31cf1b13c3853180bc7ac7e6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1979258 Commit-Queue: Chris Harrelson <chrishtr@chromium.org> Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org> Cr-Commit-Position: refs/heads/master@{#733661}

With this CL, the .content() property of a declarative shadow root (a <template shadowroot="open|closed"> element being parsed) will return null. Note that this <template> element only exists during parsing, and upon the closing </template> tag being encountered by the parser, the shadowroot will be created, the <template> contents will be moved into the shadowroot, and the <template> itself will be deleted. So this change is only visible from MutationObservers and other script that runs *during* parsing. This CL prevents that script from accessing the contents of the (potentially closed) shadow root. See point #1 at [1] for more context. [1] whatwg/dom#831 (comment) Bug: 1042130 Change-Id: Id096d1b65dc94b7a75afdc6143341eb4521da41a Cq-Do-Not-Cancel-Tryjobs: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2133308 Commit-Queue: Mason Freed <masonfreed@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Reviewed-by: Kent Tamura <tkent@chromium.org> Auto-Submit: Mason Freed <masonfreed@chromium.org> Cr-Commit-Position: refs/heads/master@{#759495}

Paint worklet already works with custom property animation running on the compositor thread, the requirement is that we need “will-change: transform” for the paint worklet element. Without that, the custom property animation will run on the main thread, such as this example: https://output.jsbin.com/muwiyux/quiet. This CL makes changes such that a custom property animation will always be composited as long as it is used by paint worklet, even if the element doesn't have "will-change: transform". The change is actually small, there are only two things we need: 1. Start the animation on compositor. 2. Ensure the compositor ticks the animation. For #1, we add a "has_paint_worklet_with_custom_prop_anim" in the Animation::PreCommit, when it is true, we always composite the animation. For #2, we give a special ElementId which is uint64_t::max() to the paint worklet element, and on the CC side, once we see that element id, we know that the animation associated with that should be ticking even if the element id doesn't have anything associated on the property tree. Bug: 987969 Change-Id: Ia849640065470e529a2b8d23a4b7b74339831c48 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2359370 Reviewed-by: Robert Flack <flackr@chromium.org> Reviewed-by: Kevin Ellis <kevers@chromium.org> Commit-Queue: Xida Chen <xidachen@chromium.org> Cr-Commit-Position: refs/heads/master@{#812056}

This change: 1) Exposes as static synchronous helpers the WebCodecs MakeMediaConfig methods' internals so that MediaSource can use them to obtain media::{Audio,Video}DecoderConfigs from WebCodecs {Audio,Video}DecoderConfigs. 2) Implements MediaSource::AddSourceBufferUsingConfig for encoded configs (not decodedMediaType), plumbing the media decoder config through the usual steps in MSE addSourceBuffer, and through new methods in WebMediaSource and WebMediaSourceImpl to let ChunkDemuxer::AddId know it will be expected to handle WebCodecs encoded chunk appends for the associated SourceBuffer. 3) Adds a tentative folder for MSE-for-WebCodecs web_tests and adds a test for the current behavior of addSourceBuffer(webcodecs decoder config) that this CL adds. Later changes will continue plumbing of the configs in ChunkDemuxer and SourceBufferState, and also add implementations and ChunkDemuxer/etc support for appendEncoded{Audio,Video}Chunks. The h264 parser and avcc conditionally created in #1, above, could then be used by SourceBuffer processing of appended EncodedVideoChunks, too. BUG=1144908 Change-Id: I90c1d90c3a28d5cc1e33b1e50e32f4cfea639784 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2548844 Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Chrome Cunningham <chcunningham@chromium.org> Reviewed-by: Dan Sanders <sandersd@chromium.org> Cr-Commit-Position: refs/heads/master@{#835058}

Currently, the CheckCanStartAnimationOnCompositor is called twice for composite background-color animation, once during paint time and once during the compositing stage. The reason is that we need the decision during paint and compositing consistent. That is, if the paint stage says the background color must be paint on the main thread, then compositing stage has to agree with that, and vice versa. However, this is dangerous because between the paint and compositing stage, things could change, especially the PaintArtifactCompositor, which is used by the CheckCanStartAnimationOnCompositor. For example, it could happen that at paint time we have not produced / updated the property nodes for the current frame and we can make decision based on what was composited on the previous frame. Then at Precommit we have potentially updated / added / removed property tree nodes. In this case, the return value of CheckCanStartAnimationOnCompositor can be different, as a result, the background color animation won't run correctly. The reason we needed to know whether the animation could be composited here is that we didn't have a way to paint the background color off the main thread. More specifically, the BackgroundColorPaintWorklet::Paint() function can paint the background color only if the animation is running on the compositor thread. This CL makes following changes: 1. Make the BackgroundColorPaintWorklet::Paint() have the ability to paint the background color even if the animation is running on the main thread. The function needs two things: the current progress of the animation and the artifacts about the animation. So all we need is just getting the progress when the animation is running on the main thread. 2. With #1 being done, we no longer need to call the CheckCanStartAnimationOnCompositor during the paint step. As a result, whether or not the animation can be running on the compositor thread is solely the decision during the compositing stage. This is much safer than the current code, because we no longer need to make a compositing decision during the paint stage. We don't need to add any new tests because we already have sufficient layout tests for background color animation being run on the compositor as well as on the main thread. As long as all tests pass, this should be safe. The main benefit of this change is that the code is now more robust, meaning that we don't need to worry about the decision made by the paint and compositing stage being different. This change is also a performance win because we no longer need to call the CheckCanStartAnimationOnCompositor twice. Bug: 1185272, 1182261 Change-Id: Ie072714fd1d05e6537e05cad45ad1da99e20125b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2740697 Reviewed-by: Robert Flack <flackr@chromium.org> Commit-Queue: Xida Chen <xidachen@chromium.org> Cr-Commit-Position: refs/heads/master@{#863622}

…eb-platform-tests#28617) Subresource Web Bundles. The problem is: when Web Bundle fetching fails due to a network error, Subresource fetch doesn't fail forever. One such case (subresource-loading-cors-error test) was timing out previously but passes successfully with this change. This CL also adds 2 WPT tests: 1. subresource-loading-network-error.https.tentative.sub.html 2. subresource-loading-web-bundle-fetch-failed.https.tentative.html Test #1 is a scenario with a different network error than the CORS one, but with the same issue of subresource fetching timing out without the change. It passes successfully after the change. Test #2 is a scenario with a Web bundle not found error, which is not directly influenced by the code added in this CL, but it expands the test coverage which was found to be lacking the error cases before. Bug: 1168449 Change-Id: Ia3abb967e36274becc86e317bc51b1272d3ae679 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2826001 Reviewed-by: Tsuyoshi Horo <horo@chromium.org> Reviewed-by: Hayato Ito <hayato@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Commit-Queue: Miras Myrzakerey <myrzakereyms@google.com> Cr-Commit-Position: refs/heads/master@{#875532} Co-authored-by: Miras Myrzakerey <myrzakereyms@google.com>

1. Only process ChildrenChanged() for the included root of a change. For example, if a <div id="root" style="display:none"> will be included because it is a potential relation target. If descendants change, the only ChildrenChanged() necessary to process is on #root. 2. Share common code for detaching a node and queuing up the appropriate children changes. This simplifies ProcessInvalidatedObjects() by removing one of the inner loops, and enables a follow-up CL to remove the outer loop as well. #1 results in a massive speedup for display none toggles. In combination with other recent changes in DetachAndRemoveFromChildrenOfAncestors(), is 7x faster for many-nodes-toggle-display-none in perf_tests . This change alone accounts for about half of the overall improvement. Follow-ups: - Restore lifecycle check by processing deferred children changes via nodes_with_pending_children_changed_ and not queuing via the traditional mechanism. While doing this, look for opportunities to consolidate more children changed events. - Remove outer loop from ProcessInvalidatedObjects(). Bug: None Change-Id: I80466fda792cd0ca6dd051065a42ba702e4cc8b1 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2946971 Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org> Commit-Queue: Aaron Leventhal <aleventhal@chromium.org> Cr-Commit-Position: refs/heads/master@{#891343}

1. Use GetWithoutInvalidation() instead of Get() in DCHECKs. We should never call Get() inside of a DCHECK(), because this can lead to a different code path depending on whether DCHECKs are enabled. 2. Get() should not cause immediate side effects. At most, it should queue up an invalidation for later processing. Fixing #1 and #2 were required in order to get past a first set of errors introduced by the new test. 3. The actual fix -- avoid infinite loop by calling a special new SlotAssignmentWillChange(), rather than ChildrenChanged(), where a minimal GetWithoutInvalidation() is called that does not lead to IsShadowContentRelevantForAccessibility() => FirstChild() => RecalcAssignedNodes() => ChildrenChanged() ... (infinite loop). A simpler potential fix is in CL:2965317 but requires more research. It's also mentioned in a TODO comment. Bug: 1219311 Change-Id: Iafaa289f241a851404ce352715d2970172a2e5f8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2961158 Reviewed-by: Joey Arhar <jarhar@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org> Commit-Queue: Aaron Leventhal <aleventhal@chromium.org> Cr-Commit-Position: refs/heads/master@{#892778}

Relative offsets should be applied after fragmentation. Since we perform layout for OOFs once they reach the fragmentation context root (if applicable), we fail to apply any relative offsets at the correct time in the case of inline containing blocks. See CL:2851595 for how this was handled for the non-inline case. The changes required to accomplish this for inline containing blocks include: 1. We currently store an accumulated relative offset in NGContainingBlock inside the OOF node, which is any relative offset from the containing block to the fragmentation context root. We also need to store an accumulated relative offset from the inline container to the containing block in order to properly apply all relative offsets at the time of fragmentation. A new struct, NGInlineContainer, was added to the OOF node to hold the inline container object and the accumulated relative offset to the containing block. 2. A relative offset was also added to the InlineContainingBlockGeometry struct so that we have access to the relative offset from #1 when creating the ContainingBlockInfo for the inline container. 3. The way that relative offsets are applied to inlines, it didn't seem straightforward to separate the relative offset from the normal offset, like we had in CL:2851595. Instead, store the relative offset for the inline and subtract it out from the OOF static position once it reaches the containing block, and subtract it from the containing block rect offset when determining the ContainingBlockInfo for the inline container. 4. Store the total relative offset (from the inline container to the fragmentation context root) in ContainingBlockInfo. This relative offset will then be applied after fragmentation is complete for the OOF as a result of CL:2851595. Bug: 1079031 Change-Id: I7198fec4c01e05ca54c51b2f215569b75b0b869e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2995308 Commit-Queue: Alison Maher <almaher@microsoft.com> Reviewed-by: Morten Stenshorne <mstensho@chromium.org> Reviewed-by: Koji Ishii <kojii@chromium.org> Cr-Commit-Position: refs/heads/master@{#902060}

This patch adds a new produceCropId() API to mediaDevices. This API is called with a DIV or IFRAME element, and adds a new base::UnguessableToken value to that element's rare data structure. This token value will be used in followup patches in order to keep track of an element's location in the page and viewport. Based on the following design document: https://docs.google.com/document/d/1dULARMnMZggfWqa_Ti_GrINRNYXGIli3XK9brzAKEV8/ Bug: 1247761 Change-Id: I01cd67e2d4e3dfa7a86289f876e48c8b55095d0a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3173396 Commit-Queue: Jordan Bayles <jophba@chromium.org> Reviewed-by: Elad Alon <eladalon@chromium.org> Reviewed-by: mark a. foltz <mfoltz@chromium.org> Reviewed-by: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/main@{#925544}

…eVisibilityKeeper::PrepareToSplitBlockElement()` before splitting a text node It does the following things when caret is collapsed in a text node in a `<p>` or `<div>` element. 1. Split the text node containing caret to insert `<br>` element 2. Insert `<br>` element after it 3. Split ancestor elements which inclusive descendants of the `<p>` or `<div>` 4. Delete the `<br>` element if unnecessary from the left paragraph #3 and #4 are performed by `HTMLEditor::SplitParagraph()` and it calls `WhiteSpaceVisibilityKeeper::PrepareToSplitBlockElement()` correctly before splitting the block. However, in the case (caret is at middle of a text node), the text has already been split to 2 nodes because of #1. Therefore, it fails to handle to keep the white-space visibility. So that I believe that the root cause of this bug is, the method does much complicated things which are required, and doing the redundant things will eat memory space due to undo transactions. However, for now, I'd like to fix this with a simple patch which just call the preparation method before splitting the text node because I'd like to uplift this if it'd be approved (Note that this is not a recent regression, the root cause was created by bug 92686 which was fixed in 17 years ago: <https://searchfox.org/mozilla-central/commit/2e66280faef73e9be218e00758d4eb738395ac83>, but must be annoying bug for users who see this frequently). The new WPTs are pass in Chrome. Differential Revision: https://phabricator.services.mozilla.com/D130950 bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1740416 gecko-commit: 73567f6c2bcfa078836a36760498bb11747561dd gecko-reviewers: m_kato, smaug

Generalized Test

a325a29

mariestaver force-pushed the lu-urltojson branch from c91e033 to a325a29 Compare June 13, 2018 19:52

mariestaver merged commit e63c6a1 into liberate-url Jun 13, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generalized Test #1

Generalized Test #1

mariestaver commented Jun 13, 2018 •

edited

Loading

Generalized Test #1

Generalized Test #1

Conversation

mariestaver commented Jun 13, 2018 • edited Loading

mariestaver commented Jun 13, 2018 •

edited

Loading