Web Inspector: Don't allow docking to a hidden web view

• Component: Web Inspector
• Status: RESOLVED FIXED

[Timothy Hatcher]

When the Inspected Web View is hidden, don't try to attach.

[Timothy Hatcher]

Created attachment 405380 [details]
Patch

[Devin Rousso]

Comment on attachment 405380 [details]
Patch

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

r=me

> Source/WebKit/UIProcess/Inspector/WebInspectorProxy.cpp:282
> +    if (!m_inspectedPage || !m_inspectorPage || !platformCanAttach(m_canAttach))

NIT: can you use `platformCanAttach(canAttach())` instead?  This is what other callers do, and it covers the future case where `canAttach` becomes more complex :)

> Source/WebKit/UIProcess/Inspector/WebInspectorProxy.cpp:314
> +    if (!m_inspectedPage || !m_inspectorPage || (!m_isAttached && m_isVisible))

Is `m_isVisible` necessary?

[Timothy Hatcher]

Created attachment 405386 [details]
Patch

[Timothy Hatcher]

Comment on attachment 405380 [details]
Patch

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

>> Source/WebKit/UIProcess/Inspector/WebInspectorProxy.cpp:282
>> +    if (!m_inspectedPage || !m_inspectorPage || !platformCanAttach(m_canAttach))
> 
> NIT: can you use `platformCanAttach(canAttach())` instead?  This is what other callers do, and it covers the future case where `canAttach` becomes more complex :)

Okay.

>> Source/WebKit/UIProcess/Inspector/WebInspectorProxy.cpp:314
>> +    if (!m_inspectedPage || !m_inspectorPage || (!m_isAttached && m_isVisible))
> 
> Is `m_isVisible` necessary?

Yes, since m_isAttached could be stale after close.

[EWS]

Committed r264999: <https://trac.webkit.org/changeset/264999>

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

[Radar WebKit Bug Importer]

<rdar://problem/66231117>