Changeset 54620 in webkit


Ignore:
Timestamp:
Feb 10, 2010 1:26:31 PM (14 years ago)
Author:
kov@webkit.org
Message:

WebCore

2010-02-10 Gustavo Noronha Silva <Gustavo Noronha Silva>

Reviewed by Xan Lopez.

[GTK] Hits assertion on history back, with page cache enabled, in specific conditions
https://bugs.webkit.org/show_bug.cgi?id=34773

When unsetting the adjustments from a ScrollView, also disconnect
them from the Scrollbars.

Test: fast/frames/frame-crash-with-page-cache.html

  • platform/gtk/ScrollViewGtk.cpp: (WebCore::ScrollView::setGtkAdjustments):
  • platform/gtk/ScrollbarGtk.cpp: (ScrollbarGtk::~ScrollbarGtk): (ScrollbarGtk::detachAdjustment):
  • platform/gtk/ScrollbarGtk.h:

LayoutTests

2010-02-10 Gustavo Noronha Silva <gustavo.noronha@collabora.co.uk>

Reviewed by Xan Lopez.

[GTK] Hits assertion on history back, with page cache enabled, in specific conditions
https://bugs.webkit.org/show_bug.cgi?id=34773

  • fast/frames/frame-crash-with-page-cache.html: Added.
  • fast/frames/resources/cached-page-1.html: Added.
  • fast/frames/resources/cached-page-2.html: Added.
  • fast/frames/resources/cached-page-3.html: Added.
  • fast/frames/resources/cached-page-iframe.html: Added.

WebKit/gtk

2010-02-09 Gustavo Noronha Silva <Gustavo Noronha Silva>

Reviewed by Xan Lopez.

[GTK] Hits assertion on history back, with page cache enabled, in specific conditions
https://bugs.webkit.org/show_bug.cgi?id=34773

Make sure cached frames have their scrollbars disconnected from
the WebView's adjustments.

  • WebCoreSupport/FrameLoaderClientGtk.cpp: (WebKit::FrameLoaderClient::savePlatformDataToCachedFrame):
Location:
trunk
Files:
6 added
9 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r54618 r54620  
     12010-02-10  Gustavo Noronha Silva  <gustavo.noronha@collabora.co.uk>
     2
     3        Reviewed by Xan Lopez.
     4
     5        [GTK] Hits assertion on history back, with page cache enabled, in specific conditions
     6        https://bugs.webkit.org/show_bug.cgi?id=34773
     7
     8        * fast/frames/frame-crash-with-page-cache.html: Added.
     9        * fast/frames/resources/cached-page-1.html: Added.
     10        * fast/frames/resources/cached-page-2.html: Added.
     11        * fast/frames/resources/cached-page-3.html: Added.
     12        * fast/frames/resources/cached-page-iframe.html: Added.
     13
    1142010-02-09  Alexey Proskuryakov  <ap@apple.com>
    215
  • trunk/WebCore/ChangeLog

    r54618 r54620  
     12010-02-10  Gustavo Noronha Silva  <gns@gnome.org>
     2
     3        Reviewed by Xan Lopez.
     4
     5        [GTK] Hits assertion on history back, with page cache enabled, in specific conditions
     6        https://bugs.webkit.org/show_bug.cgi?id=34773
     7
     8        When unsetting the adjustments from a ScrollView, also disconnect
     9        them from the Scrollbars.
     10
     11        Test: fast/frames/frame-crash-with-page-cache.html
     12
     13        * platform/gtk/ScrollViewGtk.cpp:
     14        (WebCore::ScrollView::setGtkAdjustments):
     15        * platform/gtk/ScrollbarGtk.cpp:
     16        (ScrollbarGtk::~ScrollbarGtk):
     17        (ScrollbarGtk::detachAdjustment):
     18        * platform/gtk/ScrollbarGtk.h:
     19
    1202010-02-09  Alexey Proskuryakov  <ap@apple.com>
    221
  • trunk/WebCore/platform/ScrollView.h

    r53877 r54620  
    315315#if PLATFORM(GTK)
    316316public:
    317     void setGtkAdjustments(GtkAdjustment* hadj, GtkAdjustment* vadj);
     317    void setGtkAdjustments(GtkAdjustment* hadj, GtkAdjustment* vadj, bool resetValues = true);
    318318    GtkAdjustment* m_horizontalAdjustment;
    319319    GtkAdjustment* m_verticalAdjustment;
  • trunk/WebCore/platform/gtk/ScrollViewGtk.cpp

    r52417 r54620  
    33 * Copyright (C) 2006 Michael Emmel mike.emmel@gmail.com
    44 * Copyright (C) 2007, 2009 Holger Hans Peter Freyther
    5  * Copyright (C) 2008 Collabora Ltd.
     5 * Copyright (C) 2008, 2010 Collabora Ltd.
    66 *
    77 * All rights reserved.
     
    7777 *   (hadj && vadj) || (!hadj && !vadj)
    7878 */
    79 void ScrollView::setGtkAdjustments(GtkAdjustment* hadj, GtkAdjustment* vadj)
     79void ScrollView::setGtkAdjustments(GtkAdjustment* hadj, GtkAdjustment* vadj, bool resetValues)
    8080{
    8181    ASSERT(!hadj == !vadj);
     
    8686    // Reset the adjustments to a sane default
    8787    if (m_horizontalAdjustment) {
     88        ScrollbarGtk* hScrollbar = reinterpret_cast<ScrollbarGtk*>(horizontalScrollbar());
     89        if (hScrollbar)
     90            hScrollbar->attachAdjustment(m_horizontalAdjustment);
     91
     92        ScrollbarGtk* vScrollbar = reinterpret_cast<ScrollbarGtk*>(verticalScrollbar());
     93        if (vScrollbar)
     94            vScrollbar->attachAdjustment(m_verticalAdjustment);
     95
     96        // We used to reset everything to 0 here, but when page cache
     97        // is enabled we reuse FrameViews that are cached. Since their
     98        // size is not going to change when being restored, (which is
     99        // what would cause the upper limit in the adjusments to be
     100        // set in the normal case), we make sure they are up-to-date
     101        // here. This is needed for the parent scrolling widget to be
     102        // able to report correct values.
    88103        m_horizontalAdjustment->lower = 0;
    89         m_horizontalAdjustment->upper = 0;
    90         m_horizontalAdjustment->value = 0;
     104        m_horizontalAdjustment->upper = resetValues ? 0 : frameRect().width();
     105        m_horizontalAdjustment->value = resetValues ? 0 : scrollOffset().width();
    91106        gtk_adjustment_changed(m_horizontalAdjustment);
    92107        gtk_adjustment_value_changed(m_horizontalAdjustment);
    93108
    94109        m_verticalAdjustment->lower = 0;
    95         m_verticalAdjustment->upper = 0;
    96         m_verticalAdjustment->value = 0;
     110        m_verticalAdjustment->upper = resetValues ? 0 : frameRect().height();
     111        m_verticalAdjustment->value = resetValues ? 0 : scrollOffset().height();
    97112        gtk_adjustment_changed(m_verticalAdjustment);
    98113        gtk_adjustment_value_changed(m_verticalAdjustment);
     114    } else {
     115        ScrollbarGtk* hScrollbar = reinterpret_cast<ScrollbarGtk*>(horizontalScrollbar());
     116        if (hScrollbar)
     117            hScrollbar->detachAdjustment();
     118
     119        ScrollbarGtk* vScrollbar = reinterpret_cast<ScrollbarGtk*>(verticalScrollbar());
     120        if (vScrollbar)
     121            vScrollbar->detachAdjustment();
    99122    }
    100123
  • trunk/WebCore/platform/gtk/ScrollbarGtk.cpp

    r52421 r54620  
    11/*
    22 *  Copyright (C) 2007, 2009 Holger Hans Peter Freyther zecke@selfish.org
     3 *  Copyright (C) 2010 Gustavo Noronha Silva <gns@gnome.org>
    34 *
    45 *  This library is free software; you can redistribute it and/or
     
    8889ScrollbarGtk::~ScrollbarGtk()
    8990{
     91    if (m_adjustment)
     92        detachAdjustment();
     93}
     94
     95void ScrollbarGtk::attachAdjustment(GtkAdjustment* adjustment)
     96{
     97    if (platformWidget())
     98        return;
     99
     100    if (m_adjustment)
     101        detachAdjustment();
     102
     103    m_adjustment = adjustment;
     104
     105    g_object_ref(m_adjustment);
     106    g_signal_connect(m_adjustment, "value-changed", G_CALLBACK(ScrollbarGtk::gtkValueChanged), this);
     107
     108    updateThumbProportion();
     109    updateThumbPosition();
     110}
     111
     112void ScrollbarGtk::detachAdjustment()
     113{
     114    if (!m_adjustment)
     115        return;
     116
    90117    g_signal_handlers_disconnect_by_func(G_OBJECT(m_adjustment), (gpointer)ScrollbarGtk::gtkValueChanged, this);
    91118
     
    99126    gtk_adjustment_value_changed(m_adjustment);
    100127    g_object_unref(m_adjustment);
     128    m_adjustment = 0;
    101129}
    102130
  • trunk/WebCore/platform/gtk/ScrollbarGtk.h

    r44177 r54620  
    6060    virtual void updateThumbPosition();
    6161    virtual void updateThumbProportion();
    62    
     62
     63    void detachAdjustment();
     64    void attachAdjustment(GtkAdjustment*);
    6365private:
    6466    static void gtkValueChanged(GtkAdjustment*, ScrollbarGtk*);
  • trunk/WebKit/gtk/ChangeLog

    r54561 r54620  
     12010-02-09  Gustavo Noronha Silva  <gns@gnome.org>
     2
     3        Reviewed by Xan Lopez.
     4
     5        [GTK] Hits assertion on history back, with page cache enabled, in specific conditions
     6        https://bugs.webkit.org/show_bug.cgi?id=34773
     7
     8        Make sure cached frames have their scrollbars disconnected from
     9        the WebView's adjustments.
     10
     11        * WebCoreSupport/FrameLoaderClientGtk.cpp:
     12        (WebKit::FrameLoaderClient::savePlatformDataToCachedFrame):
     13
    1142010-02-09  Gustavo Noronha Silva  <gustavo.noronha@collabora.co.uk>
    215
  • trunk/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp

    r54559 r54620  
    44 *  Copyright (C) 2007 Christian Dywan <christian@twotoasts.de>
    55 *  Copyright (C) 2008, 2009 Collabora Ltd.  All rights reserved.
    6  *  Copyright (C) 2009 Gustavo Noronha Silva <gns@gnome.org>
     6 *  Copyright (C) 2009, 2010 Gustavo Noronha Silva <gns@gnome.org>
    77 *  Copyright (C) Research In Motion Limited 2009. All rights reserved.
    88 *
     
    647647void FrameLoaderClient::detachedFromParent2()
    648648{
    649     FrameView *view = core(m_frame)->view();
    650     if (view)
    651         view->setGtkAdjustments(0, 0);
     649    notImplemented();
    652650}
    653651
     
    11171115}
    11181116
    1119 void FrameLoaderClient::savePlatformDataToCachedFrame(CachedFrame*)
    1120 {
    1121 }
    1122 
    1123 static void postCommitFrameViewSetup(WebKitWebFrame *frame, FrameView *view)
     1117void FrameLoaderClient::savePlatformDataToCachedFrame(CachedFrame* cachedFrame)
     1118{
     1119    // We need to do this here in order to disconnect the scrollbars
     1120    // that are being used by the frame that is being cached from the
     1121    // adjustments, otherwise they will react to changes in the
     1122    // adjustments, and bad things will happen.
     1123    if (cachedFrame->view())
     1124        cachedFrame->view()->setGtkAdjustments(0, 0);
     1125}
     1126
     1127static void postCommitFrameViewSetup(WebKitWebFrame *frame, FrameView *view, bool resetValues)
    11241128{
    11251129    WebKitWebView* containingWindow = getViewFromFrame(frame);
    11261130    WebKitWebViewPrivate* priv = WEBKIT_WEB_VIEW_GET_PRIVATE(containingWindow);
    1127     view->setGtkAdjustments(priv->horizontalAdjustment, priv->verticalAdjustment);
     1131    view->setGtkAdjustments(priv->horizontalAdjustment, priv->verticalAdjustment, resetValues);
    11281132
    11291133    if (priv->currentMenu) {
     
    11441148        return;
    11451149
    1146     postCommitFrameViewSetup(m_frame, cachedFrame->view());
     1150    postCommitFrameViewSetup(m_frame, cachedFrame->view(), false);
    11471151}
    11481152
     
    11631167        return;
    11641168
    1165     postCommitFrameViewSetup(m_frame, frame->view());
    1166 }
    1167 
    1168 }
     1169    postCommitFrameViewSetup(m_frame, frame->view(), true);
     1170}
     1171
     1172}
  • trunk/WebKit/gtk/tests/testwebview.c

    r54561 r54620  
    202202        g_main_loop_run(loop);
    203203
     204    /* Make sure GTK+ has time to process the changes in size, for the adjusments */
     205    while (gtk_events_pending())
     206        gtk_main_iteration();
     207
    204208    /* Make sure upper and lower bounds have been restored correctly */
    205209    g_assert_cmpfloat(lower, ==, gtk_adjustment_get_lower(adjustment));
    206210    g_assert_cmpfloat(upper, ==, gtk_adjustment_get_upper(adjustment));
    207211
    208     /* Scrollbar should return to the same position it was in.
    209      * FIXME: with page cache enabled, the result is different, we
    210      * need to figure out why!
    211      */
    212     if (!with_page_cache)
    213         g_assert_cmpfloat(gtk_adjustment_get_value(adjustment), ==, 100.0);
    214     else
    215         g_assert_cmpfloat(gtk_adjustment_get_value(adjustment), ==, 20.0);
     212    g_assert_cmpfloat(gtk_adjustment_get_value(adjustment), ==, 100.0);
    216213
    217214    g_free(effective_uri);
Note: See TracChangeset for help on using the changeset viewer.