| Summary: | Re-enable WebFrameTest.DivScrollIntoEditableTest | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | WebKit Review Bot <webkit.review.bot> | ||||||||||
| Component: | New Bugs | Assignee: | Adam Barth <abarth> | ||||||||||
| Status: | RESOLVED WONTFIX | ||||||||||||
| Severity: | Normal | CC: | abarth, buildbot, keishi, rniwa, wangxianzhu | ||||||||||
| Priority: | P2 | ||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| Bug Depends on: | 108166 | ||||||||||||
| Bug Blocks: | |||||||||||||
| Attachments: |
|
||||||||||||
|
Description
WebKit Review Bot
2012-10-05 14:36:23 PDT
Created attachment 185153 [details]
Patch
Created attachment 185154 [details]
Patch (comments updated)
The original comment above the function read "This test depends on code that is compiled conditionally." I wonder what the macro is and what the original problem was. I don't remember, but re-enabling it is likely to just cause the same failures. Perhaps we should run a try job to try to figure out what the issue was? I modified the test at several places to make it pass in my environment locally (chromium-linux). Will cr-linux ews run webkit_unit_tests? > Will cr-linux ews run webkit_unit_tests?
Yes, but only on linux. Do you know if it passes on cr-android as well?
Tried the test with the patch on chromium-android. It passed. The tested code is all in WebViewImpl::scrollFocusedNodeIntoRect(). The conditionally compiled code seems not to cause problem now, given that the test has been modified to be consistent with the current code. Comment on attachment 185154 [details]
Patch (comments updated)
OK. Let's land this and see what happens!
Comment on attachment 185154 [details] Patch (comments updated) Clearing flags on attachment: 185154 Committed r141064: <http://trac.webkit.org/changeset/141064> All reviewed patches have been landed. Closing bug. Re-opened since this is blocked by bug 108166 Test seems to be failing on WinXP so I am rolling the patch out. http://chromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20XP/builds/2419/steps/webkit_unit_tests/logs/DivScrollIntoEditableTest WebFrameTest.DivScrollIntoEditableTest: tests\WebFrameTest.cpp(974): error: Value of: webView->mainFrame()->scrollOffset().width Actual: 494 Expected: hScroll Which is: 495 Comment on attachment 185154 [details] Patch (comments updated) Attachment 185154 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/16151142 New failing tests: http/tests/inspector/appcache/appcache-manifest-with-non-existing-file.html Created attachment 185256 [details]
Use EXPECT_NEAR to tolerate inaccuracy
Comment on attachment 185256 [details]
Use EXPECT_NEAR to tolerate inaccuracy
Just found that though the changed test passes, it misses a case to test. Will update the patch soon.
Created attachment 185264 [details]
Patch
The test will be rewritten in bug 107599. We are obsoleting webkit scaling so we don't need the original test any more. |