New issue
Advanced search Search tips

Issue 871279 link

Starred by 1 user
Status: Fixed
Owner:
mdjones@chromium.org
Closed: Sep 11
Cc:
twelling...@chromium.org
tedc...@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Task

Blocking:
issue 865801



Sign in to add a comment

Visible tab changed event for Android

Project Member Reported by mdjones@chromium.org, Aug 6 
We should support the concept of a "visible tab" that can be observed by classes whose lifecycle revolves around a tab being in the foreground. In practice this will be the tab visible in StaticLayout. When switching to a different layout, onVisibleTabChanged would be called with a null tab.

In general, the idea would be to replace many of the current usages of "getCurrentTab" with this observer. Some easy replacements include:
- DownloadInfobarController
- BottomSheetController
- contextual_suggestions/FetchHelper

An interesting possible follow-up to this would be to create something like a "VisbleTabTabObserver". This would be a tab observer that is automatically attached and detached from the visible tab.

Comment 1 by twelling...@chromium.org, Aug 6

Blocking: 865801
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 5 

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

commit f7f910d0ef77448dc1fd40200bc88d36d9b267b8
Author: Matthew Jones <mdjones@chromium.org>
Date: Wed Sep 05 15:34:58 2018

Add ActivityTabProvider to ChromeActivity

This patch adds ActivityTabProvider to ChromeActivity. This class
provides an easy mechanism for classes that care about the tab
that is currently in the foreground (i.e. not in the tab
switcher/toolbar swipe/animation layout).

The observer provides either the activity's tab or null depending
on the state of the browser. The null event can be ignored depending
on the listener.

The ActivityTabObserver interface only has a single method:
onActivityTabChanged(Tab t)

When an observer is attached to the provider, onActivityTabChanged is
called on only that observer with the current tab (giving it access
without requiring a handle to a larger object).

Bug:  871279 
Change-Id: Ice961224c6690dc79c38f5d6cb255fed4730c8ce
Reviewed-on: https://chromium-review.googlesource.com/1165489
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588872}
[add] https://crrev.com/f7f910d0ef77448dc1fd40200bc88d36d9b267b8/chrome/android/java/src/org/chromium/chrome/browser/ActivityTabProvider.java
[modify] https://crrev.com/f7f910d0ef77448dc1fd40200bc88d36d9b267b8/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
[modify] https://crrev.com/f7f910d0ef77448dc1fd40200bc88d36d9b267b8/chrome/android/java_sources.gni
[add] https://crrev.com/f7f910d0ef77448dc1fd40200bc88d36d9b267b8/chrome/android/javatests/src/org/chromium/chrome/browser/ActivityTabProviderTest.java
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 5 

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

commit cca1d7f5e204cbb686173b2a1479e717b7b8f6fe
Author: Samuel Huang <huangs@chromium.org>
Date: Wed Sep 05 20:32:16 2018

Revert "Add ActivityTabProvider to ChromeActivity"

This reverts commit f7f910d0ef77448dc1fd40200bc88d36d9b267b8.

Reason for revert: https://crbug.com/881012 : chrome_public_test_apk failing on chromium.memory/Android CFI with new failure in org.chromium.chrome.browser.firstrun.FirstRunIntegrationTest#testRedirectChromeTabbedActivityToFirstRun

Original change's description:
> Add ActivityTabProvider to ChromeActivity
> 
> This patch adds ActivityTabProvider to ChromeActivity. This class
> provides an easy mechanism for classes that care about the tab
> that is currently in the foreground (i.e. not in the tab
> switcher/toolbar swipe/animation layout).
> 
> The observer provides either the activity's tab or null depending
> on the state of the browser. The null event can be ignored depending
> on the listener.
> 
> The ActivityTabObserver interface only has a single method:
> onActivityTabChanged(Tab t)
> 
> When an observer is attached to the provider, onActivityTabChanged is
> called on only that observer with the current tab (giving it access
> without requiring a handle to a larger object).
> 
> Bug:  871279 
> Change-Id: Ice961224c6690dc79c38f5d6cb255fed4730c8ce
> Reviewed-on: https://chromium-review.googlesource.com/1165489
> Commit-Queue: Matthew Jones <mdjones@chromium.org>
> Reviewed-by: Ted Choc <tedchoc@chromium.org>
> Reviewed-by: Theresa <twellington@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#588872}

TBR=tedchoc@chromium.org,twellington@chromium.org,mdjones@chromium.org

Change-Id: I2e1a6991f0478632a95b53969394a2314392a5dd
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  871279 
Reviewed-on: https://chromium-review.googlesource.com/1208361
Reviewed-by: Samuel Huang <huangs@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588990}
[delete] https://crrev.com/0c48f75c360c884d12efc451b95e78388de23f72/chrome/android/java/src/org/chromium/chrome/browser/ActivityTabProvider.java
[modify] https://crrev.com/cca1d7f5e204cbb686173b2a1479e717b7b8f6fe/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
[modify] https://crrev.com/cca1d7f5e204cbb686173b2a1479e717b7b8f6fe/chrome/android/java_sources.gni
[delete] https://crrev.com/0c48f75c360c884d12efc451b95e78388de23f72/chrome/android/javatests/src/org/chromium/chrome/browser/ActivityTabProviderTest.java
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 5 

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

commit 113591087657d122c83120f819a2923410a3f922
Author: Matthew Jones <mdjones@chromium.org>
Date: Wed Sep 05 22:03:56 2018

Reland "Add ActivityTabProvider to ChromeActivity"

This is a reland of f7f910d0ef77448dc1fd40200bc88d36d9b267b8 with
the changes from https://chromium-review.googlesource.com/c/chromium/src/+/1208293
applied.

Original change's description:
> Add ActivityTabProvider to ChromeActivity
>
> This patch adds ActivityTabProvider to ChromeActivity. This class
> provides an easy mechanism for classes that care about the tab
> that is currently in the foreground (i.e. not in the tab
> switcher/toolbar swipe/animation layout).
>
> The observer provides either the activity's tab or null depending
> on the state of the browser. The null event can be ignored depending
> on the listener.
>
> The ActivityTabObserver interface only has a single method:
> onActivityTabChanged(Tab t)
>
> When an observer is attached to the provider, onActivityTabChanged is
> called on only that observer with the current tab (giving it access
> without requiring a handle to a larger object).
>
> Bug:  871279 
> Change-Id: Ice961224c6690dc79c38f5d6cb255fed4730c8ce
> Reviewed-on: https://chromium-review.googlesource.com/1165489
> Commit-Queue: Matthew Jones <mdjones@chromium.org>
> Reviewed-by: Ted Choc <tedchoc@chromium.org>
> Reviewed-by: Theresa <twellington@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#588872}

TBR: tedchoc
Bug:  871279 
Change-Id: I9c17590de19db025110cda2171cd1f8d9c1ffa57
Reviewed-on: https://chromium-review.googlesource.com/1208714
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589018}
[add] https://crrev.com/113591087657d122c83120f819a2923410a3f922/chrome/android/java/src/org/chromium/chrome/browser/ActivityTabProvider.java
[modify] https://crrev.com/113591087657d122c83120f819a2923410a3f922/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
[modify] https://crrev.com/113591087657d122c83120f819a2923410a3f922/chrome/android/java_sources.gni
[add] https://crrev.com/113591087657d122c83120f819a2923410a3f922/chrome/android/javatests/src/org/chromium/chrome/browser/ActivityTabProviderTest.java

Comment 7 by mdjones@chromium.org, Sep 11

Status: Fixed (was: Assigned)
Main implementation is complete, marking this bug as fixed.
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 27 

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

commit 2e4facbd313f901aac4ad0f2d6d2718e78009672
Author: Matthew Jones <mdjones@chromium.org>
Date: Tue Nov 27 22:28:28 2018

Add ActivityTabTabObserver

This patch introduces the ActivityTabTabObserver, a TabObserver that
only watches the activity tab. When the activity tab changes, the
observer automatically moves from the previous tab to the current
one. Use of this API is as simple as extending the new class.

The ActivityTabTabObserver itself extends the EmptyTabObserver and
adds a convenience method for notifications about the observed tab
changing (onObservingDifferentTab).

Bug:  871279 
Change-Id: I5f3358953f5fdc5f9c4f9acd790af0b4d3d2d58a
Reviewed-on: https://chromium-review.googlesource.com/c/1347620
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611377}
[modify] https://crrev.com/2e4facbd313f901aac4ad0f2d6d2718e78009672/chrome/android/java/src/org/chromium/chrome/browser/ActivityTabProvider.java
[modify] https://crrev.com/2e4facbd313f901aac4ad0f2d6d2718e78009672/chrome/android/javatests/src/org/chromium/chrome/browser/ActivityTabProviderTest.java
►

Sign in to add a comment