[webkit-reviews] review denied: [Bug 103259] Numeric identifiers of events are not guaranteed to be unique : [Attachment 176838] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Cosmin Truta <ctruta at gmail.com>'s
request for review:
Bug 103259: Numeric identifiers of events are not guaranteed to be unique
https://bugs.webkit.org/show_bug.cgi?id=103259

Attachment 176838: Patch
https://bugs.webkit.org/attachment.cgi?id=176838&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=176838&action=review


review- because of the strange RefPtr local variable thing; otherwise looks OK

> Source/WebCore/Modules/geolocation/Geolocation.cpp:186
> +    bool addSucceeded = m_idToNotifierMap.add(id,
notifier.get()).isNewEntry;
> +    if (addSucceeded)
> +	   m_notifierToIdMap.set(notifier.release(), id);
> +    return addSucceeded;

I would write it this way:

    if (!m_idToNotifierMap.add(id, notifier.get()).isNewEntry)
	return false;
    m_notifierToIdMap.set(notifier.release(), id);
    return true;

> Source/WebCore/Modules/geolocation/Geolocation.cpp:321
> +	   RefPtr<GeoNotifier> passNotifier = notifier;
> +	   success = m_watchers.add(watchID, passNotifier.release());

This should just be:

    success = m_watches.add(watchID, notifier);

There is no need for the passNotifier local variable.