New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 749789 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 20
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocked on:
issue 800903

Blocking:
issue 725128
issue 730098



Sign in to add a comment

Session restore needs a better metric for signaling when a tab has finished loading

Project Member Reported by chrisha@chromium.org, Jul 27 2017

Issue description

Currently this uses the "document.onLoad" signal, which is known broken for modern web pages that use lightweight loaders.

I propose we use the page load metric definitions of done, which involve a measure of CPU + network quiescence.
 
Assigning to lpy@, who is actively working on the plumbing for this.

Comment 2 by zh...@chromium.org, Jul 28 2017

Blocking: 730098

Comment 3 by l...@chromium.org, Aug 9 2017

Labels: -Pri-2 Pri-1
Status: Started (was: Assigned)
raise it to P1

Comment 4 by l...@chromium.org, Sep 5 2017

Labels: Hotlist-TooManyTabs
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6161e912bd613a6487e12410344cff99ba994db4

commit 6161e912bd613a6487e12410344cff99ba994db4
Author: Peiyong Lin <lpy@chromium.org>
Date: Thu Oct 05 17:10:25 2017

Merge network idleness signal with Performance Monitor.

Previously we have a dedicated detector to detect network idleness signal and
send it to services/resource_coordinator, however, it uses a timer, thus it
can't handle cases like, if a task is being processed for a long time, network
requests won't be able to be fired, thus when the task is finished processing
the network idleness signal will be fired before network requests are fired,
which results in inaccurate network idleness signal.

So, in this patch, we excluded task processing time in network idle time
accumulation, and merged Performance Monitor life cycle report into the current
detector.

Minor: renamed kNetworkIdle to kNetworkAlmostIdle, renamed class/file name to
IdlenessDetector.

BUG= 730098 ,  749789 

Change-Id: Ied003129cd5ddf7c1ebb93019723e11e9a31be04
Reviewed-on: https://chromium-review.googlesource.com/664094
Commit-Queue: lpy <lpy@chromium.org>
Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Zhen Wang <zhenw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506773}
[modify] https://crrev.com/6161e912bd613a6487e12410344cff99ba994db4/services/resource_coordinator/observers/tab_signal_generator_impl.cc
[modify] https://crrev.com/6161e912bd613a6487e12410344cff99ba994db4/services/resource_coordinator/public/interfaces/signals.mojom
[modify] https://crrev.com/6161e912bd613a6487e12410344cff99ba994db4/third_party/WebKit/Source/core/BUILD.gn
[modify] https://crrev.com/6161e912bd613a6487e12410344cff99ba994db4/third_party/WebKit/Source/core/dom/Document.cpp
[modify] https://crrev.com/6161e912bd613a6487e12410344cff99ba994db4/third_party/WebKit/Source/core/frame/LocalFrame.cpp
[modify] https://crrev.com/6161e912bd613a6487e12410344cff99ba994db4/third_party/WebKit/Source/core/frame/LocalFrame.h
[modify] https://crrev.com/6161e912bd613a6487e12410344cff99ba994db4/third_party/WebKit/Source/core/frame/PerformanceMonitor.cpp
[modify] https://crrev.com/6161e912bd613a6487e12410344cff99ba994db4/third_party/WebKit/Source/core/frame/PerformanceMonitor.h
[modify] https://crrev.com/6161e912bd613a6487e12410344cff99ba994db4/third_party/WebKit/Source/core/loader/BUILD.gn
[modify] https://crrev.com/6161e912bd613a6487e12410344cff99ba994db4/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[add] https://crrev.com/6161e912bd613a6487e12410344cff99ba994db4/third_party/WebKit/Source/core/loader/IdlenessDetector.cpp
[add] https://crrev.com/6161e912bd613a6487e12410344cff99ba994db4/third_party/WebKit/Source/core/loader/IdlenessDetector.h
[add] https://crrev.com/6161e912bd613a6487e12410344cff99ba994db4/third_party/WebKit/Source/core/loader/IdlenessDetectorTest.cpp
[delete] https://crrev.com/c10b27b49c452d8bd72bdbb9bb1a406bfeb3b919/third_party/WebKit/Source/core/loader/NetworkQuietDetector.cpp
[delete] https://crrev.com/c10b27b49c452d8bd72bdbb9bb1a406bfeb3b919/third_party/WebKit/Source/core/loader/NetworkQuietDetector.h
[delete] https://crrev.com/c10b27b49c452d8bd72bdbb9bb1a406bfeb3b919/third_party/WebKit/Source/core/loader/NetworkQuietDetectorTest.cpp
[modify] https://crrev.com/6161e912bd613a6487e12410344cff99ba994db4/third_party/WebKit/Source/core/probe/CoreProbes.json5

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3b5422d7ec8e4ab62512b3f1fb7b5fcb440e834d

commit 3b5422d7ec8e4ab62512b3f1fb7b5fcb440e834d
Author: Peiyong Lin <lpy@chromium.org>
Date: Tue Oct 17 16:15:42 2017

Fix wrong request count when OnWillSendRequest is called.

When OnWillSendRequest is called, the new loader hasn't been added to
the fetcher, thus we should add 1 to ActiveRequestCount() so that we
have the correct network request count.

BUG= 730098 ,  749789 

Change-Id: I7e688d522d2f4272c05210ba2c6dd09666e1b5db
Reviewed-on: https://chromium-review.googlesource.com/713655
Commit-Queue: lpy <lpy@chromium.org>
Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509402}
[modify] https://crrev.com/3b5422d7ec8e4ab62512b3f1fb7b5fcb440e834d/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/3b5422d7ec8e4ab62512b3f1fb7b5fcb440e834d/third_party/WebKit/Source/core/loader/IdlenessDetector.cpp
[modify] https://crrev.com/3b5422d7ec8e4ab62512b3f1fb7b5fcb440e834d/third_party/WebKit/Source/core/loader/IdlenessDetector.h

Blocking: 725128
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1b569774f891dbdb8a1ec781ca779cefeb3b7ef4

commit 1b569774f891dbdb8a1ec781ca779cefeb3b7ef4
Author: Peiyong Lin <lpy@chromium.org>
Date: Thu Nov 02 20:48:43 2017

Plumb main thread load signal to GRC.

To complete PageAlmostIdle signal, we need to estimate when CPU is not
intensively processing tasks. MainThreadLoad is a very good approximation. In
this patch we plumb the signal to GRC and combine it with network idle signal
to figure out when a page is almost idle. This patch also adds finch trial
configuration with a parameter to tweak the percentage of main thread load we
want to consider as idle.

BUG= 749789 , 778766

Change-Id: I84b2009e6c7516d084a6c278a383e8c210da8211
Reviewed-on: https://chromium-review.googlesource.com/740574
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Zhen Wang <zhenw@chromium.org>
Commit-Queue: lpy <lpy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513599}
[modify] https://crrev.com/1b569774f891dbdb8a1ec781ca779cefeb3b7ef4/chrome/browser/resource_coordinator/tab_manager_web_contents_data.h
[modify] https://crrev.com/1b569774f891dbdb8a1ec781ca779cefeb3b7ef4/services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.cc
[modify] https://crrev.com/1b569774f891dbdb8a1ec781ca779cefeb3b7ef4/services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.h
[modify] https://crrev.com/1b569774f891dbdb8a1ec781ca779cefeb3b7ef4/services/resource_coordinator/coordination_unit/frame_coordination_unit_impl_unittest.cc
[modify] https://crrev.com/1b569774f891dbdb8a1ec781ca779cefeb3b7ef4/services/resource_coordinator/coordination_unit/mock_coordination_unit_graphs.h
[modify] https://crrev.com/1b569774f891dbdb8a1ec781ca779cefeb3b7ef4/services/resource_coordinator/coordination_unit/process_coordination_unit_impl.cc
[modify] https://crrev.com/1b569774f891dbdb8a1ec781ca779cefeb3b7ef4/services/resource_coordinator/coordination_unit/process_coordination_unit_impl.h
[modify] https://crrev.com/1b569774f891dbdb8a1ec781ca779cefeb3b7ef4/services/resource_coordinator/observers/tab_signal_generator_impl.cc
[modify] https://crrev.com/1b569774f891dbdb8a1ec781ca779cefeb3b7ef4/services/resource_coordinator/observers/tab_signal_generator_impl.h
[modify] https://crrev.com/1b569774f891dbdb8a1ec781ca779cefeb3b7ef4/services/resource_coordinator/public/cpp/resource_coordinator_features.cc
[modify] https://crrev.com/1b569774f891dbdb8a1ec781ca779cefeb3b7ef4/services/resource_coordinator/public/cpp/resource_coordinator_features.h
[modify] https://crrev.com/1b569774f891dbdb8a1ec781ca779cefeb3b7ef4/services/resource_coordinator/public/interfaces/coordination_unit.mojom
[modify] https://crrev.com/1b569774f891dbdb8a1ec781ca779cefeb3b7ef4/services/resource_coordinator/public/interfaces/signals.mojom
[modify] https://crrev.com/1b569774f891dbdb8a1ec781ca779cefeb3b7ef4/third_party/WebKit/Source/core/DEPS
[modify] https://crrev.com/1b569774f891dbdb8a1ec781ca779cefeb3b7ef4/third_party/WebKit/Source/core/loader/IdlenessDetector.cpp
[modify] https://crrev.com/1b569774f891dbdb8a1ec781ca779cefeb3b7ef4/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/RendererResourceCoordinator.cpp
[modify] https://crrev.com/1b569774f891dbdb8a1ec781ca779cefeb3b7ef4/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/RendererResourceCoordinator.h
[modify] https://crrev.com/1b569774f891dbdb8a1ec781ca779cefeb3b7ef4/third_party/WebKit/Source/platform/scheduler/renderer/DEPS
[modify] https://crrev.com/1b569774f891dbdb8a1ec781ca779cefeb3b7ef4/third_party/WebKit/Source/platform/scheduler/renderer/renderer_metrics_helper.cc
[modify] https://crrev.com/1b569774f891dbdb8a1ec781ca779cefeb3b7ef4/third_party/WebKit/Source/platform/scheduler/renderer/renderer_metrics_helper.h

Project Member

Comment 9 by bugdroid1@chromium.org, Nov 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/740f931411d0a01212d0d6cd7aa7d66d047bc09c

commit 740f931411d0a01212d0d6cd7aa7d66d047bc09c
Author: Peiyong Lin <lpy@chromium.org>
Date: Wed Nov 08 00:26:56 2017

[PageAlmostIdle] Plumb idle signal to TabManager.

This patch plumbs idle signal to TabManager in order to better schedule
background tabs loading. When PageIdleSignal feature is enabled,
TabManager::WebContentsData will ignore DidStopLoading signal from WebContents,
and only use NotifyAlmostIdle to notify TabManager a loading is considered
done.

BUG= 730098 ,  749789 , 751210, 778766

Change-Id: Iaef8abe2608bd18537ca35838a3d2d8e1c81f972
Reviewed-on: https://chromium-review.googlesource.com/755483
Reviewed-by: Zhen Wang <zhenw@chromium.org>
Commit-Queue: lpy <lpy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514668}
[modify] https://crrev.com/740f931411d0a01212d0d6cd7aa7d66d047bc09c/chrome/browser/resource_coordinator/page_signal_receiver.cc
[modify] https://crrev.com/740f931411d0a01212d0d6cd7aa7d66d047bc09c/chrome/browser/resource_coordinator/tab_manager.cc
[modify] https://crrev.com/740f931411d0a01212d0d6cd7aa7d66d047bc09c/chrome/browser/resource_coordinator/tab_manager.h
[modify] https://crrev.com/740f931411d0a01212d0d6cd7aa7d66d047bc09c/chrome/browser/resource_coordinator/tab_manager_resource_coordinator_signal_observer.cc
[modify] https://crrev.com/740f931411d0a01212d0d6cd7aa7d66d047bc09c/chrome/browser/resource_coordinator/tab_manager_stats_collector.cc
[modify] https://crrev.com/740f931411d0a01212d0d6cd7aa7d66d047bc09c/chrome/browser/resource_coordinator/tab_manager_stats_collector.h
[modify] https://crrev.com/740f931411d0a01212d0d6cd7aa7d66d047bc09c/chrome/browser/resource_coordinator/tab_manager_stats_collector_unittest.cc
[modify] https://crrev.com/740f931411d0a01212d0d6cd7aa7d66d047bc09c/chrome/browser/resource_coordinator/tab_manager_unittest.cc
[modify] https://crrev.com/740f931411d0a01212d0d6cd7aa7d66d047bc09c/chrome/browser/resource_coordinator/tab_manager_web_contents_data.cc
[modify] https://crrev.com/740f931411d0a01212d0d6cd7aa7d66d047bc09c/chrome/browser/resource_coordinator/tab_manager_web_contents_data.h
[modify] https://crrev.com/740f931411d0a01212d0d6cd7aa7d66d047bc09c/services/resource_coordinator/observers/page_signal_generator_impl.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Nov 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1529ea24851e86c5e75cc50e9cc0108bdc8a74cf

commit 1529ea24851e86c5e75cc50e9cc0108bdc8a74cf
Author: Peiyong Lin <lpy@chromium.org>
Date: Thu Nov 16 18:06:21 2017

[GRC] Plumb page almost idle signal to TabLoader.

Page almost idle signal is calculated from main thread task load and network
activity. This patch plumbs page almost idle signal to TabLoader to experiment
using this signal to schedule tab loading in SessionRestore. Currently,
SessionRestore uses timeout and onload as signals to start to load next tab. A
Finch Trial will be set up to experiment it.

BUG= 749789 , 778766

Change-Id: Id83b6decc1ade9d3377b5184f0c717ac24bebabb
Reviewed-on: https://chromium-review.googlesource.com/764569
Reviewed-by: Zhen Wang <zhenw@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: lpy <lpy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517124}
[modify] https://crrev.com/1529ea24851e86c5e75cc50e9cc0108bdc8a74cf/chrome/browser/resource_coordinator/page_signal_receiver.cc
[modify] https://crrev.com/1529ea24851e86c5e75cc50e9cc0108bdc8a74cf/chrome/browser/resource_coordinator/page_signal_receiver.h
[modify] https://crrev.com/1529ea24851e86c5e75cc50e9cc0108bdc8a74cf/chrome/browser/resource_coordinator/tab_manager_resource_coordinator_signal_observer.cc
[modify] https://crrev.com/1529ea24851e86c5e75cc50e9cc0108bdc8a74cf/chrome/browser/resource_coordinator/tab_manager_resource_coordinator_signal_observer.h
[modify] https://crrev.com/1529ea24851e86c5e75cc50e9cc0108bdc8a74cf/chrome/browser/sessions/tab_loader.cc
[modify] https://crrev.com/1529ea24851e86c5e75cc50e9cc0108bdc8a74cf/chrome/browser/sessions/tab_loader.h
[modify] https://crrev.com/1529ea24851e86c5e75cc50e9cc0108bdc8a74cf/chrome/browser/sessions/tab_loader_unittest.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Nov 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3320e8a6289d7a8cfe44a4103df15a0f9b318404

commit 3320e8a6289d7a8cfe44a4103df15a0f9b318404
Author: Zhen Wang <zhenw@chromium.org>
Date: Wed Nov 22 02:02:51 2017

Add finch param to control tab load timeout for session restore and background tab opening

This CL adds a finch param to control the tab load time out for both session
restore and background tab opening, and adds UMA metrics to record how often
timeout happens. It also moves staggered background tab opening feature
definition into its own file.

For testing, use:
./out/Release/chrome
--enable-features=StaggeredBackgroundTabOpening,\
  StaggeredBackgroundTabOpeningExperiment,\
  "CustomizedTabLoadTimeout<MyStudy" \
--force-fieldtrials=MyStudy/Group1 \
--force-fieldtrial-params=MyStudy.Group1:tabLoadTimeoutInMs/1000

Bug:  730098 , 749789 ,751210
Change-Id: If21052b1860e4a985986ef8869605998c1f7527e
Reviewed-on: https://chromium-review.googlesource.com/770256
Commit-Queue: Zhen Wang <zhenw@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Reviewed-by: lpy <lpy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518499}
[modify] https://crrev.com/3320e8a6289d7a8cfe44a4103df15a0f9b318404/chrome/browser/BUILD.gn
[modify] https://crrev.com/3320e8a6289d7a8cfe44a4103df15a0f9b318404/chrome/browser/resource_coordinator/background_tab_navigation_throttle.cc
[modify] https://crrev.com/3320e8a6289d7a8cfe44a4103df15a0f9b318404/chrome/browser/resource_coordinator/background_tab_navigation_throttle_unittest.cc
[modify] https://crrev.com/3320e8a6289d7a8cfe44a4103df15a0f9b318404/chrome/browser/resource_coordinator/tab_manager.cc
[add] https://crrev.com/3320e8a6289d7a8cfe44a4103df15a0f9b318404/chrome/browser/resource_coordinator/tab_manager_features.cc
[add] https://crrev.com/3320e8a6289d7a8cfe44a4103df15a0f9b318404/chrome/browser/resource_coordinator/tab_manager_features.h
[modify] https://crrev.com/3320e8a6289d7a8cfe44a4103df15a0f9b318404/chrome/browser/resource_coordinator/tab_manager_stats_collector.cc
[modify] https://crrev.com/3320e8a6289d7a8cfe44a4103df15a0f9b318404/chrome/browser/resource_coordinator/tab_manager_stats_collector.h
[modify] https://crrev.com/3320e8a6289d7a8cfe44a4103df15a0f9b318404/chrome/browser/resource_coordinator/tab_manager_unittest.cc
[modify] https://crrev.com/3320e8a6289d7a8cfe44a4103df15a0f9b318404/chrome/browser/sessions/session_restore_stats_collector.cc
[modify] https://crrev.com/3320e8a6289d7a8cfe44a4103df15a0f9b318404/chrome/browser/sessions/session_restore_stats_collector.h
[modify] https://crrev.com/3320e8a6289d7a8cfe44a4103df15a0f9b318404/chrome/browser/sessions/tab_loader.cc
[modify] https://crrev.com/3320e8a6289d7a8cfe44a4103df15a0f9b318404/chrome/browser/sessions/tab_loader_delegate.cc
[modify] https://crrev.com/3320e8a6289d7a8cfe44a4103df15a0f9b318404/chrome/common/chrome_features.cc
[modify] https://crrev.com/3320e8a6289d7a8cfe44a4103df15a0f9b318404/chrome/common/chrome_features.h
[modify] https://crrev.com/3320e8a6289d7a8cfe44a4103df15a0f9b318404/tools/metrics/histograms/histograms.xml

Owner: chrisha@chromium.org
Assigning to chrisha@ as lpy@ has left Chrome team.
Labels: -M-64 M-65
Blockedon: 800903
Project Member

Comment 15 by bugdroid1@chromium.org, Mar 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7cb13798c1800e9bfe967800071c6ea777fcf81a

commit 7cb13798c1800e9bfe967800071c6ea777fcf81a
Author: Chris Hamilton <chrisha@chromium.org>
Date: Mon Mar 12 21:58:00 2018

Make PageAlmostIdle transitions determinate.

This adds a final timeout to PageAlmostIdle transitions so that they always fire at some point.
This notification is intended to mean "initial load has finished", and it makes sense for this
to fire at some point. This simplifies logic in an upcoming CL which factors
observation logic out of the TabManager into a helper component.

BUG= 749789 

Change-Id: I1db03f991220eebd33251dc5258a2760042eedef
Reviewed-on: https://chromium-review.googlesource.com/929985
Commit-Queue: Chris Hamilton <chrisha@chromium.org>
Reviewed-by: lpy <lpy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542614}
[modify] https://crrev.com/7cb13798c1800e9bfe967800071c6ea777fcf81a/services/resource_coordinator/observers/page_signal_generator_impl.cc
[modify] https://crrev.com/7cb13798c1800e9bfe967800071c6ea777fcf81a/services/resource_coordinator/observers/page_signal_generator_impl.h
[modify] https://crrev.com/7cb13798c1800e9bfe967800071c6ea777fcf81a/services/resource_coordinator/observers/page_signal_generator_impl_unittest.cc

Comment 16 by ojan@chromium.org, May 8 2018

Cc: -ojan@chromium.org
Status: Fixed (was: Started)
This is long since fixed, and shipping as of M68.

Sign in to add a comment