[webkit-reviews] review denied: [Bug 61868] Web Inspector: ResourceCookiesView.resize() missing : [Attachment 95615] patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Andrey Kosyakov
<caseq at chromium.org>'s request for review:
Bug 61868: Web Inspector: ResourceCookiesView.resize() missing
https://bugs.webkit.org/show_bug.cgi?id=61868

Attachment 95615: patch
https://bugs.webkit.org/attachment.cgi?id=95615&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=95615&action=review

>>> Source/WebCore/inspector/front-end/ResourceCookiesView.js:64
>>> +	     this._cookiesTable.updateWidths();
>> 
>> Can resize() be called before show()? I.e. are you missing the null check
here?
> 
> So far we only call it in case the view is already visible
(http://www.google.com/codesearch?hl=en&vert=chromium&lr=&q=cookiesView.resize&
sbtn=Search). Whether we should do a guard check for future usages is a matter
of taste -- I'd rather not. We shouldn't be resizing something that we're not
showing at the moment.

ResourceCookiesView inherits from View, so it should probably behave as a view.
Views can get resize signals at any time while visible. I don't see synchronous
correlation between showing view and getting this._cookiesTable assigned.

In fact, if our tabbed pane was properly implemented and did delegate resize to
the tabs, your code would fail on the cookie-less resource resize, right?