Bug 209607 - Add SPI to specify whether file upload panels are uploading to an enterprise-managed destination
Summary: Add SPI to specify whether file upload panels are uploading to an enterprise-...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-03-26 10:30 PDT by David Quesada
Modified: 2020-03-26 11:58 PDT (History)
4 users (show)

See Also:


Attachments
Patch (14.96 KB, patch)
2020-03-26 10:43 PDT, David Quesada
darin: review+
Details | Formatted Diff | Diff
Patch for landing (14.93 KB, patch)
2020-03-26 11:15 PDT, David Quesada
david_quesada: commit-queue-
Details | Formatted Diff | Diff
Patch for landing II (14.94 KB, patch)
2020-03-26 11:27 PDT, David Quesada
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Quesada 2020-03-26 10:30:34 PDT
rdar://problem/60888386
Comment 1 David Quesada 2020-03-26 10:43:36 PDT
Created attachment 394626 [details]
Patch
Comment 2 Darin Adler 2020-03-26 10:54:51 PDT
Comment on attachment 394626 [details]
Patch

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

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:6609
> +    auto webView = _webView.get();

A little strange to make this local variable but only use it once.

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:6614
> +    if ([uiDelegate respondsToSelector:@selector(_webView:fileUploadPanelContentIsManagedWithInitiatingFrame:)])
> +        return [uiDelegate _webView:webView.get() fileUploadPanelContentIsManagedWithInitiatingFrame:wrapper(API::FrameInfo::create(WTFMove(_frameInfoForFileUploadPanel), _page.get()))];
> +
> +    return NO;

I would have used && here.
Comment 3 David Quesada 2020-03-26 11:07:48 PDT
(In reply to Darin Adler from comment #2)
> Comment on attachment 394626 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=394626&action=review
> 
> > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:6609
> > +    auto webView = _webView.get();
> 
> A little strange to make this local variable but only use it once.

It's used twice -- on line 6610 to call -UIDelegate, and again on line 6612 for the first parameter of _webView:fileUpload...

> 
> > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:6614
> > +    if ([uiDelegate respondsToSelector:@selector(_webView:fileUploadPanelContentIsManagedWithInitiatingFrame:)])
> > +        return [uiDelegate _webView:webView.get() fileUploadPanelContentIsManagedWithInitiatingFrame:wrapper(API::FrameInfo::create(WTFMove(_frameInfoForFileUploadPanel), _page.get()))];
> > +
> > +    return NO;
> 
> I would have used && here.

Sure.
Comment 4 David Quesada 2020-03-26 11:15:08 PDT
Created attachment 394631 [details]
Patch for landing
Comment 5 Darin Adler 2020-03-26 11:19:08 PDT
Comment on attachment 394631 [details]
Patch for landing

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

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:6611
> +    auto webView = _webView.get();
> +    id <WKUIDelegatePrivate> uiDelegate = static_cast<id <WKUIDelegatePrivate>>([webView UIDelegate]);
> +    return [uiDelegate respondsToSelector:@selector(_webView:fileUploadPanelContentIsManagedWithInitiatingFrame:)] && [uiDelegate _webView:webView.get() fileUploadPanelContentIsManagedWithInitiatingFrame:wrapper(API::FrameInfo::create(WTFMove(_frameInfoForFileUploadPanel), _page.get()))];

Gotta admit I am surprised we can call _webView.get() and then call get() again on that!

Also, sorry I suggested the && but didn’t point out the cool way to format it:

    return [uiDelegate respondsToSelector:@selector(_webView:fileUploadPanelContentIsManagedWithInitiatingFrame:)]
        && [uiDelegate _webView:webView.get() fileUploadPanelContentIsManagedWithInitiatingFrame:wrapper(API::FrameInfo::create(WTFMove(_frameInfoForFileUploadPanel), _page.get()))];
Comment 6 David Quesada 2020-03-26 11:27:06 PDT
(In reply to Darin Adler from comment #5)
> Comment on attachment 394631 [details]
> Patch for landing
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=394631&action=review
> 
> > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:6611
> > +    auto webView = _webView.get();
> > +    id <WKUIDelegatePrivate> uiDelegate = static_cast<id <WKUIDelegatePrivate>>([webView UIDelegate]);
> > +    return [uiDelegate respondsToSelector:@selector(_webView:fileUploadPanelContentIsManagedWithInitiatingFrame:)] && [uiDelegate _webView:webView.get() fileUploadPanelContentIsManagedWithInitiatingFrame:wrapper(API::FrameInfo::create(WTFMove(_frameInfoForFileUploadPanel), _page.get()))];
> 
> Gotta admit I am surprised we can call _webView.get() and then call get()
> again on that!

Yup. The first get() converts from a WeakObjCPtr to RetainPtr, then the second get() returns the raw pointer.

> 
> Also, sorry I suggested the && but didn’t point out the cool way to format
> it:
> 
>     return [uiDelegate
> respondsToSelector:@selector(_webView:
> fileUploadPanelContentIsManagedWithInitiatingFrame:)]
>         && [uiDelegate _webView:webView.get()
> fileUploadPanelContentIsManagedWithInitiatingFrame:wrapper(API::FrameInfo::
> create(WTFMove(_frameInfoForFileUploadPanel), _page.get()))];

I like that formatting better. I'll use it.
Comment 7 David Quesada 2020-03-26 11:27:58 PDT
Created attachment 394633 [details]
Patch for landing II

Fixed the formatting in -fileUploadPanelDestinationIsManaged:.
Comment 8 EWS 2020-03-26 11:58:48 PDT
Committed r259061: <https://trac.webkit.org/changeset/259061>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 394633 [details].