95589 2012-08-31 10:53:07 -0700 [Mac] Clarify -[WebView _setNotificationProvider:] 2012-08-31 10:54:33 -0700 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) Mac OS X 10.8 NEW InRadar P2 Normal --- 1 jonlee jonlee darin webkit-bug-importer oldest_to_newest 709798 0 jonlee 2012-08-31 10:53:07 -0700 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. 709799 1 webkit-bug-importer 2012-08-31 10:53:19 -0700 <rdar://problem/12216386>