Bug 198214

Summary: Update messages_unittest.py after r245715
Product: WebKit Reporter: youenn fablet <youennf>
Component: Tools / TestsAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, ap, cdumez, commit-queue, ryanhaddad, webkit-bot-watchers-bugzilla, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing none

Description youenn fablet 2019-05-23 22:29:46 PDT
Update messages_unittest.py after https://trac.webkit.org/changeset/245715 to cope with messages.py changes.
Comment 1 youenn fablet 2019-05-23 22:33:25 PDT
Created attachment 370563 [details]
Patch
Comment 2 Alexey Proskuryakov 2019-05-24 09:58:19 PDT
Comment on attachment 370563 [details]
Patch

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

> Source/WebKit/ChangeLog:3
> +        Update messages_unittest.py after https://trac.webkit.org/changeset/245715

Probably enough to say r245715 here.

> Source/WebKit/ChangeLog:9
> +        Small update to messages.py so that cancelReply is called in case of decoding error.

The patch looks good overall, but the ChangeLog confuses me. Isn't the change to messages.py more important? I know that we don't expect this path to be taken, but it has some potential for changing observed behavior anyway. If that happens, everyone will be confused by the title.
Comment 3 youenn fablet 2019-05-24 10:58:53 PDT
(In reply to Alexey Proskuryakov from comment #2)
> Comment on attachment 370563 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=370563&action=review
> 
> > Source/WebKit/ChangeLog:3
> > +        Update messages_unittest.py after https://trac.webkit.org/changeset/245715
> 
> Probably enough to say r245715 here.

Will do.

> > Source/WebKit/ChangeLog:9
> > +        Small update to messages.py so that cancelReply is called in case of decoding error.
> 
> The patch looks good overall, but the ChangeLog confuses me. Isn't the
> change to messages.py more important? I know that we don't expect this path
> to be taken, but it has some potential for changing observed behavior
> anyway. If that happens, everyone will be confused by the title.

Will update the changelog.
This patch does not change observed behavior though, r245715 does.

The changes here to messages.py are just to call cancelReply in case of decode error instead of calling completionHandler(IPC::AsyncReplyError<>...) as done in r245715.
Comment 4 youenn fablet 2019-05-24 11:08:04 PDT
Created attachment 370577 [details]
Patch for landing
Comment 5 WebKit Commit Bot 2019-05-24 11:55:35 PDT
Comment on attachment 370577 [details]
Patch for landing

Clearing flags on attachment: 370577

Committed r245749: <https://trac.webkit.org/changeset/245749>
Comment 6 WebKit Commit Bot 2019-05-24 11:55:36 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2019-05-24 11:56:30 PDT
<rdar://problem/51115730>
Comment 8 Alexey Proskuryakov 2019-05-24 13:00:50 PDT
> The changes here to messages.py are just to call cancelReply in case of decode error instead of calling completionHandler(IPC::AsyncReplyError<>...) as done in r245715.

Which is a change in behavior.