Patch to follow.
Created attachment 109645 [details] Patch
Created attachment 109655 [details] [Patch] More ResourceViews.
Comment on attachment 109655 [details] [Patch] More ResourceViews. View in context: https://bugs.webkit.org/attachment.cgi?id=109655&action=review > Source/WebCore/inspector/front-end/ResourceHeadersView.js:328 > + if (typeof headersLength === "number") { Extract all this block into a separate method and call it explicitly where needed instead of doing this odd check here.
Comment on attachment 109655 [details] [Patch] More ResourceViews. View in context: https://bugs.webkit.org/attachment.cgi?id=109655&action=review >> Source/WebCore/inspector/front-end/ResourceHeadersView.js:328 > > Extract all this block into a separate method and call it explicitly where needed instead of doing this odd check here. This method has 6 fields and 6 refresh methods that update corresponding fields. I would not want to split one of them into two parts for the following reasons: - Call site might get confused on whether it should call both methods of only one - This is a classical render(title, opt_subtitle). Not sure why call site should have two methods for this case - Imagine title owning subtitle element, how would you allow title to position (define location of) subtitle element
> Extract all this block into a separate method and call it explicitly where needed instead of doing this odd check here. Does this mean that the change suggested in required in order to let this patch in?
(In reply to comment #4) > (From update of attachment 109655 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=109655&action=review > > >> Source/WebCore/inspector/front-end/ResourceHeadersView.js:328 > > > > > Extract all this block into a separate method and call it explicitly where needed instead of doing this odd check here. > > This method has 6 fields and 6 refresh methods that update corresponding fields. I would not want to split one of them into two parts for the following reasons: > - Call site might get confused on whether it should call both methods of only one Method name should eliminate such confusion. > - This is a classical render(title, opt_subtitle). Not sure why call site should have two methods for this case Because it is a different than a public method on TreeElement or such in that it is a private method which is called from two places: at first with 3 params, at second 2 params and because of that you have to an id block to the method which could well be avoided. > - Imagine title owning subtitle element, how would you allow title to position (define location of) subtitle element Extract title creation into a separate method that would return a container where to insert the subtitle?
(In reply to comment #5) > > Extract all this block into a separate method and call it explicitly where needed instead of doing this odd check here. > > Does this mean that the change suggested in required in order to let this patch in? Although it would make the inspector code more clear, I wouldn't block this patch if you have a principal disagreement on this change.
Landed as http://svn.webkit.org/repository/webkit/trunk@96695