Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebKit export of https://bugs.webkit.org/show_bug.cgi?id=190642 #13606

Closed

Conversation

woaiwyhty
Copy link

This was written based on a previous version of the spec, and is no
longer correct.

css/css-text/overflow-wrap/overflow-wrap-break-word-002.html checks the
correct behavior instead.

Reported in w3c/csswg-drafts#3221
video.getVideoTracks()[0].stop();
t.step_timeout(() => {
assert_unreached("stop event is not fired after 2 seconds");
}, 2000);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally, we prefer just letting the test time out rather than adding an explicit timeout like this.

@woaiwyhty woaiwyhty force-pushed the wpt-export-for-webkit-190642 branch from e8a522c to 747d4a2 Compare October 19, 2018 16:40
@woaiwyhty woaiwyhty force-pushed the wpt-export-for-webkit-190642 branch from 747d4a2 to 1cee72e Compare October 19, 2018 23:54
emilio and others added 22 commits October 20, 2018 02:20
Reduce text-sizing dependency & use clearer colors in reftest flex-flexitem-percentage-prescation.html
Differential Revision: https://phabricator.services.mozilla.com/D9032

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1499863
gecko-commit: 2dd8df3604ef3a4130daea876a25efe6db16536f
gecko-integration-branch: autoland
gecko-reviewers: dbaron
This is a reland of 02cb80e67052e59d324ae1f78d2c94c63da52939

The issues seems to have been the test case {'a', 'b', 'b'} where
apparently the failed navigation still fires a load event which might
arrive sooner than fallback update. The reland modifies the test file
to suppress sending 'OBJECT_LOAD' event from the test page.

Original change's description:
> Implement fallback content for RemoteFrameOwner
>
> RemoteFrameOwner does not implement the logic for fallback content. This
> means if a cross-origin navigation fails with some error, the owner
> element (in this context, <object>) in the parent process does not get
> notified and will not use its fallback content (instead the frame might
>  show an error page).
>
> When the <object> has fallback content, it should always use that over
> the frame's error message. To support this matter, this CL implements
> fallback methods in RemoteFrameOwner. Essentially,
>
>   * When <object> creates a local frame, the corresponding frame tree
> node will be marked as the type that "can" render fallback content. This
> will propagte everywhere using FrameReplicationState.
>
>   * When the provisional loading of a frame fails, RemoteFrameOwner will
> notify the browser through the current proxy for navigation. The browser
> then uses the parent frame to notify the renderer process that the owner
> element should render its own fallback content.
>
>   * If the <object> does not specify fallback, the navigation to error
> page commits and the error page is shown.
>
>   * When the <object> renders its own content, the remote frame stays
> alive. This is a bug but not a new one; It will be fixed in future CLs.
>
> Bug: 853140
> Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
> Change-Id: Icad3934ccfc1823c0cdecd8e1223e6370ea4b3bb
> Reviewed-on: https://chromium-review.googlesource.com/c/1105635
> Commit-Queue: Ehsan Karamad <[email protected]>
> Reviewed-by: Daniel Cheng <[email protected]>
> Reviewed-by: Ehsan Karamad <[email protected]>
> Reviewed-by: Wei Li <[email protected]>
> Reviewed-by: Alex Moshchuk <[email protected]>
> Cr-Commit-Position: refs/heads/master@{#601194}

[email protected],[email protected],[email protected]

Bug: 853140
Change-Id: Ib1594aa642ea238f745fd29d191818fddc503afc
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Reviewed-on: https://chromium-review.googlesource.com/c/1292899
Reviewed-by: Ehsan Karamad <[email protected]>
Commit-Queue: Ehsan Karamad <[email protected]>
Cr-Commit-Position: refs/heads/master@{#601411}
This CL converts the current "animations" feature policy to
"layout-animations" feature policy: the former policy is about blocking
non-composited animations whereas the new policy only blocks a handful
of CSS animations which are known to (potentially) cause re-layout and
are also widely used. The current set of CSS properties considered in
the new policy is: {bottom, height, left, right,top, width}.

Bug: 874218
Change-Id: I97f08ce6b2b902c7057ea821416a9c145d26f068
Reviewed-on: https://chromium-review.googlesource.com/c/1235055
Commit-Queue: Ehsan Karamad <[email protected]>
Reviewed-by: Jeremy Roman <[email protected]>
Reviewed-by: Robert Flack <[email protected]>
Reviewed-by: Ken Buchanan <[email protected]>
Reviewed-by: Ian Clelland <[email protected]>
Cr-Commit-Position: refs/heads/master@{#601412}
…13616)

While inlining the tests into the multicol spec, I've been adding the missing assert statements. This is a set of those.
In preparation for running these tests in CI.

Verified locally with Safari Technology Preview 67:
```
export no_proxy='*'
./wpt run --metadata infrastructure/metadata/ --channel=preview safari infrastructure/
./wpt run --metadata infrastructure/metadata/ --channel=preview safari_webdriver infrastructure/
```

With Safari 12 there are still failures, but we'll not use that in CI.
The stop() function is apparently being called multiple times at the end of a test run, causing us to sometimes attempt to remove device forwards that were removed in an earlier stop() attempt but before the conditions checked in stop() to remove the device forwards get changed. This caused a traceback to be printed out at the end of every test run, complaining about attempts to remove device forwards that don't exist.

Rather than dig through to figure out why/where stop() is getting called twice, this patch just papers over the issue by making sure there is at least one device forward still listed before attempting to remove them.

Differential Revision: https://phabricator.services.mozilla.com/D9175

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1499889
gecko-commit: 6ff410edad90dbc51d72aeb722d0ff8d182606c1
gecko-integration-branch: autoland
gecko-reviewers: gbrown
Calling `match` was causing crashes in debug mode due to accessing
base::Optional<>::value without initialization. Calling match with
an unmatched request was crashing due to hitting a DCHECK. This returns
undefined now as per the spec.

Bug: 896768
Change-Id: I5d82e68ddca157a240ceaaec8990d2553366fbbb
Reviewed-on: https://chromium-review.googlesource.com/c/1289269
Commit-Queue: Rayan Kanso <[email protected]>
Reviewed-by: Peter Beverloo <[email protected]>
Cr-Commit-Position: refs/heads/master@{#601541}
This was made possible by crrev.com/c/1283482, which avoids the
performance and correctness issues this previously caused.

This relands crrev.com/c/1246730 / crrev.com/c/1252682. The ChromeOS UI
breakage was fixed by crrev.com/c/1281726 and crrev.com/c/1282462

IF THIS BREAKS ANY FURTHER CHROME UI:
Please don't revert this patch; instead, add min-height: 0 to any
inner nested flexboxes that may be affected by this patch. This is
an important change for web interop with the other browsers.

Bug: 596743
Change-Id: I9afe54dce82d41da452d1fdca8150ca22ebb6f9c
Reviewed-on: https://chromium-review.googlesource.com/c/1269235
Commit-Queue: Christian Biesinger <[email protected]>
Reviewed-by: Morten Stenshorne <[email protected]>
Cr-Commit-Position: refs/heads/master@{#601540}
A callback defined in Web IDL should take a nullable argument. We
don't enforce this in our bindings code and it can't be introspected
from JS so this is not a testable or web-facing change, just aligning
the code with a spec fix.

Spec issue: w3c/web-locks#51
Spec link: https://wicg.github.io/web-locks/#callbackdef-lockgrantedcallback

Change-Id: Icc2e076f62e45629a283d0039e2be30b4a6826c4
Reviewed-on: https://chromium-review.googlesource.com/c/1294421
Commit-Queue: Joshua Bell <[email protected]>
Commit-Queue: Victor Costan <[email protected]>
Reviewed-by: Victor Costan <[email protected]>
Cr-Commit-Position: refs/heads/master@{#601716}
Properties inherit or not according to the spec.
Properties have initial values according to the spec.
https://drafts.csswg.org/cssom-view/#property-index
While the HTTP spec further limits what values are legal, nulls are
particularly concerning, and it's safest just to reject them. See
discussion here: whatwg/xhr#165

Chrome will be the first browser to reject nulls in responses, despite
there being wpt tests for this, so we'll have to keep an eye out for
breakages.

For reference, 0x00 through 0x1F aren't allowed in header values or
fields, (https://tools.ietf.org/html/rfc7230#section-3.2 - VCHAR
excludes those characters).  CRs and LFs are of course needed, and
0x0C and 0x0B are allowed by other specs for particular
header parsers, strangely.

This CL does not affect other code that can generate HTTP response
headers, which still uses the old behavior of just removing nulls.
ServiceWorkers, extensions, WebPackages, Dial (?), and various tests
still inherit the old behavior, since they create headers directly
with a method that can't fail.  It does introduce a new helper method,
however, that they should eventually be switched to use:
HttpResponseHeaders::TryToCreate().  We should probably put off
conversion until this successfully makes it to stable.

Bug: 832086
Change-Id: Ib75ac03a6a298238cafb41eaa5f046c082fd0bdf
Reviewed-on: https://chromium-review.googlesource.com/c/1291812
Reviewed-by: Asanka Herath <[email protected]>
Commit-Queue: Matt Menke <[email protected]>
Cr-Commit-Position: refs/heads/master@{#601776}
Bug: 895723
Change-Id: If7918de7e3717b01619ee21fa87eeaffb971287c
Reviewed-on: https://chromium-review.googlesource.com/c/1292661
Reviewed-by: Kouhei Ueno <[email protected]>
Reviewed-by: Tsuyoshi Horo <[email protected]>
Commit-Queue: Kunihiko Sakamoto <[email protected]>
Cr-Commit-Position: refs/heads/master@{#601828}
This is a reland of 254369a5f6df06c2c6be067d14c2cb2a036ba173. It addresses bug
888025 by adding ASAN test expectations, as the relevant V8 feature does not
yet support running on ASAN builds.

Original change's description:
> bindings: Implement timers with V8Function
>
> This fixes bug 866610 by using the IDL infrastructure to properly enter
> the v8::Context before calling the registered callback.
>
> Also ensure eager finalization of ScheduledAction in DOMTimer to
> prevent a memory leak. Added two more effective DCHECKs to confirm.
>
> Bug: 866610
> Change-Id: I37d7bd05f035fe31856cfe68bae51aa0632cd3b1
> Reviewed-on: https://chromium-review.googlesource.com/1220486
> Reviewed-by: Nate Chapin <[email protected]>
> Reviewed-by: Yuki Shiino <[email protected]>
> Reviewed-by: Hitoshi Yoshida <[email protected]>
> Commit-Queue: Timothy Gu <[email protected]>
> Cr-Commit-Position: refs/heads/master@{#593108}

[email protected]

Bug: 866610, 888025
Change-Id: Iee5c1d6917ad7770383e06a425f96000835a663a
Reviewed-on: https://chromium-review.googlesource.com/c/1239624
Reviewed-by: Nate Chapin <[email protected]>
Reviewed-by: Hitoshi Yoshida <[email protected]>
Reviewed-by: Kentaro Hara <[email protected]>
Reviewed-by: Yuki Shiino <[email protected]>
Commit-Queue: Timothy Gu <[email protected]>
Cr-Commit-Position: refs/heads/master@{#601830}
@foolip
Copy link
Member

foolip commented Oct 24, 2019

Now that web-platform-tests/wpt-pr-bot#38 is resolved I can (hopefully) explain what happened here without the bot adding back all the reviewers and labels.

On this PR, a rebase or merge went wrong and lots of files were considered affected. wpt-pr-bot labeled and added reviewers accordingly. The issue of >100 labels was addressed in web-platform-tests/wpt-pr-bot#35, but things could still go wrong like this again.

Anyway, no action is needed here, because the changes were landed in another PR already.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.