Summary: | ResourceLoader should call ResourceHandle::setClient(0) after checking it's still the ResourceHandle's client | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Anyang Ren <anyang.ren> | ||||
Component: | Page Loading | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED INVALID | ||||||
Severity: | Normal | CC: | andersca | ||||
Priority: | P2 | ||||||
Version: | 523.x (Safari 3) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
URL: | http://trac.webkit.org/projects/webkit/changeset/24624 | ||||||
Attachments: |
|
Description
Anyang Ren
2007-08-10 16:33:52 PDT
Created attachment 15914 [details]
Verify we're still the ResourceHandle's client before clearing it out
Comment on attachment 15914 [details] Verify we're still the ResourceHandle's client before clearing it out In the future, please set the review? flag on patches. Thanks! At minimum, this patch will also need a ChangeLog entry. (I'm leaving it as review? so that the patch will be reviewed.) See this page for details: http://webkit.org/coding/contributing.html Comment on attachment 15914 [details]
Verify we're still the ResourceHandle's client before clearing it out
I don't think there's any reason to make this code change. It's only needed if ResourceLoader shares a ResourceHandle with another client. That's never done, so we don't need the code.
Unless there's something I'm missing and this actually happens.
This can happen if a WebKit embedder calls ResourceLoader::handle(), a public method, and then call ResourceHandle::setClient on the returned ResourceHandle to set its client to something else. I'd like to know if this usage of ResourceHandle::setClient is allowed. If not, perhaps ResourceHandle::setClient should be constrained to prevent a WebKit embedder from making this mistake. I just wanted to bring this issue to your attention. Please feel free to mark this bug invalid or wontfix after you have considered this issue. Thank you for your quick response. (In reply to comment #4) > This can happen if a WebKit embedder calls ResourceLoader::handle(), > a public method, and then call ResourceHandle::setClient on the > returned ResourceHandle to set its client to something else. I'd > like to know if this usage of ResourceHandle::setClient is allowed. > If not, perhaps ResourceHandle::setClient should be constrained > to prevent a WebKit embedder from making this mistake. I don't think there's a way to make this happen through the WebKit API, so I don't think it's possible for a "WebKit embedder" to get into this situation. Perhaps you're talking about someone who is making changes to WebCore/WebKit itself? If so, seeing the patches that are running into this situation would make this whole discussion much clearer. Adam, sorry about the late reply. Yes, I was experimenting with calling handle->setClient() in WebFrameLoaderClient::download to set the ResourceHandle's client to some other object that handles the download. But I don't need to do that any more. So I'm closing this bug. |