WebPage should take UserActivity directly for user input
authorbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Oct 2016 23:20:11 +0000 (23:20 +0000)
committerbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Oct 2016 23:20:11 +0000 (23:20 +0000)
https://bugs.webkit.org/show_bug.cgi?id=163813

Reviewed by Anders Carlsson.

When we receive mouse/keyboard events in a page, we want to prevent AppNap. We currently do so
via the PageThrottler. This patch is to just make the WebPage drive the UserActivity directly.

Two reasons to do so: (1) to cleanup & simplify for further refactoring. (2) The current code
isn't really achieving the desired effect. The page setting the flag in the throttler to get
the activity to be set is now a less effective way of achieving this goal, since the
PageActivityState bounces back across to the UI process & then messages back to the WebContent
process to take the UserActivity. These extra hops defeat the purpose of making sure the boost
from the initial message isn't lost.

Source/WebCore:

* page/PageThrottler.cpp:
(WebCore::PageThrottler::PageThrottler):
(WebCore::m_userInputHysteresis): Deleted.
* page/PageThrottler.h:
(WebCore::PageThrottler::didReceiveUserInput): Deleted.
    - removed PageActivityState::UserInputActivity, didReceiveUserInput, m_userInputHysteresis.

Source/WebKit2:

* WebProcess/WebPage/WebPage.cpp:
(WebKit::m_userActivityHysteresis):
    - m_userActivityHysteresis triggers updateUserActivity.
(WebKit::WebPage::setPageSuppressed):
    - setPageSuppressed starts/stops m_userActivityHysteresis.
(WebKit::WebPage::updateUserActivity):
    - update UserActivity based on state of m_userActivityHysteresis.
(WebKit::WebPage::mouseEvent):
(WebKit::WebPage::wheelEvent):
(WebKit::WebPage::keyEvent):
    - input events impulse m_userActivityHysteresis.
* WebProcess/WebPage/WebPage.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@207699 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/page/PageThrottler.cpp
Source/WebCore/page/PageThrottler.h
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/WebPage/WebPage.cpp
Source/WebKit2/WebProcess/WebPage/WebPage.h

index d4ec34c..1473e38 100644 (file)
@@ -1,3 +1,27 @@
+2016-10-21  Gavin Barraclough  <barraclough@apple.com>
+
+        WebPage should take UserActivity directly for user input
+        https://bugs.webkit.org/show_bug.cgi?id=163813
+
+        Reviewed by Anders Carlsson.
+
+        When we receive mouse/keyboard events in a page, we want to prevent AppNap. We currently do so
+        via the PageThrottler. This patch is to just make the WebPage drive the UserActivity directly.
+
+        Two reasons to do so: (1) to cleanup & simplify for further refactoring. (2) The current code
+        isn't really achieving the desired effect. The page setting the flag in the throttler to get
+        the activity to be set is now a less effective way of achieving this goal, since the
+        PageActivityState bounces back across to the UI process & then messages back to the WebContent
+        process to take the UserActivity. These extra hops defeat the purpose of making sure the boost
+        from the initial message isn't lost.
+
+        * page/PageThrottler.cpp:
+        (WebCore::PageThrottler::PageThrottler):
+        (WebCore::m_userInputHysteresis): Deleted.
+        * page/PageThrottler.h:
+        (WebCore::PageThrottler::didReceiveUserInput): Deleted.
+            - removed PageActivityState::UserInputActivity, didReceiveUserInput, m_userInputHysteresis.
+
 2016-10-21  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         Support (insertFrom|deleteBy)Composition and (insert|delete)CompositionText inputTypes for InputEvents
index 16d0395..a150d31 100644 (file)
@@ -34,7 +34,6 @@ static const double PageLoadHysteresisSeconds = 10;
 
 PageThrottler::PageThrottler(Page& page)
     : m_page(page)
-    , m_userInputHysteresis([this](HysteresisState state) { setActivityFlag(PageActivityState::UserInputActivity, state == HysteresisState::Started); })
     , m_mediaActivityHysteresis([this](HysteresisState state) { setActivityFlag(PageActivityState::MediaActivity, state == HysteresisState::Started); })
     , m_pageLoadActivityHysteresis([this](HysteresisState state) { setActivityFlag(PageActivityState::PageLoadActivity, state == HysteresisState::Started); }, PageLoadHysteresisSeconds)
     , m_mediaActivityCounter([this](RefCounterEvent) { mediaActivityCounterChanged(); })
index d67fc01..903adcb 100644 (file)
@@ -42,15 +42,14 @@ typedef PageActivityCounter::Token PageActivityAssertionToken;
 
 struct PageActivityState {
     enum {
-        UserInputActivity = 1 << 0,
-        MediaActivity = 1 << 1,
-        PageLoadActivity = 1 << 2,
+        MediaActivity = 1 << 0,
+        PageLoadActivity = 1 << 1,
     };
 
     typedef unsigned Flags;
 
     static const Flags NoFlags = 0;
-    static const Flags AllFlags = UserInputActivity | MediaActivity | PageLoadActivity;
+    static const Flags AllFlags = MediaActivity | PageLoadActivity;
 };
 
 class PageThrottler {
@@ -58,7 +57,6 @@ class PageThrottler {
 public:
     PageThrottler(Page&);
 
-    void didReceiveUserInput() { m_userInputHysteresis.impulse(); }
     PageActivityState::Flags activityState() { return m_activityState; }
     void pluginDidEvaluateWhileAudioIsPlaying() { m_mediaActivityHysteresis.impulse(); }
     PageActivityAssertionToken mediaActivityToken();
@@ -71,7 +69,6 @@ private:
 
     Page& m_page;
     PageActivityState::Flags m_activityState { PageActivityState::NoFlags };
-    HysteresisActivity m_userInputHysteresis;
     HysteresisActivity m_mediaActivityHysteresis;
     HysteresisActivity m_pageLoadActivityHysteresis;
     PageActivityCounter m_mediaActivityCounter;
index c884441..558cc1f 100644 (file)
@@ -1,3 +1,33 @@
+2016-10-21  Gavin Barraclough  <barraclough@apple.com>
+
+        WebPage should take UserActivity directly for user input
+        https://bugs.webkit.org/show_bug.cgi?id=163813
+
+        Reviewed by Anders Carlsson.
+
+        When we receive mouse/keyboard events in a page, we want to prevent AppNap. We currently do so
+        via the PageThrottler. This patch is to just make the WebPage drive the UserActivity directly.
+
+        Two reasons to do so: (1) to cleanup & simplify for further refactoring. (2) The current code
+        isn't really achieving the desired effect. The page setting the flag in the throttler to get
+        the activity to be set is now a less effective way of achieving this goal, since the
+        PageActivityState bounces back across to the UI process & then messages back to the WebContent
+        process to take the UserActivity. These extra hops defeat the purpose of making sure the boost
+        from the initial message isn't lost.
+
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::m_userActivityHysteresis):
+            - m_userActivityHysteresis triggers updateUserActivity.
+        (WebKit::WebPage::setPageSuppressed):
+            - setPageSuppressed starts/stops m_userActivityHysteresis.
+        (WebKit::WebPage::updateUserActivity):
+            - update UserActivity based on state of m_userActivityHysteresis.
+        (WebKit::WebPage::mouseEvent):
+        (WebKit::WebPage::wheelEvent):
+        (WebKit::WebPage::keyEvent):
+            - input events impulse m_userActivityHysteresis.
+        * WebProcess/WebPage/WebPage.h:
+
 2016-10-21  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         Support (insertFrom|deleteBy)Composition and (insert|delete)CompositionText inputTypes for InputEvents
index 212f14a..c086c04 100644 (file)
@@ -378,6 +378,7 @@ WebPage::WebPage(uint64_t pageID, const WebPageCreationParameters& parameters)
     , m_useAsyncScrolling(false)
     , m_viewState(parameters.viewState)
     , m_userActivity("Process suppression disabled for page.")
+    , m_userActivityHysteresis([this](HysteresisState) { updateUserActivity(); })
     , m_pendingNavigationID(0)
 #if ENABLE(WEBGL)
     , m_systemWebGLPolicy(WebGLAllowCreation)
@@ -590,9 +591,17 @@ void WebPage::setPageSuppressed(bool pageSuppressed)
     // The UserActivity keeps the processes runnable. So if the page should be suppressed, stop the activity.
     // If the page should not be supressed, start it.
     if (pageSuppressed)
-        m_userActivity.stop();
+        m_userActivityHysteresis.stop();
     else
+        m_userActivityHysteresis.start();
+}
+
+void WebPage::updateUserActivity()
+{
+    if (m_userActivityHysteresis.state() == HysteresisState::Started)
         m_userActivity.start();
+    else
+        m_userActivity.stop();
 }
 
 WebPage::~WebPage()
@@ -2238,7 +2247,7 @@ void WebPage::mouseEvent(const WebMouseEvent& mouseEvent)
 {
     TemporaryChange<bool> userIsInteractingChange { m_userIsInteracting, true };
 
-    m_page->pageThrottler().didReceiveUserInput();
+    m_userActivityHysteresis.impulse();
 
     bool shouldHandleEvent = true;
 
@@ -2293,7 +2302,7 @@ static bool handleWheelEvent(const WebWheelEvent& wheelEvent, Page* page)
 
 void WebPage::wheelEvent(const WebWheelEvent& wheelEvent)
 {
-    m_page->pageThrottler().didReceiveUserInput();
+    m_userActivityHysteresis.impulse();
 
     CurrentEvent currentEvent(wheelEvent);
 
@@ -2316,7 +2325,7 @@ void WebPage::keyEvent(const WebKeyboardEvent& keyboardEvent)
 {
     TemporaryChange<bool> userIsInteractingChange { m_userIsInteracting, true };
 
-    m_page->pageThrottler().didReceiveUserInput();
+    m_userActivityHysteresis.impulse();
 
     CurrentEvent currentEvent(keyboardEvent);
 
index dcd59de..511c4c6 100644 (file)
@@ -971,6 +971,8 @@ public:
 private:
     WebPage(uint64_t pageID, const WebPageCreationParameters&);
 
+    void updateUserActivity();
+
     // IPC::MessageSender
     IPC::Connection* messageSenderConnection() override;
     uint64_t messageSenderDestinationID() override;
@@ -1486,6 +1488,7 @@ private:
     WebCore::ViewState::Flags m_viewState;
 
     UserActivity m_userActivity;
+    WebCore::HysteresisActivity m_userActivityHysteresis;
 
     uint64_t m_pendingNavigationID;