[webkit-reviews] review granted: [Bug 210177] REGRESSION (Safari 13.1?): Web Inspector: Debugger hang at breakpoint when using Keyboard Maestro : [Attachment 396120] Patch

[bugzilla-daemon at webkit.org]

Joseph Pecoraro <joepeck at webkit.org> has granted Devin Rousso
<drousso at apple.com>'s request for review:
Bug 210177: REGRESSION (Safari 13.1?): Web Inspector: Debugger hang at
breakpoint when using Keyboard Maestro
https://bugs.webkit.org/show_bug.cgi?id=210177

Attachment 396120: Patch

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




--- Comment #5 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 396120
  --> https://bugs.webkit.org/attachment.cgi?id=396120
Patch

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

r=me

> Source/WebCore/inspector/mac/PageScriptDebugServerMac.mm:2
> + * Copyright (C) 2020 Apple Inc. All rights reserved.

This is effectively a copy of `Copyright (C) 2008-2018 Apple` code.

> Source/WebCore/inspector/mac/PageScriptDebugServerMac.mm:26
> +#include "config.h"

Style: #import

> Source/WebCore/inspector/mac/PageScriptDebugServerMac.mm:33
> +#endif // PLATFORM(MAC) && ENABLE(WEBPROCESS_NSRUNLOOP)

I don't think the endif comment is useful in this case given the small block.
In fact, we often don't even bother with ENABLE guards around an include like
this since it would be fine to include for all cases and just not have the
symbols be used. It necessary reduces clutter to remove guards around includes.
Only if it had a meaningful impact on compile times might it be worth it.

> Source/WebCore/inspector/mac/PageScriptDebugServerMac.mm:51
> +#endif // ENABLE(WEBPROCESS_NSRUNLOOP)

Ditto.