-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add device posture test driver and web platform tests #4
base: DevicePosture
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are programming issues in the converted tests that must be fixed. I've added inline comments pointing them out.
You should also use EventWatcher everywhere but the one test that wants to exercise addEventListener behavior.
Missing newline from tools/wptrunner/wptrunner/executors/protocol.py. Remove comma from resources/testdriver.js. Unused commented code in device-posture/device-posture.https.html.
Removed the requirement from all tests to set up cleanup function. All tests are now wrapped around device_posture_test call. This enables that adding of cleanup function for each test is done in one location.
Two tests where similar, so other was delete.
EventWatcher promise was waited with mixed style. Code was fixed to use only one style of waiting. After change EventWatcher eventTypes parameter value needed to change also to able to run test correctly.
Also added rest of the parameters which display-feature needs.
…trunner/executors/executormarionette.py
…r.py Every other class name has "Protocol" word
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if you were waiting for my review, but I've added some comments about things that need to be fixed in addition to anything else that was already on your list.
Event type was changed from 'onchange' to 'change'. This led change in waiting for event. Which solved the problem that 'continuous' posture had to be the first posture.
Removed because other tests already had same code and read posture through navigator.devicePosture.type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing that's just occurred to me while working on the content_shell CL upstream: all these tests that call set_device_posture() need a
t.add_cleanup(async () => {
await test_driver.clear_device_posture();
});
call to make sure nothing leaks between tests.
Since @page border box layout objects aren't in the the layout tree, any code that wants to walk up the tree to find the containing block will be in for a surprise. This would happen if percentage-based @page padding was used [1]. Recomputing padding during painting when we have already done it during layout is rather pointless anyway. Read it out directly from the fragment. [1] #1 blink::LayoutBox::ContainingBlockLogicalWidthForContent() #2 blink::LayoutBoxModelObject::ComputedCSSPadding() #3 blink::LayoutBoxModelObject::PaddingTop() #4 blink::LayoutBoxModelObject::PaddingOutsets() web-platform-tests#5 blink::BoxPainterBase::PaintFillLayer() web-platform-tests#6 blink::BoxPainterBase::PaintFillLayers() web-platform-tests#7 blink::BoxFragmentPainter::PaintBackground() Bug: 40286153 Change-Id: I1e6e92c2ce1d81aab2673ec9a877eac455534102 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5526469 Commit-Queue: Morten Stenshorne <[email protected]> Reviewed-by: Xianzhu Wang <[email protected]> Reviewed-by: Ian Kilpatrick <[email protected]> Cr-Commit-Position: refs/heads/main@{#1300711}
No description provided.