[Webkit-unassigned] [Bug 77926] [Blackberry] Clean up Networkjob and Networkmanger: remove unused variables and change some public functions into be private ones

[bugzilla-daemon at webkit.org]

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





--- Comment #13 from Chris.Guan <chris.guan at torchmobile.com.cn>  2012-02-06 23:09:26 PST ---
(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #10)
> > > (In reply to comment #7)
> > > > (In reply to comment #6)
> > > > > Why remove m_isRunning and the ASSERT? What about guard it with #ifndef NDEBUG macro so that m_isRunning and NetworkJob::isRunning() are there only for Debug mode.
> > > > 
> > > > No, it dose not make any help, because the code log makes sure this assert is never hit, because the delete timer is firing after handleNotifyClose() in which
> > > > definitely m_isRunning was set to false. I added this comments in my change log in the patch:)
> > > 
> > > 
> > > ASSERT is designed for never being hit. We ASSERT(!job->isRunning()) to make sure we don't delete a job on which handleNotifyClose() never be called. If you remove this ASSERT, we can't catch the code offending this assumption any more.
> > 
> > yes, you are right, but deleteJob() is a private function of NetworkManger, and only the friend class of networkjob can use it, actually it is deleteTimer. refactor?
> Why refactor? As long as you keep NetworkJob::isRunning() as public and keep NetowrkManager::deleteJob() AS IS, I don't think a refactor is required. My suggestion is, just as I said in the previous comment, guarding NetworkJob::isRuning() and NetworkJob::m_isRunning by #ifndef NDEBUG.
yes, I understand it, let me fix your comments in next patch.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.