RESOLVED FIXED 146439
Web page doesn't update its loading state when web process becomes suspended if there are pending network requests (XHR).
https://bugs.webkit.org/show_bug.cgi?id=146439
Summary Web page doesn't update its loading state when web process becomes suspended ...
Yongjun Zhang
Reported 2015-06-29 16:26:37 PDT
When a web page starts a XHR request after the page is finished loading, we should take a background activity token and hold it until the request is cancelled or finished. Otherwise the WebContent process could be suspended since its activity token could drop to 0 after this point.
Attachments
Patch. (2.52 KB, patch)
2015-06-29 16:52 PDT, Yongjun Zhang
darin: review+
Patch v2. (6.37 KB, patch)
2015-06-29 20:34 PDT, Yongjun Zhang
mitz: review+
Patch for landing. (6.42 KB, patch)
2015-07-01 15:22 PDT, Yongjun Zhang
no flags
Yongjun Zhang
Comment 1 2015-06-29 16:45:09 PDT
Yongjun Zhang
Comment 2 2015-06-29 16:52:58 PDT
Yongjun Zhang
Comment 3 2015-06-29 18:07:08 PDT
Hmm, this fix could have the side effect of holding the web process too long if there is a long lasting XHR request. I need to work on a better fix.
Yongjun Zhang
Comment 4 2015-06-29 18:50:16 PDT
Update the title to reflect the real problem.
Yongjun Zhang
Comment 5 2015-06-29 20:34:37 PDT
Created attachment 255810 [details] Patch v2.
Simon Fraser (smfr)
Comment 6 2015-06-30 15:37:30 PDT
Comment on attachment 255810 [details] Patch v2. View in context: https://bugs.webkit.org/attachment.cgi?id=255810&action=review > Source/WebKit2/UIProcess/WebPageProxy.h:821 > + void processWillBecomeForground(); "processWillBecomeForground" -> processWillBecomeForeground
mitz
Comment 7 2015-07-01 11:17:25 PDT
Comment on attachment 255810 [details] Patch v2. I don’t know which state transitions are possible. Is it a concern that the outcomes of the following transitions are different? Foreground -> Suspended -> Background Foreground -> Background Is it a concern that this transition will fail to restore the state? Foreground -> Suspended -> Background -> Suspended -> Foreground (because on the second entry into Suspended, m_hasNetworkRequestsOnSuspended will change to false)
Yongjun Zhang
Comment 8 2015-07-01 11:52:20 PDT
(In reply to comment #7) > Comment on attachment 255810 [details] > Patch v2. > > I don’t know which state transitions are possible. Is it a concern that the > outcomes of the following transitions are different? > Foreground -> Suspended -> Background > Foreground -> Background I could be wrong, but based on code inspection and my own test, I didn't find a scenario where the web process could go from Suspended to Background. > > Is it a concern that this transition will fail to restore the state? > Foreground -> Suspended -> Background -> Suspended -> Foreground > (because on the second entry into Suspended, m_hasNetworkRequestsOnSuspended > will change to false) Again, this ties to whether Suspended -> Background transition is possible. Thanks for the review! I will fix the typo on commit.
Yongjun Zhang
Comment 9 2015-07-01 15:22:25 PDT
Created attachment 255956 [details] Patch for landing.
WebKit Commit Bot
Comment 10 2015-07-01 16:18:56 PDT
Comment on attachment 255956 [details] Patch for landing. Clearing flags on attachment: 255956 Committed r186200: <http://trac.webkit.org/changeset/186200>
WebKit Commit Bot
Comment 11 2015-07-01 16:19:03 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.