[webkit-dev] Platform specific fields on WebCore::HistoryItem

[Darin Fisher]

On Tue, Dec 21, 2010 at 2:56 PM, Brady Eidson <beidson at apple.com> wrote:

>
> On Dec 21, 2010, at 1:57 PM, Darin Fisher wrote:
>
>
>
> On Tue, Dec 21, 2010 at 11:49 AM, Darin Fisher <darin at chromium.org> wrote:
>
>>
>>
>> On Tue, Dec 21, 2010 at 11:46 AM, Brady Eidson <beidson at apple.com> wrote:
>>
>>>
>>> On Dec 21, 2010, at 11:39 AM, Darin Fisher wrote:
>>>
>>> I'm working on fixing some session history bugs related to a
>>> HistoryItem's URL property changing.
>>> See for example the call to HistoryItem::setURL in
>>> HistoryController::updateForReload [1].
>>>
>>> I'm curious about the platform specific fields on WebCore::HistoryItem.
>>>  *** Do any of those need to
>>> be updated when the URL of the HistoryItem changes? ***
>>>
>>> Here are the fields I'm referring to:
>>>
>>> class HistoryItem ... {
>>> private:
>>>     ...
>>> #if PLATFORM(MAC)
>>>     RetainPtr<id> m_viewState;
>>>
>>>
>>> This is used for the Page Cache only.  The URL had sure better not change
>>> while the page is cached!
>>>
>>
>> OK, I will assert that it is 0.
>>
>>
> Awesome, I found a layout test where the URL of the HistoryItem changes,
> and there is an associated cached page for the HistoryItem!  I presume the
> right fix is to clear the cached page.
>
>
> No no no!!!
>
> Why is anyone trying to do anything to the URL of an item that represents a
> cached page???
>

Hmm, the testcase is simply assigning location within onload (the equivalent
of location.replace).  The document is getting replaced with a new document,
but the same HistoryItem is being overwritten with the URL of the new
document.

I'm not super familiar with the cached page implementation since Chromium
does not enable it, but if that system assumes there is a 1:1 relationship
between HistoryItems and cached pages / documents, then that could be
problematic.

I'm toying with calling HistoryController::invalidateCurrentItemCachedPage()
in this case, but maybe that would be suboptimal?




>
> In case you are interested, the layout test where this happens is:
>  fast/history/timed-refresh-in-cached-frame.html
>
>
> Shouldn't the timed refresh be paused while the page is cached, and only
> resume once the page is brought out?
>

The issue comes up well before the <meta refresh> timer expires.  I could
remove that bit of the testcase, and the issue would remain.  This is all
about issuing a client redirect on a page that has been cached.

-Darin



>
> In other words, should the URL only be changed after the page is restored?
>
> ~Brady
>
>
>
> -Darin
>
>
>>
>>
>>>
>>>     OwnPtr<HashMap<String, RetainPtr<id> > > m_transientProperties;
>>>
>>>
>>> This is to support arbitrary WebKit Mac API and has nothing to do with
>>> the URL identity of the item.
>>>
>>
>> OK, thanks!
>>
>> -Darin
>>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20101221/5e06c830/attachment-0001.html>