New issue
Advanced search Search tips

Issue 864725 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 18
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

Active tabs in minimized windows aren't loaded during session restore

Project Member Reported by chrisha@chromium.org, Jul 17

Issue description

These tabs are no longer loaded due to occlusion tracking optimizations. Fix this.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 18

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

commit 94218d2c7c21146db7306a79341c985bd10a1484
Author: Chris Hamilton <chrisha@chromium.org>
Date: Wed Jul 18 15:33:19 2018

Make TabLoader responsible for loading all tabs.

Currently there's an inversion where active tabs in minimized windows don't get
loaded as they are created. This is an optimization in the occlusion tracking code
that doesn't calls ReloadIfNecessary unless the window hosting the active tab
actually becomes visible. Due to this inversion the TabLoader can start loading
background tabs in the minimized window before the active tab in the minimized
window is ever loaded.

Giving TabLoader responsibility to load all tabs fixes this. In the case of
active and visible tabs whose loads are initiated by the browser this is fine
because calling ReloadIfNecessary twice is effectively a nop, and the TabLoader
is already smart enough to track tabs that have started loading for external
reasons.

BUG= 864725 

Change-Id: I4b86a71470b2cb1a1a9106c4d4d92ee6b18c7284
Reviewed-on: https://chromium-review.googlesource.com/1141071
Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org>
Commit-Queue: Chris Hamilton <chrisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576071}
[modify] https://crrev.com/94218d2c7c21146db7306a79341c985bd10a1484/chrome/browser/sessions/session_restore.cc
[modify] https://crrev.com/94218d2c7c21146db7306a79341c985bd10a1484/chrome/browser/sessions/tab_loader.cc
[modify] https://crrev.com/94218d2c7c21146db7306a79341c985bd10a1484/chrome/browser/sessions/tab_loader.h

Status: Fixed (was: Started)
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 19

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

commit 5f7daa0e13575bd6f67cc4359bf67634a1a4b5da
Author: Chris Hamilton <chrisha@chromium.org>
Date: Thu Jul 19 15:04:32 2018

Revert "Make TabLoader responsible for loading all tabs."

This reverts commit 94218d2c7c21146db7306a79341c985bd10a1484.

Reason for revert: Causing official build test failures.

BUG=865406

Original change's description:
> Make TabLoader responsible for loading all tabs.
> 
> Currently there's an inversion where active tabs in minimized windows don't get
> loaded as they are created. This is an optimization in the occlusion tracking code
> that doesn't calls ReloadIfNecessary unless the window hosting the active tab
> actually becomes visible. Due to this inversion the TabLoader can start loading
> background tabs in the minimized window before the active tab in the minimized
> window is ever loaded.
> 
> Giving TabLoader responsibility to load all tabs fixes this. In the case of
> active and visible tabs whose loads are initiated by the browser this is fine
> because calling ReloadIfNecessary twice is effectively a nop, and the TabLoader
> is already smart enough to track tabs that have started loading for external
> reasons.
> 
> BUG= 864725 
> 
> Change-Id: I4b86a71470b2cb1a1a9106c4d4d92ee6b18c7284
> Reviewed-on: https://chromium-review.googlesource.com/1141071
> Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org>
> Commit-Queue: Chris Hamilton <chrisha@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#576071}

TBR=chrisha@chromium.org,sebmarchand@chromium.org

Change-Id: Icc2545c1a643042e44b15aa3fb4970045bdba105
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  864725 
Reviewed-on: https://chromium-review.googlesource.com/1143565
Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Commit-Queue: Chris Hamilton <chrisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576494}
[modify] https://crrev.com/5f7daa0e13575bd6f67cc4359bf67634a1a4b5da/chrome/browser/sessions/session_restore.cc
[modify] https://crrev.com/5f7daa0e13575bd6f67cc4359bf67634a1a4b5da/chrome/browser/sessions/tab_loader.cc
[modify] https://crrev.com/5f7daa0e13575bd6f67cc4359bf67634a1a4b5da/chrome/browser/sessions/tab_loader.h

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 19

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

commit ae087c53f5ce4557bfb0b92a13651342336fe18a
Author: Chris Hamilton <chrisha@chromium.org>
Date: Thu Jul 19 20:50:22 2018

Reland "Make TabLoader responsible for loading all tabs."

This is a reland of 94218d2c7c21146db7306a79341c985bd10a1484

Original change's description:
> Make TabLoader responsible for loading all tabs.
> 
> Currently there's an inversion where active tabs in minimized windows don't get
> loaded as they are created. This is an optimization in the occlusion tracking code
> that doesn't calls ReloadIfNecessary unless the window hosting the active tab
> actually becomes visible. Due to this inversion the TabLoader can start loading
> background tabs in the minimized window before the active tab in the minimized
> window is ever loaded.
> 
> Giving TabLoader responsibility to load all tabs fixes this. In the case of
> active and visible tabs whose loads are initiated by the browser this is fine
> because calling ReloadIfNecessary twice is effectively a nop, and the TabLoader
> is already smart enough to track tabs that have started loading for external
> reasons.
> 
> BUG= 864725 
> 
> Change-Id: I4b86a71470b2cb1a1a9106c4d4d92ee6b18c7284
> Reviewed-on: https://chromium-review.googlesource.com/1141071
> Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org>
> Commit-Queue: Chris Hamilton <chrisha@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#576071}

Bug:  864725 
Change-Id: Iddde163929790d30ae4b8c565c4859215e6f6938
Reviewed-on: https://chromium-review.googlesource.com/1144080
Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org>
Commit-Queue: Chris Hamilton <chrisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576615}
[modify] https://crrev.com/ae087c53f5ce4557bfb0b92a13651342336fe18a/chrome/browser/sessions/session_restore.cc
[modify] https://crrev.com/ae087c53f5ce4557bfb0b92a13651342336fe18a/chrome/browser/sessions/tab_loader.cc
[modify] https://crrev.com/ae087c53f5ce4557bfb0b92a13651342336fe18a/chrome/browser/sessions/tab_loader.h
[modify] https://crrev.com/ae087c53f5ce4557bfb0b92a13651342336fe18a/chrome/browser/sessions/tab_loader_unittest.cc

Cc: sebmarchand@chromium.org
 Issue 860512  has been merged into this issue.

Sign in to add a comment