[Webkit-unassigned] [Bug 51262] WebPageProxy should delete its backing store after not painting for a while

Sat Feb 12 08:34:15 PST 2011

https://bugs.webkit.org/show_bug.cgi?id=51262

--- Comment #33 from Adam Roben (aroben) <aroben at apple.com>  2011-02-12 08:34:15 PST ---
Here's my latest thinking on the policy-less implementation:

DrawingAreaProxyImpl has some state that, when it changes, causes all operations performed before that state change to become invalid. When painting, we need to have performed at least one Update for the most recent state; otherwise, we'll be painting old/incorrect bits into the view.

Currently, this state consists of a single piece of data: the view's size. Whenever the view's size changes, we tell the web process, and discard any operations that occur until the web process lets us know that it has updated to match the new state (i.e., size). This is accomplished via a few means:

1) The Messages::DrawingArea::SetSize message
This is how we tell the web process when the state (i.e., size) changes.

2) The Messages::DrawingAreaProxy::DidSetSize message
This is how the web process tells us it has updated to match the new state (i.e., size).

3) The DrawingAreaProxyImpl::m_isWaitingForDidSetSize member
This is set to true when we send SetSize, and set back to false when we receive DidSetSize. This is used for two purposes:
  a) When DrawingAreaProxyImpl is asked to paint into the native view, if m_isWaitingForDidSetSize is true then we know we need to wait until the web process has updated to match the new state (i.e., size) before painting. This is accomplished via the waitForAndDispatchDidSetSize function. (If the web process takes too long to respond, we'll just paint whatever old data we have.)
  b) When the view's size changes, if m_isWaitingForDidSetSize is true then we don't send a new SetSize message. Instead, we send a new SetSize message when we receive DidSetSize, if needed. This prevents us from sending many redundant SetSize messages (if the view is being resized faster than the web process can respond).

4) The sequence numbers passed from the web process to the UI process, and the DrawingAreaProxyImpl::m_lastDidSetSizeSequenceNumber member
This is used to ignore messages that are for an old state (i.e., size). Every time the web process sends a message to DrawingAreaProxyImpl, it includes a monotonically increasing sequence number. When a DidSetSize message is received, DrawingAreaProxyImpl::m_lastDidSetSizeSequenceNumber is updated to match the DidSetSize message's sequence number. If DrawingAreaProxyImpl receives a message with a sequence number less than m_lastDidSetSizeSequenceNumber, it knows that it is for an old state (i.e., size) and ignores it. This is needed because waitForAndDispatchDidSetSize allows us to receive messages out of order.

I'm going to be adding a new piece of data to the state: the backing store. Whenever the backing store is thrown away, we now have entered a new state where any old operations are no longer valid. Here's how I think we can accomplish this:

I) Transform the "sequence number" into a "state ID", and move control of the state ID to the UI process
DrawingAreaProxyImpl would be in charge of incrementing the state ID whenever the state (i.e., size) changes. The new state ID would be passed to the web process in the SetSize message, and DrawingAreaImpl would store the state ID. DrawingAreaImpl would then pass its stored state ID back to the UI process the same way the sequence number is currently passed.

II) Perform renames to match the "sequence number" -> "state ID" transformation:
  a) Messages::DrawingArea::SetSize -> UpdateState
  b) Messages::DrawingAreaProxy::DidSetSize -> DidUpdateState
  c) DrawingAreaProxyImpl::m_isWaitingForDidSetSize -> m_isWaitingForDidUpdateState
  d) DrawingAreaProxyImpl::waitForAndDispatchDidSetSize -> waitForAndDispatchDidUpdateState
  e) DrawingAreaImpl::m_inSetSize -> m_inUpdateState

III) Add a boolean argument to the UpdateState message that specifies whether the update needs to happen immediately or can be deferred until the next paint
DrawingAreaImpl would always store the last state ID it sent to the UI process in a data member. For deferred state updates, it would only update the current state ID, not the last sent state ID. The next time DrawingAreaImpl does something that would end up sending a message to the UI process, it checks whether the last sent state ID matches the current state ID. If it matches, it proceeds as normal, but if it doesn't, it sends a DidUpdateState message instead of whatever message it was going to send. (Implicit in this is the requirement that DidUpdateState contain a superset of the information it was going to send in the normal message. This is currently the case: DidSetSize includes all the information that any other message can contain, and more.)

With this groundwork in place, I think throwing away a backing store becomes this:

A) When DrawingAreaProxyImpl throws away the backing store, update the state ID and send a deferred UpdateState message to the web process.
B) When DrawingAreaProxyImpl is asked to paint into the native view, if m_isWaitingForDidUpdateState is true, send a non-deferred UpdateState message to the web process without incrementing the state ID before calling waitForAndDispatchDidUpdateState.
C) When DrawingAreaImpl gets an UpdateState message, if the state ID in the message matches DrawingAreaImpl's current state ID, ignore the message (because we've already updated to match this state).

I believe this addresses all the problems mentioned so far, except for comment 24. I also think the generalization of the [Did]SetSize logic means that adding the logic to deal with throwing away a backing store can be added to DrawingArea[Proxy]Impl without making those classes harder to understand.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.