[webkit-reviews] review denied: [Bug 35274] REGRESSION(r55107): Inspector docking is busted. : [Attachment 49315] 3rd time's the charm

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Brady Eidson
<beidson at apple.com>'s request for review:
Bug 35274: REGRESSION(r55107): Inspector docking is busted.
https://bugs.webkit.org/show_bug.cgi?id=35274

Attachment 49315: 3rd time's the charm
https://bugs.webkit.org/attachment.cgi?id=49315&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
The IDL change looks fine. I would only ask that you check
InspectorFrontendHost.canAttach for existence before calling into it. Otherwise
Chromium gets doomed. We have not yet upstreamed the frontend interface. I
don't follow the general logic though:

> +    return constrainedAttachedWindowHeight(preferredHeight,
inspectedPageHeight) <= inspectedPageHeight * maximumAttachedHeightRatio;

If I inline constrainedAttachedWindowHeight, I get:

max(minimumAttachedHeight, min(preferredHeight, inspectedPageHeight *
maximumAttachedHeightRatio)) <= inspectedPageHeight *
maximumAttachedHeightRatio

this evolves to

max(minimumAttachedHeight, {x | x <= inspectedPageHeight *
maximumAttachedHeightRatio}) <= inspectedPageHeight *
maximumAttachedHeightRatio

So this thing is only false if minimumAttachedHeight >	inspectedPageHeight *
maximumAttachedHeightRatio. This looks like a strange check. What do I miss? r-
unless you convince me this is the right check.