- Revision
- 201171
- Author
- [email protected]
- Date
- 2016-05-19 10:58:16 -0700 (Thu, 19 May 2016)
Log Message
Web Inspector: timelines should not update via requestAnimationFrame unless Web Inspector is visible
https://bugs.webkit.org/show_bug.cgi?id=157897
<rdar://problem/26330802>
Reviewed by Timothy Hatcher.
Source/WebInspectorUI:
The timelines overview tries to animate using requestAnimationFrame, even if the
inspector frontend is not really visible. When it does this, requestAnimationFrame
simply stalls out until the inspector becomes visible. If a recording is started
while the inspector is not visible, then when it is shown again, the timeline will
start to animate from 0s instead of the current time. This happens because the
requestAnimationFrame was requested when the current time actually was 0, and it
finally executes some time later, when the current time is no longer accurate.
Since the timelines animate by calculating time elapsed since the previous frame
rather than using event timestamps, there is no way for the timelines to skip forward
in their animations in scenarios where the current time becomes arbitrarily skewed.
To fix this, consider the visibility state of the frontend as reported by the UIProcess.
Fire a global notification when visibility state changes, and start or stop updating
the current time as the frontend becomes visible or not shown.
This does not affect most other uses of requestAnimationFrame, which are used as
timers to call updateLayout at an appropriate time. The timelines case is different
because the current time is fixed prior to requesting an animation frame, and
later animation frames are only triggered by earlier requests, so there's nothing to
coalesce.
* UserInterface/Base/Main.js:
(WebInspector.loaded): Initialize WebInspector.visible.
* UserInterface/Base/Object.js: Add new event.
* UserInterface/Protocol/InspectorFrontendAPI.js:
(InspectorFrontendAPI.setIsVisible): Added.
* UserInterface/Test/Test.js:
(WebInspector.updateVisibilityState): Add a stub.
* UserInterface/Views/TimelineRecordingContentView.js:
(WebInspector.TimelineRecordingContentView):
(WebInspector.TimelineRecordingContentView.prototype._inspectorVisibilityStateChanged):
If visibility state changes while capturing, then start or stop updating the
current time as appropriate. Otherwise, refresh the timelines with updated
times so that they know about the recording's updated start/current/end time.
(WebInspector.TimelineRecordingContentView.prototype._startUpdatingCurrentTime):
Bail out if the Web Inspector frontend is not visible to the user and won't be
able to service requestAnimationFrames immediately.
Source/WebKit2:
The UIProcess needs to notify the Inspector frontend when it is truly visible.
The frontend can't use document.visibilityState because it doesn't seem to work
if the inspector frontend's WKWebView is created but not attached to a window.
* UIProcess/WebInspectorProxy.cpp:
(WebKit::WebInspectorProxy::open):
(WebKit::WebInspectorProxy::didClose):
Send visibility updates to the inspector process when the inspector becomes
"visible" or "not visible". It becomes visible if it is attached to the
inspected page's window, or gets its own native window.
* WebProcess/WebPage/WebInspectorUI.cpp:
(WebKit::WebInspectorUI::frontendLoaded):
(WebKit::WebInspectorUI::setDockingUnavailable):
(WebKit::WebInspectorUI::setIsVisible):
Call InspectorFrontendAPI.updateVisibilityState to let the frontend know.
* WebProcess/WebPage/WebInspectorUI.h:
* WebProcess/WebPage/WebInspectorUI.messages.in:
Add new message.
Modified Paths
Diff
Modified: trunk/Source/WebInspectorUI/ChangeLog (201170 => 201171)
--- trunk/Source/WebInspectorUI/ChangeLog 2016-05-19 17:27:11 UTC (rev 201170)
+++ trunk/Source/WebInspectorUI/ChangeLog 2016-05-19 17:58:16 UTC (rev 201171)
@@ -1,3 +1,54 @@
+2016-05-19 Brian Burg <[email protected]>
+
+ Web Inspector: timelines should not update via requestAnimationFrame unless Web Inspector is visible
+ https://bugs.webkit.org/show_bug.cgi?id=157897
+ <rdar://problem/26330802>
+
+ Reviewed by Timothy Hatcher.
+
+ The timelines overview tries to animate using requestAnimationFrame, even if the
+ inspector frontend is not really visible. When it does this, requestAnimationFrame
+ simply stalls out until the inspector becomes visible. If a recording is started
+ while the inspector is not visible, then when it is shown again, the timeline will
+ start to animate from 0s instead of the current time. This happens because the
+ requestAnimationFrame was requested when the current time actually was 0, and it
+ finally executes some time later, when the current time is no longer accurate.
+ Since the timelines animate by calculating time elapsed since the previous frame
+ rather than using event timestamps, there is no way for the timelines to skip forward
+ in their animations in scenarios where the current time becomes arbitrarily skewed.
+
+ To fix this, consider the visibility state of the frontend as reported by the UIProcess.
+ Fire a global notification when visibility state changes, and start or stop updating
+ the current time as the frontend becomes visible or not shown.
+
+ This does not affect most other uses of requestAnimationFrame, which are used as
+ timers to call updateLayout at an appropriate time. The timelines case is different
+ because the current time is fixed prior to requesting an animation frame, and
+ later animation frames are only triggered by earlier requests, so there's nothing to
+ coalesce.
+
+ * UserInterface/Base/Main.js:
+ (WebInspector.loaded): Initialize WebInspector.visible.
+
+ * UserInterface/Base/Object.js: Add new event.
+
+ * UserInterface/Protocol/InspectorFrontendAPI.js:
+ (InspectorFrontendAPI.setIsVisible): Added.
+
+ * UserInterface/Test/Test.js:
+ (WebInspector.updateVisibilityState): Add a stub.
+
+ * UserInterface/Views/TimelineRecordingContentView.js:
+ (WebInspector.TimelineRecordingContentView):
+ (WebInspector.TimelineRecordingContentView.prototype._inspectorVisibilityStateChanged):
+ If visibility state changes while capturing, then start or stop updating the
+ current time as appropriate. Otherwise, refresh the timelines with updated
+ times so that they know about the recording's updated start/current/end time.
+
+ (WebInspector.TimelineRecordingContentView.prototype._startUpdatingCurrentTime):
+ Bail out if the Web Inspector frontend is not visible to the user and won't be
+ able to service requestAnimationFrames immediately.
+
2016-05-18 Timothy Hatcher <[email protected]>
Web Inspector: Classes toggle wraps in some localizations
Modified: trunk/Source/WebInspectorUI/UserInterface/Base/Main.js (201170 => 201171)
--- trunk/Source/WebInspectorUI/UserInterface/Base/Main.js 2016-05-19 17:27:11 UTC (rev 201170)
+++ trunk/Source/WebInspectorUI/UserInterface/Base/Main.js 2016-05-19 17:58:16 UTC (rev 201171)
@@ -176,6 +176,8 @@
y: 0
};
+ this.visible = false;
+
this._windowKeydownListeners = [];
};
@@ -711,6 +713,12 @@
this._updateDockNavigationItems();
};
+WebInspector.updateVisibilityState = function(visible)
+{
+ this.visible = visible;
+ this.notifications.dispatchEventToListeners(WebInspector.Notification.VisibilityStateDidChange);
+}
+
WebInspector.handlePossibleLinkClick = function(event, frame, alwaysOpenExternally)
{
var anchorElement = event.target.enclosingNodeOrSelfWithNodeName("a");
Modified: trunk/Source/WebInspectorUI/UserInterface/Base/Object.js (201170 => 201171)
--- trunk/Source/WebInspectorUI/UserInterface/Base/Object.js 2016-05-19 17:27:11 UTC (rev 201170)
+++ trunk/Source/WebInspectorUI/UserInterface/Base/Object.js 2016-05-19 17:58:16 UTC (rev 201171)
@@ -216,4 +216,5 @@
ExtraDomainsActivated: "extra-domains-activated",
TabTypesChanged: "tab-types-changed",
DebugUIEnabledDidChange: "debug-ui-enabled-did-change",
+ VisibilityStateDidChange: "visibility-state-did-change",
};
Modified: trunk/Source/WebInspectorUI/UserInterface/Protocol/InspectorFrontendAPI.js (201170 => 201171)
--- trunk/Source/WebInspectorUI/UserInterface/Protocol/InspectorFrontendAPI.js 2016-05-19 17:27:11 UTC (rev 201170)
+++ trunk/Source/WebInspectorUI/UserInterface/Protocol/InspectorFrontendAPI.js 2016-05-19 17:58:16 UTC (rev 201171)
@@ -68,6 +68,11 @@
WebInspector.updateDockedState(side);
},
+ setIsVisible: function(visible)
+ {
+ WebInspector.updateVisibilityState(visible);
+ },
+
showConsole: function()
{
WebInspector.showConsoleTab();
Modified: trunk/Source/WebInspectorUI/UserInterface/Test/Test.js (201170 => 201171)
--- trunk/Source/WebInspectorUI/UserInterface/Test/Test.js 2016-05-19 17:27:11 UTC (rev 201170)
+++ trunk/Source/WebInspectorUI/UserInterface/Test/Test.js 2016-05-19 17:58:16 UTC (rev 201171)
@@ -95,6 +95,7 @@
// Add stubs that are called by the frontend API.
WebInspector.updateDockedState = () => {};
WebInspector.updateDockingAvailability = () => {};
+WebInspector.updateVisibilityState = () => {};
window.InspectorTest = new FrontendTestHarness();
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/TimelineRecordingContentView.js (201170 => 201171)
--- trunk/Source/WebInspectorUI/UserInterface/Views/TimelineRecordingContentView.js 2016-05-19 17:27:11 UTC (rev 201170)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/TimelineRecordingContentView.js 2016-05-19 17:58:16 UTC (rev 201171)
@@ -91,6 +91,8 @@
WebInspector.TimelineView.addEventListener(WebInspector.TimelineView.Event.RecordWasFiltered, this._recordWasFiltered, this);
+ WebInspector.notifications.addEventListener(WebInspector.Notification.VisibilityStateDidChange, this._inspectorVisibilityStateChanged, this);
+
for (let instrument of this._recording.instruments)
this._instrumentAdded(instrument);
@@ -341,6 +343,33 @@
this.dispatchEventToListeners(WebInspector.ContentView.Event.SupplementalRepresentedObjectsDidChange);
}
+ _inspectorVisibilityStateChanged()
+ {
+ if (WebInspector.timelineManager.activeRecording !== this._recording)
+ return;
+
+ // Stop updating since the results won't be rendered anyway.
+ if (!WebInspector.visible && this._updating) {
+ this._stopUpdatingCurrentTime();
+ return;
+ }
+
+ // Nothing else to do if the current time was not being updated.
+ if (!WebInspector.visible)
+ return;
+
+ let {startTime, endTime} = this.representedObject;
+ if (!WebInspector.timelineManager.isCapturing()) {
+ // Force the overview to render data from the entire recording.
+ // This is necessary if the recording was started when the inspector was not
+ // visible because the views were never updated with currentTime/endTime.
+ this._updateTimes(startTime, endTime, endTime);
+ return;
+ }
+
+ this._startUpdatingCurrentTime(endTime);
+ }
+
_update(timestamp)
{
if (this._waitingToResetCurrentTime) {
@@ -401,9 +430,13 @@
if (this._updating)
return;
- if (typeof startTime === "number")
+ // Don't update the current time if the Inspector is not visible, as the requestAnimationFrames won't work.
+ if (!WebInspector.visible)
+ return;
+
+ if (typeof startTime === "number" && !isNaN(this._currentTime))
this._currentTime = startTime;
- else if (!isNaN(this._currentTime)) {
+ else {
// This happens when you stop and later restart recording.
// COMPATIBILITY (iOS 9): Timeline.recordingStarted events did not include a timestamp.
// We likely need to jump into the future to a better current time which we can
Modified: trunk/Source/WebKit2/ChangeLog (201170 => 201171)
--- trunk/Source/WebKit2/ChangeLog 2016-05-19 17:27:11 UTC (rev 201170)
+++ trunk/Source/WebKit2/ChangeLog 2016-05-19 17:58:16 UTC (rev 201171)
@@ -1,5 +1,34 @@
2016-05-19 Brian Burg <[email protected]>
+ Web Inspector: timelines should not update via requestAnimationFrame unless Web Inspector is visible
+ https://bugs.webkit.org/show_bug.cgi?id=157897
+ <rdar://problem/26330802>
+
+ Reviewed by Timothy Hatcher.
+
+ The UIProcess needs to notify the Inspector frontend when it is truly visible.
+ The frontend can't use document.visibilityState because it doesn't seem to work
+ if the inspector frontend's WKWebView is created but not attached to a window.
+
+ * UIProcess/WebInspectorProxy.cpp:
+ (WebKit::WebInspectorProxy::open):
+ (WebKit::WebInspectorProxy::didClose):
+ Send visibility updates to the inspector process when the inspector becomes
+ "visible" or "not visible". It becomes visible if it is attached to the
+ inspected page's window, or gets its own native window.
+
+ * WebProcess/WebPage/WebInspectorUI.cpp:
+ (WebKit::WebInspectorUI::frontendLoaded):
+ (WebKit::WebInspectorUI::setDockingUnavailable):
+ (WebKit::WebInspectorUI::setIsVisible):
+ Call InspectorFrontendAPI.updateVisibilityState to let the frontend know.
+
+ * WebProcess/WebPage/WebInspectorUI.h:
+ * WebProcess/WebPage/WebInspectorUI.messages.in:
+ Add new message.
+
+2016-05-19 Brian Burg <[email protected]>
+
Web Inspector: use a consistent prefix for injected scripts
https://bugs.webkit.org/show_bug.cgi?id=157715
<rdar://problem/26287188>
Modified: trunk/Source/WebKit2/UIProcess/WebInspectorProxy.cpp (201170 => 201171)
--- trunk/Source/WebKit2/UIProcess/WebInspectorProxy.cpp 2016-05-19 17:27:11 UTC (rev 201170)
+++ trunk/Source/WebKit2/UIProcess/WebInspectorProxy.cpp 2016-05-19 17:58:16 UTC (rev 201171)
@@ -581,6 +581,7 @@
return;
m_isVisible = true;
+ m_inspectorPage->process().send(Messages::WebInspectorUI::SetIsVisible(m_isVisible), m_inspectorPage->pageID());
platformOpen();
}
@@ -590,13 +591,14 @@
if (!m_inspectorPage)
return;
- m_inspectorPage->process().removeMessageReceiver(Messages::WebInspectorProxy::messageReceiverName(), m_inspectedPage->pageID());
-
m_isVisible = false;
m_isProfilingPage = false;
m_showMessageSent = false;
m_ignoreFirstBringToFront = false;
+ m_inspectorPage->process().send(Messages::WebInspectorUI::SetIsVisible(m_isVisible), m_inspectorPage->pageID());
+ m_inspectorPage->process().removeMessageReceiver(Messages::WebInspectorProxy::messageReceiverName(), m_inspectedPage->pageID());
+
if (m_isAttached)
platformDetach();
Modified: trunk/Source/WebKit2/WebProcess/WebPage/WebInspectorUI.cpp (201170 => 201171)
--- trunk/Source/WebKit2/WebProcess/WebPage/WebInspectorUI.cpp 2016-05-19 17:27:11 UTC (rev 201170)
+++ trunk/Source/WebKit2/WebProcess/WebPage/WebInspectorUI.cpp 2016-05-19 17:58:16 UTC (rev 201171)
@@ -95,6 +95,7 @@
// cleared due to a reload, the dock state won't be resent from UIProcess.
setDockingUnavailable(m_dockingUnavailable);
setDockSide(m_dockSide);
+ setIsVisible(m_isVisible);
WebProcess::singleton().parentProcessConnection()->send(Messages::WebInspectorProxy::FrontendLoaded(), m_inspectedPageIdentifier);
@@ -178,10 +179,18 @@
void WebInspectorUI::setDockingUnavailable(bool unavailable)
{
+ m_dockingUnavailable = unavailable;
+
m_frontendAPIDispatcher.dispatchCommand(ASCIILiteral("setDockingUnavailable"), unavailable);
- m_dockingUnavailable = unavailable;
}
+void WebInspectorUI::setIsVisible(bool visible)
+{
+ m_isVisible = visible;
+
+ m_frontendAPIDispatcher.dispatchCommand(ASCIILiteral("setIsVisible"), visible);
+}
+
void WebInspectorUI::changeAttachedWindowHeight(unsigned height)
{
WebProcess::singleton().parentProcessConnection()->send(Messages::WebInspectorProxy::SetAttachedWindowHeight(height), m_inspectedPageIdentifier);
Modified: trunk/Source/WebKit2/WebProcess/WebPage/WebInspectorUI.h (201170 => 201171)
--- trunk/Source/WebKit2/WebProcess/WebPage/WebInspectorUI.h 2016-05-19 17:27:11 UTC (rev 201170)
+++ trunk/Source/WebKit2/WebProcess/WebPage/WebInspectorUI.h 2016-05-19 17:58:16 UTC (rev 201171)
@@ -74,6 +74,8 @@
void setDockSide(DockSide);
void setDockingUnavailable(bool);
+ void setIsVisible(bool);
+
void didSave(const String& url);
void didAppend(const String& url);
@@ -126,6 +128,7 @@
uint64_t m_inspectedPageIdentifier { 0 };
bool m_underTest { false };
bool m_dockingUnavailable { false };
+ bool m_isVisible { false };
DockSide m_dockSide { DockSide::Undocked };
unsigned m_inspectionLevel { 1 };
Modified: trunk/Source/WebKit2/WebProcess/WebPage/WebInspectorUI.messages.in (201170 => 201171)
--- trunk/Source/WebKit2/WebProcess/WebPage/WebInspectorUI.messages.in 2016-05-19 17:27:11 UTC (rev 201170)
+++ trunk/Source/WebKit2/WebProcess/WebPage/WebInspectorUI.messages.in 2016-05-19 17:58:16 UTC (rev 201171)
@@ -27,6 +27,7 @@
AttachedRight()
Detached()
SetDockingUnavailable(bool unavailable)
+ SetIsVisible(bool visible)
ShowConsole()
ShowResources()
