[Webkit-unassigned] [Bug 152480] ASSERT(m_downloads.isEmpty()) fails in DownloadProxyMap::~DownloadProxyMap()

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=152480

--- Comment #7 from David Quesada <david_quesada at apple.com> ---
(In reply to Chris Dumez from comment #6)
> Comment on attachment 364075 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=364075&action=review
> 
> > Source/WebKit/UIProcess/Downloads/DownloadProxyMap.cpp:62
> > +    auto protectedProcessPool = makeRefPtr(m_process->processPool());
> 
> Since the DownloadProxyMap is owned by the NetworkProcessProxy, I think it
> would be clearer to ref m_process here:
> auto protectedThis = makeRefPtr(m_process);

I thought about doing that at first, but the WebProcessPool has a unique_ptr to the NetworkProcessProxy, so I assumed it would be wrong to hold another reference outside of the "unique" reference. If we only ref the NetworkProcessProxy here, then the WebProcessPool would still be deallocated when invalidating the DownloadProxy, and the NetworkProcessProxy will be deallocated after a short existence without a WebProcessPool. I thought protecting the process pool would be safer, and I couldn't think of any downsides to doing so that would justify changing the process pool's unique_ptr to a Ref and ensuring the NetworkProcessProxy can outlive the WebProcessPool without any issues.


(Also, the win EWS issues appear unrelated - the tests are failing with or without this patch. And the mac-wk2 failure is also unrelated - when I checked on Friday, many other unrelated patches were seeing this test fail, and it was marked as a failure in r242688)

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20190310/b068b37e/attachment.html>