Bug 21475

Summary: Provide support for the Geolocation API
Product: WebKit Reporter: Andre-John Mas <andrejohn.mas>
Component: WebCore JavaScriptAssignee: Greg Bolsinga <bolsinga>
Status: RESOLVED FIXED    
Severity: Enhancement CC: andersca, ddkilzer, koivisto, pknight, rik, sam, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch to implement this feature
none
updated patch
none
updated patch (moves items into WebCore/page)
none
updated other build systems
sam: review-
addresses Sam's review comments.
none
Forgot to add GeolocationService to all build projects
none
Address navigator properties enumeration, add test for when Geolocation is not implmented
none
Create test cases!
none
Add missed changes to other build systems
sam: review-
updated
ddkilzer: review-
updated patch (so ddkilzer can apply it for me) ddkilzer: review-

Andre-John Mas
Reported 2008-10-08 10:07:23 PDT
The W3C has published a draft specification for a Geolocation API: http://dev.w3.org/geo/api/spec-source.html It would be nice to see this added to Webkit. How the data is retrieved would depend on the platform. For example on the iPhone this could use the CoreLocation service and in other environments this could be got from the router or a user specified setting (useful in desktop environments). In all cases the user should be asked to share the location with the web site. An implementation is currently available for Firefox, in the form of an extension: http://labs.mozilla.com/2008/10/introducing-geode/ This is an API that is accessed by Javascript and can be used to provide longitude and latitude information.
Attachments
Patch to implement this feature (88.14 KB, patch)
2008-10-09 16:29 PDT, Greg Bolsinga
no flags
updated patch (86.58 KB, patch)
2008-10-09 20:33 PDT, Greg Bolsinga
no flags
updated patch (moves items into WebCore/page) (83.38 KB, patch)
2008-10-10 10:28 PDT, Greg Bolsinga
no flags
updated other build systems (86.12 KB, patch)
2008-10-10 10:49 PDT, Greg Bolsinga
sam: review-
addresses Sam's review comments. (92.55 KB, patch)
2008-10-13 22:26 PDT, Greg Bolsinga
no flags
Forgot to add GeolocationService to all build projects (94.30 KB, patch)
2008-10-14 16:12 PDT, Greg Bolsinga
no flags
Address navigator properties enumeration, add test for when Geolocation is not implmented (103.38 KB, patch)
2008-10-15 13:11 PDT, Greg Bolsinga
no flags
Create test cases! (128.93 KB, patch)
2008-10-16 11:41 PDT, Greg Bolsinga
no flags
Add missed changes to other build systems (130.47 KB, patch)
2008-10-16 14:33 PDT, Greg Bolsinga
sam: review-
updated (109.94 KB, patch)
2008-10-17 14:43 PDT, Greg Bolsinga
ddkilzer: review-
updated patch (so ddkilzer can apply it for me) (110.24 KB, patch)
2008-10-23 17:53 PDT, Greg Bolsinga
ddkilzer: review-
Sam Weinig
Comment 1 2008-10-08 14:11:27 PDT
Assigning to Greg as he is already working on this.
Andrei Popescu
Comment 2 2008-10-09 02:35:44 PDT
There's an implementation in Gears, too: http://code.google.com/apis/gears/api_geolocation.html
Greg Bolsinga
Comment 3 2008-10-09 16:29:56 PDT
Created attachment 24244 [details] Patch to implement this feature This is a patch to implement this feature. On anything but iPhone, it will return NULL / undefined for navigator.geolocation.
Simon Fraser (smfr)
Comment 4 2008-10-09 17:34:49 PDT
A few comments from a quick glance: + long watchId = reinterpret_cast<long>(notifier.get()); + GeoNotifier* notifier= reinterpret_cast<GeoNotifier*>(watchId); These are pretty gross. Use a HashMap instead? +#if PLATFORM(IPHONE) +#ifdef __OBJC__ +@class CLLocation; +#else +class CLLocation; +#endif +#endif Do you really want to pollute WebCore/dom files with platform-specific stuff? + Geoposition(double latitude, double longitude, double altitude, double accuracy, + double altitudeAccuracy, double heading, double velocity, + unsigned long long timestamp) + : m_latitude(latitude) + , m_longitude(longitude) + , m_altitude(altitude) + , m_accuracy(accuracy) ... Follow WebKit formatting conventions.
Greg Bolsinga
Comment 5 2008-10-09 20:33:52 PDT
Created attachment 24253 [details] updated patch Addresses Simon's issues. I think I have the formatting correct; I'm not entirely certain what was wrong with the ctor, but I matched another I found.
Sam Weinig
Comment 6 2008-10-09 21:07:13 PDT
+#if PLATFORM(IPHONE) I don't believe this is the correct #if. I think we should add ENABLE(GEOLOCATION) platform macro and wrap all related functionality in that so it is not compiled on unsupported platforms. Geolocation.h and the like should not be in WebCore/dom which is for core DOM functionality, but probably rather WebCore/page is the best place for it now.
Greg Bolsinga
Comment 7 2008-10-09 21:17:14 PDT
Antti didn't want to have this all wrapped up in an #ifdef so that there were not too many 'configurations' of WebCore.
Greg Bolsinga
Comment 8 2008-10-10 10:28:45 PDT
Created attachment 24264 [details] updated patch (moves items into WebCore/page) updated patch (moves items into WebCore/page)
Greg Bolsinga
Comment 9 2008-10-10 10:49:06 PDT
Created attachment 24267 [details] updated other build systems I forgot to update the other build systems.
Sam Weinig
Comment 10 2008-10-10 23:41:19 PDT
Comment on attachment 24267 [details] updated other build systems In addition to the comments we discussed on IRC about abstracting the platform differences behind an abstraction in WebCore/platform and adding tests, here are some comments. I don't think you are using the latest license. We have a two clause version that correctly identifies Apple as Apple, Inc. in all places. You can find an example of it in ScrollbarTheme.h. + if (exec->hadException()) { + m_frame->domWindow()->console()->reportCurrentException(exec); + + raisedException = true; + } Extra newline. + virtual void handleEvent(Geoposition*, bool& raisedException); +private: + JSCustomPositionCallback(JSC::JSObject* callback, Frame*); There should be a newline before private: + virtual void handleEvent(PositionError*); +private: + JSCustomPositionErrorCallback(JSC::JSObject* callback, Frame*); Same here. + if (Frame* frame = toJSDOMWindow(exec->dynamicGlobalObject())->impl()->frame()) + positionCallback = JSCustomPositionCallback::create(object, frame); This should probably use the lexicalGlobalObject. I don't think any of the uses of the dynamic global object are correct in the patch and all should probably be replaced with use of the lexical one. +Geolocation::Geolocation(Frame* frame) + : m_frame(frame) +{ + +} Extra newline. + { + GeoNotifierSet::const_iterator end = m_oneShots.end(); The { seems only scope the for loops differently so they can use the same variable names. I think a better strategy would be to make the names more verbose (oneShotsEnd, and watchesEnd). + void timerFired(Timer<GeoNotifier>* timer); No need to include parameter name. + RefPtr<PositionCallback> m_successCallback; + RefPtr<PositionErrorCallback> m_errorCallback; + Timer<GeoNotifier> m_timer; We usually don't line up the variable names like this. + static PassRefPtr<Geolocation> create(Frame* frame); No need to include parameter name. + void clearWatch(long watchId); I think this should be an int. long in IDL translates to int in cpp. + void handleNewPosition(PassRefPtr<Geoposition> newPosition); + void handleError(PositionError* error); + Geolocation(Frame* frame); + virtual bool startUpdating(PositionOptions* options) = 0; No need to include parameter names, + typedef HashSet< RefPtr<GeoNotifier> > GeoNotifierSet; Extra space before RefPtr. + typedef HashMap<long, RefPtr<GeoNotifier> > GeoNotifierMap; Again, probably int instead of long. +#include <wtf/Platform.h> +#include <wtf/RefCounted.h> +#include "PlatformString.h" These should be sorted. + , m_timestamp(timestamp) + {} + The braces should go on separate lines. + unsigned long long timestamp() const { return m_timestamp; } We typedef DOMTypeStamp in Event.h. You can use that for this. + DOMString toString(); Though I did not look at the spec, I suspect this should be [DontEnum]. +#if PLATFORM(IPHONE) + if (!m_geolocation) + m_geolocation = Geolocation::create(m_frame); + return m_geolocation.get(); +#else + return NULL; +#endif Clearly, +#if PLATFORM(IPHONE) is wrong here. I think we should introduce ENABLE(GEOLOCATION). Also, the NULL should just be 0. Just returning 0 here is really not enough because the geolocation property is still enumerable and will still return true for hasProperty and the corresponding 'geolocation' in window.navigator feature checks. I agree with Antti (he said this on irc) that there are good reasons to want to compile this feature to ensure it doesn't die of code rot, but I think it needs to not be detectable. It looks like the Geolocation is not getting marked by its parent. I assumed the JSPluginArray and JSMimeTypeArray are having the same problem, but we should follow the paradigm set in DOMWindow and add optionalGeolocation methods and CustomMark to JSNavigator. + virtual void handleEvent(Geoposition* position, bool& raisedException) = 0; position parameter name is not needed. +#include <wtf/Platform.h> +#include <wtf/RefCounted.h> +#include "PlatformString.h" Please sort. + , m_message(message) + {} + Braces should go on separate lines. + long code() const { return m_code; } + void setCode(long code) { m_code = code; } + long m_code; Again, long == int.
Greg Bolsinga
Comment 11 2008-10-13 22:26:46 PDT
Created attachment 24339 [details] addresses Sam's review comments. This should cover everything Sam mentions except for enumerating Navigator properties when ENABLE(GEOLCOATION) is not on, and a test case. For the Test case, it isn't clear how to test if the platform doesn't support any location services. For when there are location services, since it can be tested from anywhere, the test will need to handle multiple locations.
Greg Bolsinga
Comment 12 2008-10-14 16:12:14 PDT
Created attachment 24350 [details] Forgot to add GeolocationService to all build projects See above.
Greg Bolsinga
Comment 13 2008-10-15 13:11:44 PDT
Created attachment 24365 [details] Address navigator properties enumeration, add test for when Geolocation is not implmented Geolocation support will compile. Navigator.idl and Navigator.cpp now use ENABLE(GEOLOCATION) to turn this off. All the support code compiles but is not linked in. I added a test for when ENABLE(GEOLOCATION) is off. If it is implemented, this test will fail, and should be ignored. Once it is implemented a new test will need to be created. Due to how Navigator's properties are created (via IDL) with ENABLE_GEOLOCATION, it isn't clear how to have a test that will determine at runtime if Geolocation support is available. So I did not do any DRT time only work.
Timothy Hatcher
Comment 14 2008-10-15 23:27:43 PDT
Thid IDL files should not be added to the Resources. The shout be in the project, but not part of a target. 16198 FE80D7C70E9C1F25000D6F75 /* Geolocation.idl in Resources */, 16199 FE80D7CA0E9C1F25000D6F75 /* Geoposition.idl in Resources */, 16200 FE80D7CC0E9C1F25000D6F75 /* PositionCallback.idl in Resources */, 16201 FE80D7CE0E9C1F25000D6F75 /* PositionError.idl in Resources */, 16202 FE80D7D00E9C1F25000D6F75 /* PositionErrorCallback.idl in Resources */, 16203 FE80D7D20E9C1F25000D6F75 /* PositionOptions.idl in Resources */,
Greg Bolsinga
Comment 15 2008-10-16 11:41:26 PDT
Created attachment 24400 [details] Create test cases! I have implemented test cases. geolocation-not-implmented verifies that geolocation isn't there, as nothing has a platform implementation yet. The geolocation-fake-* tests put WebCore into a mode for platforms that do not support Geolocation to give fake results for geolocation to test this code out. In addition, the IDL files are no longer in the resources.
Greg Bolsinga
Comment 16 2008-10-16 14:33:37 PDT
Created attachment 24425 [details] Add missed changes to other build systems Added missed added files to WebCore/GNUmakefile.am WebCore/WebCore.vcproj/WebCore.vcproj WebCore/WebCore.pro WebCore/WebCoreSources.bkl Otherwise is the same as to the previous patch (less subsequent WebKit repro updates).
Sam Weinig
Comment 17 2008-10-16 18:05:54 PDT
Comment on attachment 24425 [details] Add missed changes to other build systems + * ChangeLog: This should not be in here. +#include "config.h" +#include "JSCustomPositionCallback.h" +#include "Console.h" MIssing newline between JSCustomPositionCallback.h and Console.h. +#include <kjs/JSObject.h> +#include <kjs/protect.h> +#include "PositionCallback.h" +#include <wtf/Forward.h> This should be sorted. +#include "config.h" +#include "JSGeolocation.h" +#include "DOMWindow.h" Missing newline. +JSC::JSValue* toJS(JSC::ExecState* exec, Geolocation* object) The JSC:: are not needed. + return getDOMObjectWrapper<JSGeolocation>(exec, object); + else + return jsUndefined(); The else not needed. + int w = m_impl->watchPosition(positionCallback.release(), positionErrorCallback.release(), positionOptions.get()); The w variable name is a little vague. Perhaps watchID? +#include "config.h" +#include "Geolocation.h" +#include "PositionError.h" Missing newline. +#include <wtf/Platform.h> +#include <wtf/HashMap.h> +#include <wtf/HashSet.h> +#include <wtf/OwnPtr.h> +#include <wtf/PassRefPtr.h> +#include <wtf/RefCounted.h> +#include <wtf/RefPtr.h> + +#include "GeolocationService.h" +#include "PositionCallback.h" +#include "PositionErrorCallback.h" +#include "PositionOptions.h" +#include "Timer.h" We usually put the wtf headers last. +void Geolocation::disconnectFrame() +{ + m_frame = 0; +} This should probably stop all the watchers and oneShots. + RefPtr<PositionError> error = new PositionError(4, "Timed out"); + m_errorCallback->handleEvent(error.get()); This is leaking the PostionError. You should use the create idiom for this. The handleEvent should probably also take a PassRefPtr. + RefPtr<GeoNotifier> notifier = new GeoNotifier(successCallback, errorCallback, options); This also leaks + RefPtr<PositionError> error = new PositionError(1, "Unable to Start"); Here too. + RefPtr<GeoNotifier> notifier = m_watchers.get(watchId); + if (notifier) + m_watchers.remove(watchId); There is no reason to call get here. + GeoNotifierSet::const_iterator oneShotsEnd = m_oneShots.end(); + for (GeoNotifierSet::const_iterator it = m_oneShots.begin(); it != oneShotsEnd; ++it) { + RefPtr<GeoNotifier> notifier = *it; + + if (notifier->m_errorCallback) + notifier->m_errorCallback->handleEvent(error); + } + m_oneShots.clear(); + + GeoNotifierMap::const_iterator watchersEnd = m_watchers.end(); + for (GeoNotifierMap::const_iterator it = m_watchers.begin(); it != watchersEnd; ++it) { + RefPtr<GeoNotifier> notifier = (*it).second; + + if (notifier->m_errorCallback) + notifier->m_errorCallback->handleEvent(error); + } These both need to copy the HashSet/HashMap to avoid it being mutated while iterating it. Why isn't the watchers map cleared? + GeoNotifierSet::const_iterator oneShotsEnd = m_oneShots.end(); + for (GeoNotifierSet::const_iterator it = m_oneShots.begin(); it != oneShotsEnd; ++it) { Need to copy it here too. + RefPtr<PositionError> error = new PositionError(5, "JavaScript Error"); This leaks. Also, the error should probably be a bit more generic and say something like "An exception was thrown" so it is not JS specific. + GeoNotifierMap::const_iterator watchersEnd = m_watchers.end(); + for (GeoNotifierMap::const_iterator it = m_watchers.begin(); it != watchersEnd; ++it) { Same issues for this too. You can probably use the copying to a Vector to get the best efficiency. +#include <wtf/Platform.h> Not necessary here. +#if ENABLE_GEOLOCATION + readonly attribute Geolocation geolocation; +#else + readonly attribute [DontEnum] Geolocation geolocation; +#endif I don't think this will do it. For instance, I think navigator.hasProperty("geolocation") or "geolocation" in navigator will still return true. + int code() const { return m_code; } + void setCode(int code) { m_code = code; } + const String& message() const { return m_message; } + void setMessage(const String& message) { m_message = message; } setCode and setMessage are not needed as the properties are readonly. +#include "config.h" + +#include "GeolocationService.h" Newline in the wrong place. +GeolocationService::GeolocationService(GeolocationServiceClient* client) + : m_geolocationServiceClient(client) +{ +} It would probably be better to assert that you have a client and remove the null checks. +class GeolocationServiceClient +{ Brace should go on previous line. For GeolocationService, I think a better design would be to have implementations provide the methods instead of requiring them to subclass by using pure virtual methods. +#include "config.h" +#include "GeolocationServiceFake.h" +#include "WebCoreGeolocation.h" Newline. + m_lastPosition = new Geoposition(37.33264,-122.03099,0.0,1.0,0.0,0.0,0.0,1000.0); Where is this? Can we make this configurable like WebCoreShouldUseFakeGeolocation()? + GeolocationService::positionChanged(); No prefix needed here. Do the PositionOptions work if you pass in a vanilla js object? Looking great. r- due to the leaks.
Greg Bolsinga
Comment 18 2008-10-17 14:43:26 PDT
Created attachment 24463 [details] updated This addresses the items in Sam's review. Objects go through ::create() for RefPtrs. Header order cleaned up. Doesn't add page to the back/forward cache if it is currently using Geolocation (from IRC). stopsUpdating when Geolocation::disconnectFrame is called. Copies data structures before iterating (nice catch). The GeolocationServiceFake (and its tests) were removed from the patch. Having geolocation be a property on Navigator only when in DRT has proven to be complex. The test to verify that geolocation hasn't leaked into navigator is some fashion is retained and more robust than the last patch. I will file a followup bug for the GeolocationServiceFake implementation and test.
Greg Bolsinga
Comment 19 2008-10-17 14:49:31 PDT
I filed https://bugs.webkit.org/show_bug.cgi?id=21717 for the GeolocationServiceFake implementation and test cases.
Sam Weinig
Comment 20 2008-10-23 16:43:12 PDT
Comment on attachment 24463 [details] updated r=me. There are a bunch of comments below, but I think we have gone back and forth enough to land this patch. I would encourage you to make all the changes, but you can use your discretion. > + // should this be on the Frame? No, and this comment should go away. > +void Geolocation::sendErrorToOneShots(PositionError* error) > +{ > + const GeoNotifierSet copy = m_oneShots; I think it is more efficient to copy this to a Vector and iterate the vector. There is a function call copyToVector(const HashSet<>, Vector<>) that you can use. > +void Geolocation::sendErrorToWatchers(PositionError* error) > +{ > + const GeoNotifierMap copy = m_watchers; Same here. Since you only need the hash values, you can use copyValuesToVector(). > +void Geolocation::sendPositionToOneShots(Geoposition* position) > +{ > + const GeoNotifierSet copy = m_oneShots; And here. > + > +void Geolocation::sendPositionToWatchers(Geoposition* position) > +{ > + const GeoNotifierMap copy = m_watchers; Once again > +#include "GeolocationService.h" > +#include "PositionCallback.h" > +#include "PositionErrorCallback.h" > +#include "PositionOptions.h" > +#include "Timer.h" > + > +#include <wtf/Platform.h> > +#include <wtf/HashMap.h> > +#include <wtf/HashSet.h> > +#include <wtf/OwnPtr.h> > +#include <wtf/PassRefPtr.h> > +#include <wtf/RefCounted.h> > +#include <wtf/RefPtr.h> These don't have to be paragraphed. > + > + interface [ > + GenerateConstructor > + ] Geolocation { If this is generating a constructor, you need to update DOMWindow.idl (with the requisite #ifdef) to add the constructor there too. > + interface [ > + GenerateConstructor > + ] Geoposition { Same deal with the constructor. > + > + [DontEnum] DOMString toString(); We usually #ifdef these only for JS. > > bool onLine() const; > + Geolocation* geolocation() const; > + // This is used for GC marking. > + Geolocation* optionalGeolocation() const { return m_geolocation.get(); } > private: Need a newline before private. > +class PositionError : public RefCounted<PositionError> { > +public: > + enum ErrorCode { > + PERMISSION_ERROR = 1, > + LOCATION_PROVIDER_ERROR, > + POSITION_NOT_FOUND_ERROR, > + TIMEOUT_ERROR, > + UNKNOWN_ERROR > + }; Perhaps we should give feedback to the WG that the error codes should have constants? > + > + class PositionErrorCallback : public RefCounted<PositionErrorCallback> { > + public: > + virtual ~PositionErrorCallback() { } > + virtual void handleEvent(PositionError* position) = 0; No need for the parameter name here. > + > + bool enableHighAccuracy() const { return m_highAccuracy; } > + void setEnableHighAccuracy(bool enable) { m_highAccuracy = enable; } > + long timeout() const { return m_timeout; } > + void setTimeout(long t) { m_timeout = t; } Should be ints. Are the set methods ever used? > + > + bool m_highAccuracy; > + long m_timeout; Should be an int. > + > +class GeolocationService : public Noncopyable { > +public: > + static GeolocationService* create(GeolocationServiceClient*); > + virtual ~GeolocationService() {} > + > + virtual bool startUpdating(PositionOptions*) = 0; > + virtual void stopUpdating() = 0; > + > + virtual Geoposition* lastPosition() const = 0; > + virtual PositionError* lastError() const = 0; Since we are not doing the FakeService anymore, I don't think these need to be virtual at all.
Greg Bolsinga
Comment 21 2008-10-23 17:53:13 PDT
Created attachment 24627 [details] updated patch (so ddkilzer can apply it for me) This addresses Sam's review. The setter is needed by JS* object. I needed to keep GeolocationService::stopUpdating virtual so WebCore would link (it is used by Geolocation). I removed the GenerateConstructors from GeoPosition and GeoLocation.
David Kilzer (:ddkilzer)
Comment 22 2008-10-23 22:07:40 PDT
$ svn commit JavaScriptCore WebCore LayoutTests Sending JavaScriptCore/ChangeLog Sending JavaScriptCore/wtf/Platform.h Sending LayoutTests/ChangeLog Adding LayoutTests/geolocation Adding LayoutTests/geolocation/geolocation-not-implemented-expected.txt Adding LayoutTests/geolocation/geolocation-not-implemented.html Adding LayoutTests/geolocation/geolocation-test.js Sending WebCore/ChangeLog Sending WebCore/DerivedSources.make Sending WebCore/GNUmakefile.am Sending WebCore/WebCore.pro Sending WebCore/WebCore.vcproj/WebCore.vcproj Sending WebCore/WebCore.xcodeproj/project.pbxproj Sending WebCore/WebCoreSources.bkl Adding WebCore/bindings/js/JSCustomPositionCallback.cpp Adding WebCore/bindings/js/JSCustomPositionCallback.h Adding WebCore/bindings/js/JSCustomPositionErrorCallback.cpp Adding WebCore/bindings/js/JSCustomPositionErrorCallback.h Sending WebCore/bindings/js/JSDOMBinding.cpp Sending WebCore/bindings/js/JSDOMBinding.h Sending WebCore/bindings/js/JSDOMWindowCustom.cpp Adding WebCore/bindings/js/JSGeolocationCustom.cpp Sending WebCore/bindings/js/JSNavigatorCustom.cpp Sending WebCore/dom/Document.cpp Sending WebCore/dom/Document.h Sending WebCore/loader/FrameLoader.cpp Adding WebCore/page/Geolocation.cpp Adding WebCore/page/Geolocation.h Adding WebCore/page/Geolocation.idl Adding WebCore/page/Geoposition.cpp Adding WebCore/page/Geoposition.h Adding WebCore/page/Geoposition.idl Sending WebCore/page/Navigator.cpp Sending WebCore/page/Navigator.h Sending WebCore/page/Navigator.idl Adding WebCore/page/PositionCallback.h Adding WebCore/page/PositionCallback.idl Adding WebCore/page/PositionError.h Adding WebCore/page/PositionError.idl Adding WebCore/page/PositionErrorCallback.h Adding WebCore/page/PositionErrorCallback.idl Adding WebCore/page/PositionOptions.h Adding WebCore/page/PositionOptions.idl Adding WebCore/platform/GeolocationService.cpp Adding WebCore/platform/GeolocationService.h Transmitting file data ............................................ Committed revision 37840. http://trac.webkit.org/changeset/37840
David Kilzer (:ddkilzer)
Comment 23 2008-10-23 22:40:39 PDT
Follow-up GTK fix: $ svn commit WebCore Sending WebCore/ChangeLog Sending WebCore/GNUmakefile.am Transmitting file data .. Committed revision 37841. http://trac.webkit.org/changeset/37841
David Kilzer (:ddkilzer)
Comment 24 2008-10-24 00:45:06 PDT
Comment on attachment 24627 [details] updated patch (so ddkilzer can apply it for me) I rolled out r37840 and r37841. There are too many issues yet to be resolved with this patch. Committed r37842 http://trac.webkit.org/changeset/37842 - Derived sources missing from Xcode project. (Adding them causes build failures.) DerivedSources/WebCore/JSPositionCallback.cpp DerivedSources/WebCore/JSPositionCallback.h DerivedSources/WebCore/JSPositionErrorCallback.cpp DerivedSources/WebCore/JSPositionErrorCallback.h All DOM* generated classes for *.idl files. - Changes to WebKit/mac/MigrateHeaders.make for DOM* classes. - Windows build breaks. - GTK, Qt and wx builds break (Qt is currently broken) and each platform's build files are missing *.idl and other source files.
David Kilzer (:ddkilzer)
Comment 25 2008-10-24 14:08:02 PDT
$ svn commit JavaScriptCore WebCore LayoutTests Sending JavaScriptCore/ChangeLog Sending JavaScriptCore/wtf/Platform.h Sending LayoutTests/ChangeLog Adding LayoutTests/geolocation/geolocation-not-implemented-expected.txt Adding LayoutTests/geolocation/geolocation-not-implemented.html Adding LayoutTests/geolocation/geolocation-test.js Sending WebCore/ChangeLog Sending WebCore/DerivedSources.make Sending WebCore/GNUmakefile.am Sending WebCore/WebCore.pro Sending WebCore/WebCore.vcproj/WebCore.vcproj Sending WebCore/WebCore.xcodeproj/project.pbxproj Sending WebCore/WebCoreSources.bkl Adding WebCore/bindings/js/JSCustomPositionCallback.cpp Adding WebCore/bindings/js/JSCustomPositionCallback.h Adding WebCore/bindings/js/JSCustomPositionErrorCallback.cpp Adding WebCore/bindings/js/JSCustomPositionErrorCallback.h Sending WebCore/bindings/js/JSDOMBinding.cpp Sending WebCore/bindings/js/JSDOMBinding.h Sending WebCore/bindings/js/JSDOMWindowCustom.cpp Adding WebCore/bindings/js/JSGeolocationCustom.cpp Sending WebCore/bindings/js/JSNavigatorCustom.cpp Sending WebCore/dom/Document.cpp Sending WebCore/dom/Document.h Sending WebCore/loader/FrameLoader.cpp Adding WebCore/page/Geolocation.cpp Adding WebCore/page/Geolocation.h Adding WebCore/page/Geolocation.idl Adding WebCore/page/Geoposition.cpp Adding WebCore/page/Geoposition.h Adding WebCore/page/Geoposition.idl Sending WebCore/page/Navigator.cpp Sending WebCore/page/Navigator.h Sending WebCore/page/Navigator.idl Adding WebCore/page/PositionCallback.h Adding WebCore/page/PositionCallback.idl Adding WebCore/page/PositionError.h Adding WebCore/page/PositionError.idl Adding WebCore/page/PositionErrorCallback.h Adding WebCore/page/PositionErrorCallback.idl Adding WebCore/page/PositionOptions.h Adding WebCore/page/PositionOptions.idl Adding WebCore/platform/GeolocationService.cpp Adding WebCore/platform/GeolocationService.h Transmitting file data ............................................ Committed revision 37854. $ svn commit WebCore Sending WebCore/ChangeLog Sending WebCore/WebCore.pro Transmitting file data .. Committed revision 37857.
David Kilzer (:ddkilzer)
Comment 27 2008-10-24 14:52:31 PDT
Gtk build fix: $ svn commit WebCore Sending WebCore/ChangeLog Sending WebCore/GNUmakefile.am Transmitting file data .. Committed revision 37866. http://trac.webkit.org/changeset/37866
Note You need to log in before you can comment on or make changes to this bug.