[Webkit-unassigned] [Bug 89735] Web Inspector: Provide context menu 'Delete all watch expressions.'

[bugzilla-daemon at webkit.org]

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





--- Comment #21 from Yury Semikhatsky <yurys at chromium.org>  2012-06-28 03:11:41 PST ---
(In reply to comment #17)
> (From update of attachment 149507 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=149507&action=review
> 
> > Source/WebCore/inspector/front-end/WatchExpressionsSidebarPane.js:262
> > +        this.watchExpressions = [];
> 
> BTW, I may have mislead you with this, if watchExpression happens to be a "reference" to an array stored in settings that we also want to update...

We shouldn't return a reference to an array from the setting and rely on the storage to be updated when the array is modified. All settings modifications must be done with Setting.set method which among other things will update the internal reference.


(In reply to comment #20)
> (In reply to comment #17)
> > (From update of attachment 149507 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=149507&action=review
> > 
> > > Source/WebCore/inspector/front-end/WatchExpressionsSidebarPane.js:262
> > > +        this.watchExpressions = [];
> > 
> > BTW, I may have mislead you with this, if watchExpression happens to be a "reference" to an array stored in settings that we also want to update...
> 
> Thanks Andrey for the Comment. I understand your point. Posted the patch with correction.

The new code is confusing. If we want to protect Setting's internal structure we should return a copy of the setting structure from Setting.get() Anyways in this particular place this.watchExpressions = []; works just fine as is followed by a call to Setting.set().

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