Bug 173636

Summary: [iOS DnD] [WK2] Add drag-and-drop release logging around WKContentView
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: WebKit2Assignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, bdakin, commit-queue, megan_gardner, thorton
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Add logging in one more spot.
thorton: review+
Patch for landing none

Description Wenson Hsieh 2017-06-20 21:10:15 PDT 
Adding RELEASE_LOGs to key points in the drag-and-drop lifecycle will make sysdiagnoses actually contain useful information for debugging.
Comment 1 Wenson Hsieh 2017-06-20 21:13:38 PDT 
Created attachment 313484 [details]
Patch
Comment 2 Wenson Hsieh 2017-06-20 23:08:39 PDT 
Created attachment 313493 [details]
Add logging in one more spot.
Comment 3 Tim Horton 2017-06-21 10:57:30 PDT 
Comment on attachment 313493 [details]
Add logging in one more spot.

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

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:4518
> +            RELEASE_LOG(DragAndDrop, "Drag session failed: %p (no draggable content at %s)", session, NSStringFromCGPoint(dragOrigin).UTF8String);

Building an NSString just to print things? Why not just print the components?

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:4531
> +        RELEASE_LOG(DragAndDrop, "Drag session requested: %p at element bounds: %s", session, NSStringFromCGRect(state.elementBounds).UTF8String);

And again

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:4721
> +        RELEASE_LOG(DragAndDrop, "Loaded data at %tu files", fileURLs.count);

*at*?
Comment 4 Wenson Hsieh 2017-06-21 11:00:19 PDT 
Comment on attachment 313493 [details]
Add logging in one more spot.

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

Thanks!

>> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:4518
>> +            RELEASE_LOG(DragAndDrop, "Drag session failed: %p (no draggable content at %s)", session, NSStringFromCGPoint(dragOrigin).UTF8String);
> 
> Building an NSString just to print things? Why not just print the components?

Sounds good -- I'll log the CGPoint x/y directly.

>> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:4531
>> +        RELEASE_LOG(DragAndDrop, "Drag session requested: %p at element bounds: %s", session, NSStringFromCGRect(state.elementBounds).UTF8String);
> 
> And again

👍

>> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:4721
>> +        RELEASE_LOG(DragAndDrop, "Loaded data at %tu files", fileURLs.count);
> 
> *at*?

I suppose "into" would be more fitting here -- fixed.
Comment 5 Wenson Hsieh 2017-06-21 11:15:08 PDT 
Created attachment 313534 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2017-06-21 11:54:48 PDT 
Comment on attachment 313534 [details]
Patch for landing

Clearing flags on attachment: 313534

Committed r218642: <http://trac.webkit.org/changeset/218642>