[webkit-reviews] review denied: [Bug 222296] [Cocoa] Web Inspector: Add support for reloading the inspected page via _WKInspectorExtension : [Attachment 421268] Patch v1.0

[bugzilla-daemon at webkit.org]

BJ Burg <bburg at apple.com> has denied Patrick Angle <pangle at apple.com>'s request
for review:
Bug 222296: [Cocoa] Web Inspector: Add support for reloading the inspected page
via _WKInspectorExtension
https://bugs.webkit.org/show_bug.cgi?id=222296

Attachment 421268: Patch v1.0

https://bugs.webkit.org/attachment.cgi?id=421268&action=review




--- Comment #2 from BJ Burg <bburg at apple.com> ---
Comment on attachment 421268
  --> https://bugs.webkit.org/attachment.cgi?id=421268
Patch v1.0

View in context: https://bugs.webkit.org/attachment.cgi?id=421268&action=review

r-, need to remove unnecessary argument from API method's completion handler.

>
Source/WebInspectorUI/UserInterface/Controllers/WebInspectorExtensionController
.js:149
> +	   for (let target of WI.targets) {

I'm pretty sure it would be enough to reload the main target only? Devin?

If you keep this code, please remove unnecessary braces.

>
Source/WebInspectorUI/UserInterface/Controllers/WebInspectorExtensionController
.js:151
> +	   }

This should return a promise in the happy path so that the originating call to
-[WKInspectorExtension reloadWithOptions:] does not complete prior to the
command being handled (i.e., it could return an error that should be
propagated).

> Source/WebKit/UIProcess/API/Cocoa/_WKInspectorExtension.h:67
> + * @param userAgent If specified, sets a custom user agent for the page to
be sent in the `User-Agent` header and returned by calls to
`navigator.userAgent` made by scripts running in the page.

I think these options are applicable to the next navigation only, right? I
would mention that in the @discussion.

> Source/WebKit/UIProcess/API/Cocoa/_WKInspectorExtension.h:72
> +- (void)reloadIgnoringCache:(BOOL)ignoreCache userAgent:(NSString
*)userAgent injectedScript:(NSString *)injectedScript
completionHandler:(void(^)(NSError * _Nullable, NSDictionary * _Nullable
result))completionHandler;

There is no return value for this operation, you can drop the 2nd parameter of
the completion handler and associated comments/code (throughout).