Skip to content

hi#1

Closed
jdm wants to merge 7 commits intomasterfrom
servo_export_18746
Closed

hi#1
jdm wants to merge 7 commits intomasterfrom
servo_export_18746

Conversation

@jdm
Copy link
Owner

@jdm jdm commented Oct 4, 2017

hi

foolip and others added 7 commits October 4, 2017 06:33
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.
Done by listing all files using setTimeout and manually makes
reasonably-sized groups.
@jdm jdm closed this Oct 4, 2017
@jdm jdm deleted the servo_export_18746 branch October 4, 2017 15:01
jdm pushed a commit that referenced this pull request Nov 10, 2017
Synchronize with original
jdm pushed a commit that referenced this pull request Jan 31, 2018
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 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
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}
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}
jdm pushed a commit that referenced this pull request Sep 15, 2019
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}
jdm pushed a commit that referenced this pull request Sep 15, 2019
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}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants