[webkit-reviews] review granted: [Bug 215239] Always suspend IDB work when network process is prepared to suspend : [Attachment 406119] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Sihui Liu <sihui_liu at apple.com>'s
request for review:
Bug 215239: Always suspend IDB work when network process is prepared to suspend
https://bugs.webkit.org/show_bug.cgi?id=215239

Attachment 406119: Patch

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




--- Comment #3 from Geoffrey Garen <ggaren at apple.com> ---
Comment on attachment 406119
  --> https://bugs.webkit.org/attachment.cgi?id=406119
Patch

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

r=me

It's a little unfortunate that all transactions started before suspension will
be aborted, in contrast to transactions started after suspension, which will
just be paused and delayed. Is there a way to delay in progress transactions
instead of aborting them? I guess not, since an in progress transaction holds a
database file lock? Would it be crazy to abort a transaction without signaling
an error and then put it back in the queue to try again when we resume?

> Source/WebKit/ChangeLog:9
> +	   We do not suspend IDB work in network process when there is ongoing
transaction because network process is 

network process => the network process
ongoing transaction => an ongoing transaction
network process => the network process

> Source/WebKit/ChangeLog:10
> +	   going to ask UI process to hold process assertion for it. However,
it is possible that the request from network

UI process => the UI process
process assertion => a process assertion
network => the network

> Source/WebKit/ChangeLog:11
> +	   process does not reach UI process before UI process thinks think it
is Okay to put network process to suspend. 

UI process => the UI process
UI process => the UI process
thinks think -> thinks
Okay => okay
put network process to suspend => suspend the network process

I believe it's runningboard, and not the UI process, that ultimately suspends
the network process. So, I think this comment should talk about runningboard.
Or maybe you could talk about the UI process dropping its foreground assertion
on the network process? Is that what ultimately triggers runningboard to
suspend the network process?

> Source/WebKit/ChangeLog:12
> +	   In this case, IDB in network process can continue its transaction,
holding file locks, and network process will 

network process => the network process
network process => the network process

> Source/WebKit/ChangeLog:13
> +	   be killed when it becomes suspened. Network process crash can cause
worse result than aborting ongoing 

Network process crash => A network process crash
worse result => a worse result

> Source/WebKit/ChangeLog:15
> +	   let's just suspend IDB work when network process receives
prepareToSuspend.

network process => the network process