Adopt assertion SPI for managing tab lifecycle.
<rdar://problem/15939827>
Created attachment 224933 [details] Patch
Member variables and functions could do with a better name but I couldn't come up with anything. Let me know if this is fitting in well architecturally or if it needs to be somewhere else.
Comment on attachment 224933 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=224933&action=review > Source/WebKit2/Configurations/WebKit2.xcconfig:35 > +FRAMEWORK_AND_LIBRARY_LDFLAGS_iphonesimulator = -lobjc -framework CFNetwork -framework CoreFoundation -framework CoreGraphics -framework CoreText -framework Foundation -framework GraphicsServices -framework ImageIO -framework UIKit -framework WebKit -lMobileGestalt -lassertion_extension; Why is this only needed in the simulator? > Source/WebKit2/Platform/IPC/Connection.h:283 > +public: > + xpc_connection_t xpcConnection() { return m_xpcConnection; } > + I would move this up to just below the line that reads: static bool identifierIsNull(Identifier identifier) { return identifier.port == MACH_PORT_NULL; } > Source/WebKit2/UIProcess/WebPageProxy.cpp:1031 > +#if PLATFORM(IOS) > + if (mayHaveChanged & ViewState::IsInWindow) > + process().setProcessState(m_viewState & ViewState::IsInWindow ? WebProcessProxy::Foreground : WebProcessProxy::Background); > +#endif Is this accounting for the case where a process has multiple WebPageProxies? > Source/WebKit2/UIProcess/WebProcessProxy.h:228 > +#if PLATFORM(IOS) > +public: > + enum ProcessState { > + Background, > + Foreground > + }; > + > + void setProcessState(ProcessState); > + > +private: > + void updateProcessState(); > + > + ProcessState m_processState; > +#endif If possible, I would love to keep this not iOS specific (with other ports just always being in the Foreground state). I would bet Gavin has opinions on this as well, given all the time he has spent lately, cleaning up ViewState stuff.
Comment on attachment 224933 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=224933&action=review >> Source/WebKit2/Configurations/WebKit2.xcconfig:35 >> +FRAMEWORK_AND_LIBRARY_LDFLAGS_iphonesimulator = -lobjc -framework CFNetwork -framework CoreFoundation -framework CoreGraphics -framework CoreText -framework Foundation -framework GraphicsServices -framework ImageIO -framework UIKit -framework WebKit -lMobileGestalt -lassertion_extension; > > Why is this only needed in the simulator? FRAMEWORK_AND_LIBRARY_LDFLAGS_iphoneos (below) includes FRAMEWORK_AND_LIBRARY_LDFLAGS_iphonesimulator. iphonesimucator seems to be for frameworks applicable to both. iphoneos is hardware specific. >> Source/WebKit2/UIProcess/WebPageProxy.cpp:1031 >> +#endif > > Is this accounting for the case where a process has multiple WebPageProxies? Hmm.. So you mean when we have one web process handling multiple WebPageProxy. I don't think this is handling that. I see we have a vector of WebPageProxy objects in WebProcessProxy. Should we be going through this list every time so we don't mark a process as being background inadvertently? >> Source/WebKit2/UIProcess/WebProcessProxy.h:228 >> +#endif > > If possible, I would love to keep this not iOS specific (with other ports just always being in the Foreground state). I would bet Gavin has opinions on this as well, given all the time he has spent lately, cleaning up ViewState stuff. Ok. I can do that and have empty implementations for other ports.
Created attachment 225165 [details] Patch
Simpler patch that doesn't keep the state in WebProcessProxy. We don't really need to. Though it might be advantageous to have a local state for debugging purposes.
Comment on attachment 225165 [details] Patch 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())
Committed r164737: <http://trac.webkit.org/changeset/164737>