Bug 72195 - [chromium] Expose mock scrollbars to window.internals
Summary: [chromium] Expose mock scrollbars to window.internals
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adrienne Walker
URL:
Keywords:
Depends on:
Blocks: 132085
  Show dependency treegraph
 
Reported: 2011-11-11 16:16 PST by Adrienne Walker
Modified: 2014-04-23 15:44 PDT (History)
9 users (show)

See Also:


Attachments
Patch (12.38 KB, patch)
2011-11-11 16:20 PST, Adrienne Walker
no flags Details | Formatted Diff | Diff
Now with more exports (15.68 KB, patch)
2011-11-11 18:39 PST, Adrienne Walker
no flags Details | Formatted Diff | Diff
Patch (16.37 KB, patch)
2011-11-12 12:40 PST, Adrienne Walker
no flags Details | Formatted Diff | Diff
Fourth try's a charm (18.42 KB, patch)
2011-11-13 10:34 PST, Adrienne Walker
no flags Details | Formatted Diff | Diff
Oops. Don't export the wrong symbol from WebKit2.def, causing a linker error in WebKit.dll (15.76 KB, patch)
2011-11-15 09:40 PST, Adrienne Walker
no flags Details | Formatted Diff | Diff
Rebaselined (15.00 KB, patch)
2011-11-15 09:48 PST, Adrienne Walker
jamesr: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adrienne Walker 2011-11-11 16:16:07 PST
[chromium] Expose mock scrollbars to window.internals
Comment 1 Adrienne Walker 2011-11-11 16:20:28 PST
Created attachment 114797 [details]
Patch
Comment 2 WebKit Review Bot 2011-11-11 16:22:40 PST
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Comment 3 Adrienne Walker 2011-11-11 16:23:45 PST
I realize that some ports set mock scrollbars on all the time, but I felt like exposing a per-test setting seemed more flexible.

I'm hoping this patch will alleviate some of our scrollbar pixel difference woes.
Comment 4 Gustavo Noronha (kov) 2011-11-11 17:44:35 PST
Comment on attachment 114797 [details]
Patch

Attachment 114797 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10448591
Comment 5 Adrienne Walker 2011-11-11 18:39:07 PST
Created attachment 114810 [details]
Now with more exports
Comment 6 Gustavo Noronha (kov) 2011-11-11 19:11:36 PST
Comment on attachment 114810 [details]
Now with more exports

Attachment 114810 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10350700
Comment 7 Adrienne Walker 2011-11-12 12:40:17 PST
Created attachment 114841 [details]
Patch
Comment 8 Adrienne Walker 2011-11-13 10:34:47 PST
Created attachment 114860 [details]
Fourth try's a charm
Comment 9 Adrienne Walker 2011-11-13 19:17:15 PST
(In reply to comment #8)
> Created an attachment (id=114860) [details]
> Fourth try's a charm

This link error really makes no sense.  Why is WebKit.dll referencing Internals.h? Shouldn't that be contained within WebCoreTestSupport?

11>   Creating library C:\cygwin\home\buildbot\Webkit\WebKitBuild\Debug\lib\WebKit.lib and object C:\cygwin\home\buildbot\Webkit\WebKitBuild\Debug\lib\WebKit.exp
11>WebKit.exp : error LNK2001: unresolved external symbol "public: void __thiscall WebCore::Internals::setMockScrollbarsEnabled(class WebCore::Document *,bool,int &)" (?setMockScrollbarsEnabled@Internals@WebCore@@QAEXPAVDocument@2@_NAAH@Z)
Comment 10 Darin Fisher (:fishd, Google) 2011-11-14 14:37:31 PST
Comment on attachment 114860 [details]
Fourth try's a charm

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

> Source/WebKit/chromium/public/WebSettings.h:107
> +    virtual void setMockScrollbarsEnabled(bool) = 0;

Chromium WebKit API changes LGTM
Comment 11 Adam Roben (:aroben) 2011-11-15 09:16:47 PST
(In reply to comment #9)
> (In reply to comment #8)
> > Created an attachment (id=114860) [details] [details]
> > Fourth try's a charm
> 
> This link error really makes no sense.  Why is WebKit.dll referencing Internals.h? Shouldn't that be contained within WebCoreTestSupport?

WebCoreTestSupport is a static library that is linked into DRT and WTR and calls various WebCore functions. WebCore (on Windows) is a static library that is linked into WebKit.dll.

For DRT/WTR to be able to call WebCore functions (via WebCoreTestSupport), those functions need to be exported from WebKit.dll.

You can fix your linker error by adding the appropriate mangled symbols to WebKit2.def. You can see there are already multiple Internals-related functions listed there toward the bottom.
Comment 12 Adrienne Walker 2011-11-15 09:40:14 PST
Created attachment 115179 [details]
Oops. Don't export the wrong symbol from WebKit2.def, causing a linker error in WebKit.dll
Comment 13 Adrienne Walker 2011-11-15 09:43:03 PST
(In reply to comment #11)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > Created an attachment (id=114860) [details] [details] [details]
> > > Fourth try's a charm
> > 
> > This link error really makes no sense.  Why is WebKit.dll referencing Internals.h? Shouldn't that be contained within WebCoreTestSupport?
> 
> WebCoreTestSupport is a static library that is linked into DRT and WTR and calls various WebCore functions. WebCore (on Windows) is a static library that is linked into WebKit.dll.
> 
> For DRT/WTR to be able to call WebCore functions (via WebCoreTestSupport), those functions need to be exported from WebKit.dll.
> 
> You can fix your linker error by adding the appropriate mangled symbols to WebKit2.def. You can see there are already multiple Internals-related functions listed there toward the bottom.

Thanks for all the info and explanation.  :)

I think I see what happened.  I copied and pasted the wrong symbol (the caller, not the callee) into WebKit2.def, so WebKit.dll expected that function to exist when it was building.  Oops.
Comment 14 Adrienne Walker 2011-11-15 09:48:14 PST
Created attachment 115182 [details]
Rebaselined
Comment 15 James Robinson 2011-11-16 13:47:46 PST
Comment on attachment 115182 [details]
Rebaselined

R=me
Comment 16 Adrienne Walker 2011-11-16 18:09:39 PST
Committed r100545: <http://trac.webkit.org/changeset/100545>