NEW 95589
[Mac] Clarify -[WebView _setNotificationProvider:]
https://bugs.webkit.org/show_bug.cgi?id=95589
Summary [Mac] Clarify -[WebView _setNotificationProvider:]
Jon Lee
Reported 2012-08-31 10:53:07 PDT
From bug 95465: >>> Source/WebKit/mac/WebView/WebView.mm:6526 >>> } >> >> It seems a little non-obvious that this method does nothing once a notification provider is set. Are the callers able to tolerate this behavior? The alternative would be to teach this function to unregister the old provider when setting a new one. > > The provider is set once, typically during initialization of the web view. There is no reason I can think of that one would want to switch providers in the middle of the web view's lifetime. The case is similar with other kinds of providers like geolocation. > > Because of the use case, I opted to just go with initialize-once rather than unregister-the-old-provider, since that is not the intended use for this method. Maybe renaming this to _initializeNotificationProvider would be more descriptive. The name change is one possible way to increase clarity. Another would be to add an ASSERT_NOT_REACHED along with early return instead of nesting the function inside an if. The _private == nil case seems expected and harmless while the !_private->_notificationProvider seems like it always reflects a programming error. Might also want to assert that the provider is non-nil or something.
Attachments
Radar WebKit Bug Importer
Comment 1 2012-08-31 10:53:19 PDT
Note You need to log in before you can comment on or make changes to this bug.