Bug 95589
Summary: | [Mac] Clarify -[WebView _setNotificationProvider:] | ||
---|---|---|---|
Product: | WebKit | Reporter: | Jon Lee <jonlee> |
Component: | DOM | Assignee: | Jon Lee <jonlee> |
Status: | NEW | ||
Severity: | Normal | CC: | darin, webkit-bug-importer |
Priority: | P2 | Keywords: | InRadar |
Version: | 528+ (Nightly build) | ||
Hardware: | Mac | ||
OS: | OS X 10.8 |
Jon Lee
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 | ||
---|---|---|
Add attachment proposed patch, testcase, etc. |
Radar WebKit Bug Importer
<rdar://problem/12216386>