Remove CustomTab and have all CustomTabActivity using Tab

Remove CustomTab and have all CustomTabActivity using Tab

*Adds a TabDelegateFactory that handles the creating of various
delegates for Tab.
*Adds child classes for the delegate factory in DocumentTab and
FullscreenActivityTab and Custom tabs.
*Removes the class CustomTab and gets the activity to handle
initialization.

BUG=546182
Committed: https://crrev.com/5dbc149bebecea186b693b3d780b6965eeffed0f
Cr-Commit-Position: refs/heads/master@{#356385}

[Yusuf, 2015-10-21 21:49:38]

yusufo@chromium.org changed reviewers:
+ dfalcantara@chromium.org, dtrainor@chromium.org

[gone, 2015-10-22 17:16:54]

Added a bunch of annotations for myself since the CL seems to be mostly moving
things.  I'm confused about where the Tab is actually created now, though.

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java
(right):

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java:237:
private void createTab() {
Where does this actually get called from now?  I'm not seeing it anywhere

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java:238:
String url = IntentHandler.getUrlFromIntent(getIntent());
moved from CustomTab#finishNativeInitialization

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java:256:
CustomTabsConnection customTabsConnection =
moved from CustomTab#CustomTab

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java:325:
}
moved from CustomTab#loadUrlAndTrackFromTimestamp

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabDelegateFactory.java
(right):

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabDelegateFactory.java:45:
*/
moved from CustomTab#CustomTabNavigationDelegate

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabDelegateFactory.java:47:
private static final String TAG = "cr_customtabs";
remove cr_; TAGs include it by default now

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabDelegateFactory.java:119:

moved from CustomTab#createWebContentsDelegate

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabDelegateFactory.java:128:
private String mTargetUrl;
nit: move above constructor

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabDelegateFactory.java:187:
@Override
moved from FullScreenActivity#ChromeContextMenuPopulator

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabObserver.java
(right):

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabObserver.java:24:
class CustomTabObserver extends EmptyTabObserver {
pulled out of CustomTab#CustomTabObserver

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
File chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java (right):

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java:2058: private
TabWebContentsDelegateAndroid createWebContentsDelegate() {
nit: can you have a follow up CL that moves all of the factory methods to one
location?

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/tab/TabDelegateFactory.java
(right):

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/tab/TabDelegateFactory.java:1:
// Copyright 2014 The Chromium Authors. All rights reserved.
nit: I'VE GONE BACK IN TIME

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/tab/TabDelegateFactory.java:18:
/**
actually describe the functions, not just the params

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/FullscreenDelegateFactory.java
(right):

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/webapps/FullscreenDelegateFactory.java:33:
public class FullscreenDelegateFactory extends TabDelegateFactory {
FullScreen for consistency.

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/webapps/FullscreenDelegateFactory.java:41:
public void activateContents() {
moved from FullScreenActivity#FullScreenTabWebContentsDelegateAndroid

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/webapps/FullscreenDelegateFactory.java:62:
@Override
moved from FullScreenActivityTab#createWebContentsDelegate

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/webapps/FullscreenDelegateFactory.java:70:
return new ContextMenuPopulator() {
moved from FullScreenActivityTab#createContextMenuPopulator

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/webapps/FullscreenDelegateFactory.java:74:
// public ContextMenuPopulator()
may as well combine these two lines with the actual declarations

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/webapps/FullscreenDelegateFactory.java:81:
public boolean shouldShowContextMenu(ContextMenuParams params) {
This shouldn't diverge from ChromeContextMenuPopulator; I think the other one
kept getting additions to it but this remained stagnant.  Can a follow up CL/bug
be filed to combine all of these?  The bug listed below isn't a good one for
that.

[David Trainor- moved to gerrit, 2015-10-26 21:04:40]

a few nits.  but dan's comments covered most of it :).

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java
(right):

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java:272:
mTab.addObserver(mTabObserver);
Who removes the relevant observer(s) if/when tabs get swapped around?

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
File chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java (right):

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java:1298: private
AppBannerManager createAppBannerManager() {
If it's private, just get rid of the method?

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java:2058: private
TabWebContentsDelegateAndroid createWebContentsDelegate() {
On 2015/10/22 17:16:53, dfalcantara wrote:
> nit: can you have a follow up CL that moves all of the factory methods to one
> location?

Or just get rid of them.  No point in having a method for a one liner creation
call if it's not meant to be overridden.

[Yusuf, 2015-10-26 21:28:30]

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java
(right):

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java:237:
private void createTab() {
On 2015/10/22 17:16:53, dfalcantara wrote:
> Where does this actually get called from now?  I'm not seeing it anywhere

From finishNativeInitialization. Which has totally been there from patch 1.
There was a "glitch" in codereview I think....

Anywhere it is there now.

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java:272:
mTab.addObserver(mTabObserver);
On 2015/10/26 21:04:40, David Trainor wrote:
> Who removes the relevant observer(s) if/when tabs get swapped around?

Added to ondestroy for now. Once we add swapping, should be handled within the
same call that will do all swapping.

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabDelegateFactory.java
(right):

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabDelegateFactory.java:47:
private static final String TAG = "cr_customtabs";
On 2015/10/22 17:16:53, dfalcantara wrote:
> remove cr_; TAGs include it by default now

Done.

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabDelegateFactory.java:128:
private String mTargetUrl;
On 2015/10/22 17:16:53, dfalcantara wrote:
> nit: move above constructor

Done.

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
File chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java (right):

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java:1298: private
AppBannerManager createAppBannerManager() {
On 2015/10/26 21:04:40, David Trainor wrote:
> If it's private, just get rid of the method?

Done.

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java:2058: private
TabWebContentsDelegateAndroid createWebContentsDelegate() {
On 2015/10/26 21:04:40, David Trainor wrote:
> On 2015/10/22 17:16:53, dfalcantara wrote:
> > nit: can you have a follow up CL that moves all of the factory methods to
one
> > location?
> 
> Or just get rid of them.  No point in having a method for a one liner creation
> call if it's not meant to be overridden.

Removed!

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/tab/TabDelegateFactory.java
(right):

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/tab/TabDelegateFactory.java:1:
// Copyright 2014 The Chromium Authors. All rights reserved.
On 2015/10/22 17:16:53, dfalcantara wrote:
> nit: I'VE GONE BACK IN TIME

Done.

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/tab/TabDelegateFactory.java:18:
/**
On 2015/10/22 17:16:53, dfalcantara wrote:
> actually describe the functions, not just the params

Done.

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/FullscreenDelegateFactory.java
(right):

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/webapps/FullscreenDelegateFactory.java:74:
// public ContextMenuPopulator()
On 2015/10/22 17:16:53, dfalcantara wrote:
> may as well combine these two lines with the actual declarations

Done.

https://chromiumcodereview.appspot.com/1416093004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/webapps/FullscreenDelegateFactory.java:81:
public boolean shouldShowContextMenu(ContextMenuParams params) {
On 2015/10/22 17:16:53, dfalcantara wrote:
> This shouldn't diverge from ChromeContextMenuPopulator; I think the other one
> kept getting additions to it but this remained stagnant.  Can a follow up
CL/bug
> be filed to combine all of these?  The bug listed below isn't a good one for
> that.

Done.

[Yusuf, 2015-10-26 21:30:09]

The CQ bit was checked by yusufo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-26 21:33:21]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1416093004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1416093004/20001

[commit-bot: I haz the power, 2015-10-26 22:05:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-26 22:05:18]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[Yusuf, 2015-10-26 22:16:48]

The CQ bit was checked by yusufo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-26 22:17:50]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1416093004/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1416093004/40001

[commit-bot: I haz the power, 2015-10-26 23:58:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-26 23:58:17]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_android_r...)

[gone, 2015-10-27 17:40:00]

lgtm % dtrainor's comments

https://chromiumcodereview.appspot.com/1416093004/diff/20001/chrome/android/j...
File
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java
(right):

https://chromiumcodereview.appspot.com/1416093004/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java:315:
/* (non-Javadoc)
Eh, this should just be

/**
 * @see org.chromium.chrome.browser.ChromeActivity#onDestroyInternal()
 */

https://chromiumcodereview.appspot.com/1416093004/diff/20001/chrome/android/j...
File
chrome/android/java/src/org/chromium/chrome/browser/tab/TabDelegateFactory.java
(right):

https://chromiumcodereview.appspot.com/1416093004/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/tab/TabDelegateFactory.java:30:
* Creates the intercept navigation delegate the tab will be initialized with.
{@link InterceptNavigationDelegate}, same all throughout file

https://chromiumcodereview.appspot.com/1416093004/diff/40001/chrome/android/j...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/FullScreenDelegateFactory.java
(right):

https://chromiumcodereview.appspot.com/1416093004/diff/40001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/webapps/FullScreenDelegateFactory.java:30:
* A {@link TabDelegateFactory} class to be used in all {@link Tab} owned
nit: instances owned

[Yusuf, 2015-10-27 18:40:08]

https://chromiumcodereview.appspot.com/1416093004/diff/20001/chrome/android/j...
File
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java
(right):

https://chromiumcodereview.appspot.com/1416093004/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java:315:
/* (non-Javadoc)
On 2015/10/27 17:40:00, dfalcantara wrote:
> Eh, this should just be
> 
> /**
>  * @see org.chromium.chrome.browser.ChromeActivity#onDestroyInternal()
>  */

Done.

https://chromiumcodereview.appspot.com/1416093004/diff/20001/chrome/android/j...
File
chrome/android/java/src/org/chromium/chrome/browser/tab/TabDelegateFactory.java
(right):

https://chromiumcodereview.appspot.com/1416093004/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/tab/TabDelegateFactory.java:30:
* Creates the intercept navigation delegate the tab will be initialized with.
On 2015/10/27 17:40:00, dfalcantara wrote:
> {@link InterceptNavigationDelegate}, same all throughout file

Done.

https://chromiumcodereview.appspot.com/1416093004/diff/40001/chrome/android/j...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/FullScreenDelegateFactory.java
(right):

https://chromiumcodereview.appspot.com/1416093004/diff/40001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/webapps/FullScreenDelegateFactory.java:30:
* A {@link TabDelegateFactory} class to be used in all {@link Tab} owned
On 2015/10/27 17:40:00, dfalcantara wrote:
> nit: instances owned

Done.

[Yusuf, 2015-10-27 18:40:13]

The CQ bit was checked by yusufo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-27 18:41:47]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1416093004/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1416093004/60001

[commit-bot: I haz the power, 2015-10-27 20:21:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-27 20:21:56]

Dry run: This issue passed the CQ dry run.

[David Trainor- moved to gerrit, 2015-10-27 20:27:54]

lgtm

[Yusuf, 2015-10-27 20:28:10]

The CQ bit was checked by yusufo@chromium.org

[Yusuf, 2015-10-27 20:28:10]

The patchset sent to the CQ was uploaded after l-g-t-m from
dfalcantara@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/1416093004/#ps60001
(title: "Fixed tests and nits")

[commit-bot: I haz the power, 2015-10-27 20:29:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1416093004/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1416093004/60001

[commit-bot: I haz the power, 2015-10-27 20:36:18]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-10-27 20:37:54]

Patchset 4 (id:??) landed as
https://crrev.com/5dbc149bebecea186b693b3d780b6965eeffed0f
Cr-Commit-Position: refs/heads/master@{#356385}