PARTIAL: Begin pull request task #3

jugglinmike · 2018-08-14T21:35:38Z

No description provided.

ghost · 2018-08-14T21:35:39Z

Submitting the task to Taskcluster failed. Details

InterpreterError at template.tasks[1].metadata.name: unknown context value chunk

ghost · 2018-08-14T21:40:23Z

Submitting the task to Taskcluster failed. Details

InterpreterError at template.tasks[1].metadata.name: unknown context value chunk

ghost · 2018-08-14T21:41:18Z

Submitting the task to Taskcluster failed. Details

InterpreterError at template.tasks[1].metadata.owner: object has no property pusher

ghost · 2018-08-14T21:45:41Z

Submitting the task to Taskcluster failed. Details

InterpreterError at template.tasks[1].metadata.owner: object has no property pusher

ghost · 2018-08-14T21:47:24Z

Submitting the task to Taskcluster failed. Details

InterpreterError at template.tasks[1].payload.command[3]: object has no property ref

ghost · 2018-08-15T01:57:42Z

Submitting the task to Taskcluster failed. Details

InterpreterError at template.tasks[1].payload.command[3]: unknown context value command

This simplification is intended to support a future extension to validate pull requests.

ghost · 2018-08-20T21:03:23Z

Submitting the task to Taskcluster failed. Details

expected hexadecimal character at line 133, column 58:
... ffected | base64 -d | sed 's/\x0/\n/g';
^

ghost · 2018-08-20T21:05:55Z

Submitting the task to Taskcluster failed. Details

expected hexadecimal character at line 133, column 58:
... ffected | base64 -d | sed 's/\x0/\n/g';
^

This turned out to be unnecessary

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

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}

…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}

…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

jugglinmike force-pushed the taskcluster-stability branch from 1b1c0fc to a3aea8f Compare August 14, 2018 21:47

jugglinmike force-pushed the taskcluster-stability branch from dcbe2b4 to 7ba3043 Compare August 15, 2018 01:59

jugglinmike added 21 commits August 16, 2018 11:57

[tools] Use more descriptive script name

8ef57a5

[tools] Generalize repository cloning procedure

f7f3019

This simplification is intended to support a future extension to validate pull requests.

TEMPORARY Use de-privileged worker type

2466423

Refactor

71e3c10

Try to get TaskCluster to fetch new image

f687d73

Just use a new tag

a4afafe

Correct fetch command

9f42fe5

Fix things

8a22600

PARTIAL: Begin pull request task

56bae6c

Correct array generation

d9640c5

Correct task name

e7c34f1

Correct event reference

c37e6d2

Correct invocation of provisioning script

4c90457

Correct "owner" e-mail (as per TaskCluster docs)

9859d25

Modify a test

a190a95

Refactor

03a8732

Corret task IDs

225001f

Correct command

aeca034

Lazy commit message

20885b2

Integrate new feature

ef19dee

Restructure artifact specification

49127dd

jugglinmike force-pushed the taskcluster-stability branch from 0064a9b to 49127dd Compare August 16, 2018 21:01

jugglinmike added 7 commits August 16, 2018 21:32

Inspect command output

72408a6

Ensure base revision is available

9caed66

Restructure code retrieval logic

2901c0d

Add statement delimiters

6f788bb

Specify URL of git remote

79bdc96

Refactor

8f17efc

Disable builds for GitHub push events

de7af72

jugglinmike force-pushed the master branch from 452fbcb to 7dbb4b6 Compare August 20, 2018 21:02

Escape escape

a75cd46

Reformat

af1f51b

jugglinmike force-pushed the taskcluster-stability branch from f372451 to af1f51b Compare August 20, 2018 21:14

jugglinmike added 8 commits August 20, 2018 17:34

Correct syntax

bbf61eb

Correct argument ordering

6a3a995

Download manifest file

c2dcd4f

Introduce instability

c7712d6

Revert to original "start" script

a02cc9e

This turned out to be unnecessary

Only attempt to gzip files that are present

5f12938

Reformat

4d9ba3a

Correct browser detection logic

87ce5f3

jugglinmike force-pushed the master branch 2 times, most recently from a19a236 to 6ae38c9 Compare August 23, 2018 17:58

jugglinmike force-pushed the master branch from 6ae38c9 to e570d8f Compare August 31, 2018 01:35

jugglinmike force-pushed the master branch from dba6e62 to da7e173 Compare October 16, 2018 17:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PARTIAL: Begin pull request task #3

PARTIAL: Begin pull request task #3

jugglinmike commented Aug 14, 2018

ghost commented Aug 14, 2018

ghost commented Aug 14, 2018

ghost commented Aug 14, 2018

ghost commented Aug 14, 2018

ghost commented Aug 14, 2018

ghost commented Aug 15, 2018

ghost commented Aug 20, 2018

ghost commented Aug 20, 2018

PARTIAL: Begin pull request task #3

Are you sure you want to change the base?

PARTIAL: Begin pull request task #3

Conversation

jugglinmike commented Aug 14, 2018

ghost commented Aug 14, 2018

ghost commented Aug 14, 2018

ghost commented Aug 14, 2018

ghost commented Aug 14, 2018

ghost commented Aug 14, 2018

ghost commented Aug 15, 2018

ghost commented Aug 20, 2018

ghost commented Aug 20, 2018