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

[Darin Fisher]

On Tue, Dec 21, 2010 at 3:59 PM, Darin Fisher <darin at chromium.org> wrote:

> 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?
>

OK, false alarm.  The issue is as follows:

The cached page corresponding to the new document is assigned to the
HistoryItem before we change its URL.  In other words, I don't need to do
anything special.

-Darin



>
>
>
>
>>
>> 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/5b0f8b8a/attachment.html>