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

Issue 762133 link

Starred by 5 users
Status: Fixed
Owner:
█ zh...@chromium.org
Last visit > 30 days ago
Closed: Dec 2017
Cc:
edwardjung@chromium.org
ojan@chromium.org
chrisha@chromium.org
pkasting@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Feature

Blocking:
issue 730098
issue 751210



Sign in to add a comment

Fix tabstrip UI for staggered background tab opening

Project Member Reported by zh...@chromium.org, Sep 5 2017 
Currently the tabstrip UI shows "Loading..." or "Untitled" when the background tab's loading is paused. After the discussion with UI team about the UI impact. We will add logic to restore favicon (when possible, otherwise use generic 'page' favicon), and set hostname as title for deferred-load background tabs.

Details:
- When background tab's loading is paused: show favicon + hostname.
- When background tab starts to load: this will be controlled by the loading process and show favicon and title set by the page.
- When the background tab has been loaded: favicon and tiltle set by the page.

Comment 1 by zh...@chromium.org, Sep 5 2017

Blocking: 751210 730098
We can set up finch experiment to see the impact from different UI. We can use TabManager.BackgroundTabOpening.SwitchToTab to check if any UI would trigger the user to switch tabs more often.

Comment 2 by zh...@chromium.org, Sep 5 2017 

Issue 761791 has been merged into this issue.

Comment 3 by edwardjung@chromium.org, Sep 5 2017 

What metrics outcome would validate the experiment? 

A reduction in all the tab state buckets? Since the user can identify which tab it is they want and switch to it directly rather than systematically through all the tabs.

I'm thinking specifically the 'Tab is loading' and 'Tab is not loading' buckets would be reduced, since that would be cycling through all the tabs behaviour. In the past even the ones not currently loading would have a spinner. 

Does that sound right?

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

Yes. I am expecting the total count and the 'Tab is not loading'/'Tab is loading' buckets are both reduced.
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 4 2017 

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

commit a60bcdada48c6a307d1b4dd7d02302f216645607
Author: Zhen Wang <zhenw@chromium.org>
Date: Fri Nov 03 23:51:56 2017

Update spinner/title logic for session restore and background tab opening

This prototype updates the spinner/title logic for session restore and
staggered background tab opening feature.

For session restore, we do not want to show the spinner for backgrounded
session-restore created tabs.

For staggered background tab opening feature, when a background tab's
navigation is delayed, we want to show the favicon from history service and
the host as the title for better user experience.

Bug:  762133 , 758684
Change-Id: I16a029f261229c553edefad1fe2d71b3df9c7a3a
Reviewed-on: https://chromium-review.googlesource.com/734201
Commit-Queue: Zhen Wang <zhenw@chromium.org>
Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513969}
[modify] https://crrev.com/a60bcdada48c6a307d1b4dd7d02302f216645607/chrome/browser/resource_coordinator/tab_manager.cc
[modify] https://crrev.com/a60bcdada48c6a307d1b4dd7d02302f216645607/chrome/browser/resource_coordinator/tab_manager.h
[modify] https://crrev.com/a60bcdada48c6a307d1b4dd7d02302f216645607/chrome/browser/resource_coordinator/tab_manager_unittest.cc
[modify] https://crrev.com/a60bcdada48c6a307d1b4dd7d02302f216645607/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/a60bcdada48c6a307d1b4dd7d02302f216645607/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
[modify] https://crrev.com/a60bcdada48c6a307d1b4dd7d02302f216645607/chrome/browser/ui/tab_helpers.cc
[add] https://crrev.com/a60bcdada48c6a307d1b4dd7d02302f216645607/chrome/browser/ui/tab_ui_helper.cc
[add] https://crrev.com/a60bcdada48c6a307d1b4dd7d02302f216645607/chrome/browser/ui/tab_ui_helper.h
[modify] https://crrev.com/a60bcdada48c6a307d1b4dd7d02302f216645607/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
[modify] https://crrev.com/a60bcdada48c6a307d1b4dd7d02302f216645607/chrome/browser/ui/views/tabs/browser_tab_strip_controller.h
[modify] https://crrev.com/a60bcdada48c6a307d1b4dd7d02302f216645607/chrome/browser/ui/views/tabs/tab.cc
[modify] https://crrev.com/a60bcdada48c6a307d1b4dd7d02302f216645607/chrome/browser/ui/views/tabs/tab.h
[modify] https://crrev.com/a60bcdada48c6a307d1b4dd7d02302f216645607/chrome/browser/ui/views/tabs/tab_renderer_data.cc
[modify] https://crrev.com/a60bcdada48c6a307d1b4dd7d02302f216645607/chrome/browser/ui/views/tabs/tab_renderer_data.h
[modify] https://crrev.com/a60bcdada48c6a307d1b4dd7d02302f216645607/chrome/browser/ui/views/tabs/tab_unittest.cc
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 6 2017 

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

commit dc9231eb012b76968f2fe3c3332ec131e0cb3e5b
Author: Zhen Wang <zhenw@chromium.org>
Date: Mon Nov 06 06:51:29 2017

Revert "Update spinner/title logic for session restore and background tab opening"

This reverts commit a60bcdada48c6a307d1b4dd7d02302f216645607.

Reason for revert:

Crashing in canary. See http://crbug.com/781669

Original change's description:
> Update spinner/title logic for session restore and background tab opening
> 
> This prototype updates the spinner/title logic for session restore and
> staggered background tab opening feature.
> 
> For session restore, we do not want to show the spinner for backgrounded
> session-restore created tabs.
> 
> For staggered background tab opening feature, when a background tab's
> navigation is delayed, we want to show the favicon from history service and
> the host as the title for better user experience.
> 
> Bug:  762133 , 758684
> Change-Id: I16a029f261229c553edefad1fe2d71b3df9c7a3a
> Reviewed-on: https://chromium-review.googlesource.com/734201
> Commit-Queue: Zhen Wang <zhenw@chromium.org>
> Reviewed-by: Chris Hamilton <chrisha@chromium.org>
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#513969}

TBR=avi@chromium.org,sky@chromium.org,chrisha@chromium.org,zhenw@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  762133 , 758684, 781669
Change-Id: I2208a3a80480edc8684ce733ae4b0be793a4b258
Reviewed-on: https://chromium-review.googlesource.com/753709
Reviewed-by: Zhen Wang <zhenw@chromium.org>
Commit-Queue: Zhen Wang <zhenw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514094}
[modify] https://crrev.com/dc9231eb012b76968f2fe3c3332ec131e0cb3e5b/chrome/browser/resource_coordinator/tab_manager.cc
[modify] https://crrev.com/dc9231eb012b76968f2fe3c3332ec131e0cb3e5b/chrome/browser/resource_coordinator/tab_manager.h
[modify] https://crrev.com/dc9231eb012b76968f2fe3c3332ec131e0cb3e5b/chrome/browser/resource_coordinator/tab_manager_unittest.cc
[modify] https://crrev.com/dc9231eb012b76968f2fe3c3332ec131e0cb3e5b/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/dc9231eb012b76968f2fe3c3332ec131e0cb3e5b/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
[modify] https://crrev.com/dc9231eb012b76968f2fe3c3332ec131e0cb3e5b/chrome/browser/ui/tab_helpers.cc
[delete] https://crrev.com/57e1b8ab157e9bbe862b1dbebeee5bcf04756b37/chrome/browser/ui/tab_ui_helper.cc
[delete] https://crrev.com/57e1b8ab157e9bbe862b1dbebeee5bcf04756b37/chrome/browser/ui/tab_ui_helper.h
[modify] https://crrev.com/dc9231eb012b76968f2fe3c3332ec131e0cb3e5b/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
[modify] https://crrev.com/dc9231eb012b76968f2fe3c3332ec131e0cb3e5b/chrome/browser/ui/views/tabs/browser_tab_strip_controller.h
[modify] https://crrev.com/dc9231eb012b76968f2fe3c3332ec131e0cb3e5b/chrome/browser/ui/views/tabs/tab.cc
[modify] https://crrev.com/dc9231eb012b76968f2fe3c3332ec131e0cb3e5b/chrome/browser/ui/views/tabs/tab.h
[modify] https://crrev.com/dc9231eb012b76968f2fe3c3332ec131e0cb3e5b/chrome/browser/ui/views/tabs/tab_renderer_data.cc
[modify] https://crrev.com/dc9231eb012b76968f2fe3c3332ec131e0cb3e5b/chrome/browser/ui/views/tabs/tab_renderer_data.h
[modify] https://crrev.com/dc9231eb012b76968f2fe3c3332ec131e0cb3e5b/chrome/browser/ui/views/tabs/tab_unittest.cc
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 7 2017

Labels: merge-merged-3260
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/73679839bd0520ad541e3b0bdccbca44ca0abc4d

commit 73679839bd0520ad541e3b0bdccbca44ca0abc4d
Author: Zhen Wang <zhenw@chromium.org>
Date: Tue Nov 07 00:48:19 2017

Revert "Update spinner/title logic for session restore and background tab opening"

This reverts commit a60bcdada48c6a307d1b4dd7d02302f216645607.

Reason for revert:

Crashing in canary. See http://crbug.com/781669

Original change's description:
> Update spinner/title logic for session restore and background tab opening
> 
> This prototype updates the spinner/title logic for session restore and
> staggered background tab opening feature.
> 
> For session restore, we do not want to show the spinner for backgrounded
> session-restore created tabs.
> 
> For staggered background tab opening feature, when a background tab's
> navigation is delayed, we want to show the favicon from history service and
> the host as the title for better user experience.
> 
> Bug:  762133 , 758684
> Change-Id: I16a029f261229c553edefad1fe2d71b3df9c7a3a
> Reviewed-on: https://chromium-review.googlesource.com/734201
> Commit-Queue: Zhen Wang <zhenw@chromium.org>
> Reviewed-by: Chris Hamilton <chrisha@chromium.org>
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#513969}

TBR=avi@chromium.org,sky@chromium.org,chrisha@chromium.org,zhenw@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  762133 , 758684, 781669
Change-Id: I2208a3a80480edc8684ce733ae4b0be793a4b258
Reviewed-on: https://chromium-review.googlesource.com/753709
Reviewed-by: Zhen Wang <zhenw@chromium.org>
Commit-Queue: Zhen Wang <zhenw@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#514094}(cherry picked from commit dc9231eb012b76968f2fe3c3332ec131e0cb3e5b)
Reviewed-on: https://chromium-review.googlesource.com/756186
Reviewed-by: Abdul Syed <abdulsyed@google.com>
Cr-Commit-Position: refs/branch-heads/3260@{#2}
Cr-Branched-From: eec83ceb16d47f70f5804a140687e268a6230b5c-refs/heads/master@{#514066}
[modify] https://crrev.com/73679839bd0520ad541e3b0bdccbca44ca0abc4d/chrome/browser/resource_coordinator/tab_manager.cc
[modify] https://crrev.com/73679839bd0520ad541e3b0bdccbca44ca0abc4d/chrome/browser/resource_coordinator/tab_manager.h
[modify] https://crrev.com/73679839bd0520ad541e3b0bdccbca44ca0abc4d/chrome/browser/resource_coordinator/tab_manager_unittest.cc
[modify] https://crrev.com/73679839bd0520ad541e3b0bdccbca44ca0abc4d/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/73679839bd0520ad541e3b0bdccbca44ca0abc4d/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
[modify] https://crrev.com/73679839bd0520ad541e3b0bdccbca44ca0abc4d/chrome/browser/ui/tab_helpers.cc
[delete] https://crrev.com/949c1540cbf883023a48fec4dc6a720a3817ef2e/chrome/browser/ui/tab_ui_helper.cc
[delete] https://crrev.com/949c1540cbf883023a48fec4dc6a720a3817ef2e/chrome/browser/ui/tab_ui_helper.h
[modify] https://crrev.com/73679839bd0520ad541e3b0bdccbca44ca0abc4d/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
[modify] https://crrev.com/73679839bd0520ad541e3b0bdccbca44ca0abc4d/chrome/browser/ui/views/tabs/browser_tab_strip_controller.h
[modify] https://crrev.com/73679839bd0520ad541e3b0bdccbca44ca0abc4d/chrome/browser/ui/views/tabs/tab.cc
[modify] https://crrev.com/73679839bd0520ad541e3b0bdccbca44ca0abc4d/chrome/browser/ui/views/tabs/tab.h
[modify] https://crrev.com/73679839bd0520ad541e3b0bdccbca44ca0abc4d/chrome/browser/ui/views/tabs/tab_renderer_data.cc
[modify] https://crrev.com/73679839bd0520ad541e3b0bdccbca44ca0abc4d/chrome/browser/ui/views/tabs/tab_renderer_data.h
[modify] https://crrev.com/73679839bd0520ad541e3b0bdccbca44ca0abc4d/chrome/browser/ui/views/tabs/tab_unittest.cc
Project Member

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

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

commit 3fb94bff1f5368ee0f4f89590d496d21b08739ce
Author: Zhen Wang <zhenw@chromium.org>
Date: Wed Nov 08 22:27:07 2017

Reland - Update spinner/title logic for session restore and background tab opening

This CL relands the previous CL with fix for the crash (see crbug.com/781669)

Previous CL: https://chromium-review.googlesource.com/c/chromium/src/+/734201

**** Original CL discription ****

This prototype updates the spinner/title logic for session restore and
staggered background tab opening feature.

For session restore, we do not want to show the spinner for backgrounded
session-restore created tabs.

For staggered background tab opening feature, when a background tab's
navigation is delayed, we want to show the favicon from history service and
the host as the title for better user experience.

Bug:  762133 , 758684
Change-Id: I32643d5b269c4afa5c7e30b23b07d6bda16ef386
Reviewed-on: https://chromium-review.googlesource.com/755540
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Zhen Wang <zhenw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514972}
[modify] https://crrev.com/3fb94bff1f5368ee0f4f89590d496d21b08739ce/chrome/browser/resource_coordinator/background_tab_navigation_throttle.cc
[modify] https://crrev.com/3fb94bff1f5368ee0f4f89590d496d21b08739ce/chrome/browser/resource_coordinator/background_tab_navigation_throttle_unittest.cc
[modify] https://crrev.com/3fb94bff1f5368ee0f4f89590d496d21b08739ce/chrome/browser/resource_coordinator/tab_manager.cc
[modify] https://crrev.com/3fb94bff1f5368ee0f4f89590d496d21b08739ce/chrome/browser/resource_coordinator/tab_manager.h
[modify] https://crrev.com/3fb94bff1f5368ee0f4f89590d496d21b08739ce/chrome/browser/resource_coordinator/tab_manager_unittest.cc
[modify] https://crrev.com/3fb94bff1f5368ee0f4f89590d496d21b08739ce/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/3fb94bff1f5368ee0f4f89590d496d21b08739ce/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
[modify] https://crrev.com/3fb94bff1f5368ee0f4f89590d496d21b08739ce/chrome/browser/ui/tab_helpers.cc
[add] https://crrev.com/3fb94bff1f5368ee0f4f89590d496d21b08739ce/chrome/browser/ui/tab_ui_helper.cc
[add] https://crrev.com/3fb94bff1f5368ee0f4f89590d496d21b08739ce/chrome/browser/ui/tab_ui_helper.h
[modify] https://crrev.com/3fb94bff1f5368ee0f4f89590d496d21b08739ce/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
[modify] https://crrev.com/3fb94bff1f5368ee0f4f89590d496d21b08739ce/chrome/browser/ui/views/tabs/browser_tab_strip_controller.h
[modify] https://crrev.com/3fb94bff1f5368ee0f4f89590d496d21b08739ce/chrome/browser/ui/views/tabs/tab.cc
[modify] https://crrev.com/3fb94bff1f5368ee0f4f89590d496d21b08739ce/chrome/browser/ui/views/tabs/tab.h
[modify] https://crrev.com/3fb94bff1f5368ee0f4f89590d496d21b08739ce/chrome/browser/ui/views/tabs/tab_renderer_data.cc
[modify] https://crrev.com/3fb94bff1f5368ee0f4f89590d496d21b08739ce/chrome/browser/ui/views/tabs/tab_renderer_data.h
[modify] https://crrev.com/3fb94bff1f5368ee0f4f89590d496d21b08739ce/chrome/browser/ui/views/tabs/tab_unittest.cc
Project Member

Comment 9 by bugdroid1@chromium.org, Dec 6 2017 

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

commit d74f79c552d31930411087580bc014780b1d7bf6
Author: Zhen Wang <zhenw@chromium.org>
Date: Wed Dec 06 17:36:00 2017

Show throbber when background tab resumes loading

This CL fixes a bug about the spinner logic for staggered background
tab opening feature. In this fix, we keep track of if the WebContents
was hiding throbber. So later when we need to show throbber, we can force
update the spinner even if the loading state is not changing.

Before: background tab keeps showing favicon even when it resume to load.
After: background tab will show spinner when it resumes to load.

For more details, see  crbug.com/762133  for the desired behavior.

Bug:  762133 ,751210
Change-Id: Ice1643f3ebf08d2f1bf94a47245b6e272187d0fa
Reviewed-on: https://chromium-review.googlesource.com/810037
Commit-Queue: Zhen Wang <zhenw@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522128}
[modify] https://crrev.com/d74f79c552d31930411087580bc014780b1d7bf6/chrome/browser/ui/cocoa/tabs/tab_strip_controller.h
[modify] https://crrev.com/d74f79c552d31930411087580bc014780b1d7bf6/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm

Comment 10 by zh...@chromium.org, Dec 8 2017

Status: Fixed (was: Started)
Project Member

Comment 11 by bugdroid1@chromium.org, Dec 11 2017

Labels: merge-merged-3282
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d51d96cc896d23588f7751ea8e79db7b4577ec29

commit d51d96cc896d23588f7751ea8e79db7b4577ec29
Author: Zhen Wang <zhenw@chromium.org>
Date: Mon Dec 11 17:57:53 2017

Show throbber when background tab resumes loading

This CL fixes a bug about the spinner logic for staggered background
tab opening feature. In this fix, we keep track of if the WebContents
was hiding throbber. So later when we need to show throbber, we can force
update the spinner even if the loading state is not changing.

Before: background tab keeps showing favicon even when it resume to load.
After: background tab will show spinner when it resumes to load.

For more details, see  crbug.com/762133  for the desired behavior.

Bug:  762133 ,751210
Change-Id: Ice1643f3ebf08d2f1bf94a47245b6e272187d0fa
Reviewed-on: https://chromium-review.googlesource.com/810037
Commit-Queue: Zhen Wang <zhenw@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#522128}(cherry picked from commit d74f79c552d31930411087580bc014780b1d7bf6)
Reviewed-on: https://chromium-review.googlesource.com/820090
Reviewed-by: Zhen Wang <zhenw@chromium.org>
Cr-Commit-Position: refs/branch-heads/3282@{#131}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[modify] https://crrev.com/d51d96cc896d23588f7751ea8e79db7b4577ec29/chrome/browser/ui/cocoa/tabs/tab_strip_controller.h
[modify] https://crrev.com/d51d96cc896d23588f7751ea8e79db7b4577ec29/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
►

Sign in to add a comment