Bug 85692 - Convert isPageBoxVisible to use Internals interface
Summary: Convert isPageBoxVisible to use Internals interface
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gyuyoung Kim
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-04 19:38 PDT by Gyuyoung Kim
Modified: 2012-05-06 17:38 PDT (History)
5 users (show)

See Also:


Attachments
Patch (30.00 KB, patch)
2012-05-04 20:51 PDT, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (30.01 KB, patch)
2012-05-04 23:51 PDT, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (31.19 KB, patch)
2012-05-05 06:53 PDT, Gyuyoung Kim
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gyuyoung Kim 2012-05-04 19:38:44 PDT
Adjust isPageBoxVisible tests to use Internals instead of LayoutTestController interface. In my humble opinion, isPageBoxVisible() is able to be supported by Internals. Because, it looks this function is implemented by WebCore layer, not WebKit layer. I'd like to know how do reviewers think about this patch.
Comment 1 Gyuyoung Kim 2012-05-04 20:51:19 PDT
Created attachment 140379 [details]
Patch
Comment 2 Early Warning System Bot 2012-05-04 21:05:51 PDT
Comment on attachment 140379 [details]
Patch

Attachment 140379 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12626348
Comment 3 WebKit Review Bot 2012-05-04 21:17:23 PDT
Comment on attachment 140379 [details]
Patch

Attachment 140379 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12543527
Comment 4 Build Bot 2012-05-04 21:20:05 PDT
Comment on attachment 140379 [details]
Patch

Attachment 140379 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12602355
Comment 5 Build Bot 2012-05-04 21:47:04 PDT
Comment on attachment 140379 [details]
Patch

Attachment 140379 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12531360
Comment 6 Early Warning System Bot 2012-05-04 22:10:10 PDT
Comment on attachment 140379 [details]
Patch

Attachment 140379 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12626362
Comment 7 Gyuyoung Kim 2012-05-04 23:51:09 PDT
Created attachment 140382 [details]
Patch
Comment 8 Build Bot 2012-05-05 00:18:34 PDT
Comment on attachment 140382 [details]
Patch

Attachment 140382 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12507439
Comment 9 Gyuyoung Kim 2012-05-05 06:53:37 PDT
Created attachment 140397 [details]
Patch
Comment 10 Darin Adler 2012-05-05 17:22:34 PDT
Comment on attachment 140397 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=140397&action=review

> Tools/DumpRenderTree/mac/LayoutTestControllerMac.mm:-315
> -    return [mainFrame isPageBoxVisible:pageNumber];

I can’t speak for the other ports, but for the Mac port, the isPageBoxVisible: method exists only to support this DumpRenderTree feature. So we should also be removing that method from WebKit entirely.
Comment 11 WebKit Review Bot 2012-05-06 15:42:33 PDT
Comment on attachment 140397 [details]
Patch

Clearing flags on attachment: 140397

Committed r116246: <http://trac.webkit.org/changeset/116246>
Comment 12 WebKit Review Bot 2012-05-06 15:42:41 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 Andy Estes 2012-05-06 17:29:13 PDT
This patch broke the build on my machine due to leaving behind an unused function. I removed the function in <http://trac.webkit.org/changeset/116252>.
Comment 14 Gyuyoung Kim 2012-05-06 17:38:22 PDT
(In reply to comment #13)
> This patch broke the build on my machine due to leaving behind an unused function. I removed the function in <http://trac.webkit.org/changeset/116252>.

Thank you for your fix for build break.