WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
39440
[chromium] Adds Geolocation support to DumpRenderTree.
https://bugs.webkit.org/show_bug.cgi?id=39440
Summary
[chromium] Adds Geolocation support to DumpRenderTree.
Marcus Bulach
Reported
2010-05-20 12:01:50 PDT
[chromium] Adds Geolocation support to DumpRenderTree.
Attachments
Patch
(11.56 KB, patch)
2010-05-20 12:12 PDT
,
Marcus Bulach
no flags
Details
Formatted Diff
Diff
Patch
(14.40 KB, patch)
2010-05-21 03:39 PDT
,
Marcus Bulach
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Marcus Bulach
Comment 1
2010-05-20 12:12:56 PDT
Created
attachment 56620
[details]
Patch
Kent Tamura
Comment 2
2010-05-20 20:45:11 PDT
Comment on
attachment 56620
[details]
Patch Thank you for making the patch. WebKitTools/DumpRenderTree/chromium is not the primary tester yet. You need to change webkit/tools/test_shell/ in Chromium too. WebKit/chromium/public/WebGeolocationServiceMock.h:55 + static bool m_mockGeolocationPermission; We don't prepend "m_" for a static field in WebKit style. WebKitTools/DumpRenderTree/chromium/LayoutTestController.cpp:1303 + WebGeolocationServiceMock::setMockGeolocationError(arguments[0].toInt32(), WebString::fromUTF8(arguments[1].toString())); nit: We have cppVariantToWebString(). r- for the style error.
Marcus Bulach
Comment 3
2010-05-21 03:39:33 PDT
Created
attachment 56691
[details]
Patch
Marcus Bulach
Comment 4
2010-05-21 03:43:10 PDT
thanks Tamura-san! replies inline: (In reply to
comment #2
)
> (From update of
attachment 56620
[details]
) > Thank you for making the patch. > WebKitTools/DumpRenderTree/chromium is not the primary tester yet. You need to change webkit/tools/test_shell/ in Chromium too.
yep, the change has been reviewed and I'll submit it soon:
http://codereview.chromium.org/2094003/show
> > > WebKit/chromium/public/WebGeolocationServiceMock.h:55 > + static bool m_mockGeolocationPermission; > We don't prepend "m_" for a static field in WebKit style.
Done.
> > > WebKitTools/DumpRenderTree/chromium/LayoutTestController.cpp:1303 > + WebGeolocationServiceMock::setMockGeolocationError(arguments[0].toInt32(), WebString::fromUTF8(arguments[1].toString())); > nit: We have cppVariantToWebString().
ahn, thanks! I went ahead and replaced all occurrences so it won't catch the next reader. however, please, let me know if you'd prefer to this tidy up in a separate patch, and I'd be happy to revert these changes. would you mind taking another look? thanks!
> > > r- for the style error.
Kent Tamura
Comment 5
2010-05-21 03:46:00 PDT
Comment on
attachment 56691
[details]
Patch OK.
WebKit Commit Bot
Comment 6
2010-05-23 02:07:44 PDT
Comment on
attachment 56691
[details]
Patch Clearing flags on attachment: 56691 Committed
r60032
: <
http://trac.webkit.org/changeset/60032
>
WebKit Commit Bot
Comment 7
2010-05-23 02:07:50 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug