[webkit-reviews] review granted: [Bug 129188] [iOS][WebKit2] Adopt SPI for managing tabs : [Attachment 225165] Patch

[bugzilla-daemon at webkit.org]

Gavin Barraclough <barraclough at apple.com> has granted Pratik Solanki
<psolanki at apple.com>'s request for review:
Bug 129188: [iOS][WebKit2] Adopt SPI for managing tabs
https://bugs.webkit.org/show_bug.cgi?id=129188

Attachment 225165: Patch
https://bugs.webkit.org/attachment.cgi?id=225165&action=review

------- Additional Comments from Gavin Barraclough <barraclough at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=225165&action=review


> Source/WebKit2/UIProcess/WebPageProxy.cpp:1028
> +    if (mayHaveChanged & ViewState::IsInWindow)

Hey Pratik, I think you should be able to check "changed" instead of
"mayHaveChanged" – you should probably only need to update the process state if
visibility actually has changed, right?
(I'm probably going to remove mayHaveChanged soon).

> Source/WebKit2/UIProcess/ios/WebProcessProxyIOS.mm:83
> +    copyValuesToVector(m_pageMap, pages);

I think you could just iterate the page map directly, instead of copying to a
vector, using:
	for (const auto& page : m_pageMap.values())