- Revision
- 138278
- Author
- simon.fra...@apple.com
- Date
- 2012-12-20 11:30:59 -0800 (Thu, 20 Dec 2012)
Log Message
Lots of sticky tests failing in WK2
https://bugs.webkit.org/show_bug.cgi?id=105464
Source/WebCore:
Reviewed by Brady Eidson.
Fixing this bug altered the timing of InternalSettings::Backup()
and restoreTo(), such that 'canStartMedia' on Page would get saved as false,
and then restored as false, which breaks all media tests.
'canStartMedia' is set to false initially by the WebPage constructor, and is also
set to false when the WKView leaves the window, so treating it like other Settings
that can be saved and restored is unreliable, as seen here. It's easier and simpler
to just always reset it to 'true' between tests.
* testing/InternalSettings.cpp:
(WebCore::InternalSettings::Backup::Backup):
(WebCore::InternalSettings::Backup::restoreTo):
(WebCore::InternalSettings::InternalSettings):
(WebCore::InternalSettings::reset):
* testing/InternalSettings.h:
(Backup):
Tools:
Reviewed by Beth Dakin.
WebKitTestRunner had a race between snapshotting in the UI process,
and resettting after the test in the web process. InjectedBundle::done()
was a bad place to call page()->resetAfterTest(), because of this race;
it could reset the scroll position before the UI snapshot had been obtained.
Fix by moving the call to page()->resetAfterTest() into didReceiveMessage(),
for the "Reset" message which will come in before the next test.
* WebKitTestRunner/InjectedBundle/InjectedBundle.cpp:
(WTR::InjectedBundle::didReceiveMessage):
(WTR::InjectedBundle::done):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (138277 => 138278)
--- trunk/Source/WebCore/ChangeLog 2012-12-20 19:16:00 UTC (rev 138277)
+++ trunk/Source/WebCore/ChangeLog 2012-12-20 19:30:59 UTC (rev 138278)
@@ -1,3 +1,27 @@
+2012-12-19 Simon Fraser <simon.fra...@apple.com>
+
+ Lots of sticky tests failing in WK2
+ https://bugs.webkit.org/show_bug.cgi?id=105464
+
+ Reviewed by Brady Eidson.
+
+ Fixing this bug altered the timing of InternalSettings::Backup()
+ and restoreTo(), such that 'canStartMedia' on Page would get saved as false,
+ and then restored as false, which breaks all media tests.
+
+ 'canStartMedia' is set to false initially by the WebPage constructor, and is also
+ set to false when the WKView leaves the window, so treating it like other Settings
+ that can be saved and restored is unreliable, as seen here. It's easier and simpler
+ to just always reset it to 'true' between tests.
+
+ * testing/InternalSettings.cpp:
+ (WebCore::InternalSettings::Backup::Backup):
+ (WebCore::InternalSettings::Backup::restoreTo):
+ (WebCore::InternalSettings::InternalSettings):
+ (WebCore::InternalSettings::reset):
+ * testing/InternalSettings.h:
+ (Backup):
+
2012-12-20 Erik Arvidsson <a...@chromium.org>
REGRESSION(r138263): Don't use fastGetAttribute for HTMLNames::classAttr because it breaks on SVGElement
Modified: trunk/Source/WebCore/testing/InternalSettings.cpp (138277 => 138278)
--- trunk/Source/WebCore/testing/InternalSettings.cpp 2012-12-20 19:16:00 UTC (rev 138277)
+++ trunk/Source/WebCore/testing/InternalSettings.cpp 2012-12-20 19:30:59 UTC (rev 138278)
@@ -61,7 +61,7 @@
namespace WebCore {
-InternalSettings::Backup::Backup(Page* page, Settings* settings)
+InternalSettings::Backup::Backup(Settings* settings)
: m_originalPasswordEchoDurationInSeconds(settings->passwordEchoDurationInSeconds())
, m_originalPasswordEchoEnabled(settings->passwordEchoEnabled())
, m_originalFixedElementsLayoutRelativeToFrame(settings->fixedElementsLayoutRelativeToFrame())
@@ -91,7 +91,6 @@
#if ENABLE(DIALOG_ELEMENT)
, m_originalDialogElementEnabled(RuntimeEnabledFeatures::dialogElementEnabled())
#endif
- , m_canStartMedia(page->canStartMedia())
, m_originalForceCompositingMode(settings->forceCompositingMode())
, m_originalCompositingForFixedPositionEnabled(settings->acceleratedCompositingForFixedPositionEnabled())
, m_originalCompositingForScrollableFramesEnabled(settings->acceleratedCompositingForScrollableFramesEnabled())
@@ -108,7 +107,7 @@
}
-void InternalSettings::Backup::restoreTo(Page* page, Settings* settings)
+void InternalSettings::Backup::restoreTo(Settings* settings)
{
settings->setPasswordEchoDurationInSeconds(m_originalPasswordEchoDurationInSeconds);
settings->setPasswordEchoEnabled(m_originalPasswordEchoEnabled);
@@ -139,7 +138,6 @@
#if ENABLE(DIALOG_ELEMENT)
RuntimeEnabledFeatures::setDialogElementEnabled(m_originalDialogElementEnabled);
#endif
- page->setCanStartMedia(m_canStartMedia);
settings->setForceCompositingMode(m_originalForceCompositingMode);
settings->setAcceleratedCompositingForFixedPositionEnabled(m_originalCompositingForFixedPositionEnabled);
settings->setAcceleratedCompositingForScrollableFramesEnabled(m_originalCompositingForScrollableFramesEnabled);
@@ -168,16 +166,17 @@
InternalSettings::InternalSettings(Page* page)
: m_page(page)
- , m_backup(page, page->settings())
+ , m_backup(page->settings())
{
}
void InternalSettings::reset()
{
page()->setPageScaleFactor(1, IntPoint(0, 0));
+ page()->setCanStartMedia(true);
- m_backup.restoreTo(page(), settings());
- m_backup = Backup(page(), settings());
+ m_backup.restoreTo(settings());
+ m_backup = Backup(settings());
}
Settings* InternalSettings::settings() const
Modified: trunk/Source/WebCore/testing/InternalSettings.h (138277 => 138278)
--- trunk/Source/WebCore/testing/InternalSettings.h 2012-12-20 19:16:00 UTC (rev 138277)
+++ trunk/Source/WebCore/testing/InternalSettings.h 2012-12-20 19:30:59 UTC (rev 138278)
@@ -46,8 +46,8 @@
public:
class Backup {
public:
- Backup(Page*, Settings*);
- void restoreTo(Page*, Settings*);
+ Backup(Settings*);
+ void restoreTo(Settings*);
double m_originalPasswordEchoDurationInSeconds;
bool m_originalPasswordEchoEnabled;
@@ -78,7 +78,6 @@
#if ENABLE(DIALOG_ELEMENT)
bool m_originalDialogElementEnabled;
#endif
- bool m_canStartMedia;
bool m_originalForceCompositingMode;
bool m_originalCompositingForFixedPositionEnabled;
bool m_originalCompositingForScrollableFramesEnabled;
Modified: trunk/Tools/ChangeLog (138277 => 138278)
--- trunk/Tools/ChangeLog 2012-12-20 19:16:00 UTC (rev 138277)
+++ trunk/Tools/ChangeLog 2012-12-20 19:30:59 UTC (rev 138278)
@@ -1,3 +1,22 @@
+2012-12-19 Simon Fraser <simon.fra...@apple.com>
+
+ Lots of sticky tests failing in WK2
+ https://bugs.webkit.org/show_bug.cgi?id=105464
+
+ Reviewed by Beth Dakin.
+
+ WebKitTestRunner had a race between snapshotting in the UI process,
+ and resettting after the test in the web process. InjectedBundle::done()
+ was a bad place to call page()->resetAfterTest(), because of this race;
+ it could reset the scroll position before the UI snapshot had been obtained.
+
+ Fix by moving the call to page()->resetAfterTest() into didReceiveMessage(),
+ for the "Reset" message which will come in before the next test.
+
+ * WebKitTestRunner/InjectedBundle/InjectedBundle.cpp:
+ (WTR::InjectedBundle::didReceiveMessage):
+ (WTR::InjectedBundle::done):
+
2012-12-19 Filip Pizlo <fpi...@apple.com>
DFG speculation checks that take JumpList should consolidate OSRExits
Modified: trunk/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp (138277 => 138278)
--- trunk/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp 2012-12-20 19:16:00 UTC (rev 138277)
+++ trunk/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp 2012-12-20 19:30:59 UTC (rev 138278)
@@ -176,6 +176,8 @@
resetLocalSettings();
m_testRunner->removeAllWebNotificationPermissions();
+ page()->resetAfterTest();
+
return;
}
if (WKStringIsEqualToUTF8CString(messageName, "CallAddChromeInputFieldCallback")) {
@@ -301,8 +303,6 @@
closeOtherPages();
- page()->resetAfterTest();
-
m_state = Idle;
}
