Wrong started_in_foreground for OnStart() in session restore for PlzNavigate |
|||||
Issue descriptionChrome Version: 61.0.3152.0 (Developer Build) (64-bit) Built from origin/master, commit 0735b748b69bd38f8467f4514e2c7dc9c6f2166d, July 7. What steps will reproduce the problem? (1) Open one Chrome window with 1 tab cnn.com out/Release/chrome --enable-browser-side-navigation (2) Turn on "Continue Where I Left Off" Startup setting (3) Close the Chrome window (4) Reopen Chrome What is the expected result? started_in_foreground in PageLoadMetricsObserver::OnStart() is true. What happens instead? started_in_foreground in PageLoadMetricsObserver::OnStart() is false.
,
Jul 7 2017
,
Jul 7 2017
Thanks for investigating! As Zhen mentioned in an earlier email, we fixed this for some cases by passing the CreateParams to TabHelpers. https://codereview.chromium.org/2894973002 However, there are some cases where tabs are created in one part of the code, and passed to tab helpers in another. For these, we were unable to pass the CreateParams to tab helpers. Session restore appears to be one such case. We should figure out how to plumb the CreateParams to tab helpers for session restore. Duc, would you be able to figure out the call path for WebContents created by the session restore code to be provided to AttachTabHelpers(...)? If we can trace this, we may be able to figure out how to plumb the CreateParams through.
,
Jul 10 2017
,
Jul 11 2017
,
Jul 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/086c3c5421ce6dd29298b5313580f9beb0a6d779 commit 086c3c5421ce6dd29298b5313580f9beb0a6d779 Author: Duc Bui <ducbui@google.com> Date: Fri Jul 14 18:15:08 2017 Fix started_in_foreground for OnStart() in session restore. started_in_foreground in PageLoadMetricsObserver::OnStart() is false for foreground tabs in session restore. This patches fixes 2 things: 1. Correct the in_foreground variable in MetricsWebContentsObserver constructor. 2. Pass the correct visibility status of tabs in session_restore.cc. TBR=xiyuan@chromium.org BUG= 740252 Change-Id: Ic8ab868bc850b20070813ec659d64c60345cc160 Reviewed-on: https://chromium-review.googlesource.com/569578 Commit-Queue: Duc Bui <ducbui@google.com> Reviewed-by: James Cook <jamescook@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Bryan McQuade <bmcquade@chromium.org> Reviewed-by: Fadi Meawad <fmeawad@chromium.org> Cr-Commit-Position: refs/heads/master@{#486809} [modify] https://crrev.com/086c3c5421ce6dd29298b5313580f9beb0a6d779/chrome/browser/page_load_metrics/metrics_web_contents_observer.cc [modify] https://crrev.com/086c3c5421ce6dd29298b5313580f9beb0a6d779/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc [modify] https://crrev.com/086c3c5421ce6dd29298b5313580f9beb0a6d779/chrome/browser/page_load_metrics/page_load_tracker.h [modify] https://crrev.com/086c3c5421ce6dd29298b5313580f9beb0a6d779/chrome/browser/sessions/session_restore.cc [modify] https://crrev.com/086c3c5421ce6dd29298b5313580f9beb0a6d779/chrome/browser/sessions/session_restore_browsertest_chromeos.cc
,
Jul 14 2017
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by ducbui@google.com
, Jul 7 2017