Removing NavigationClient dependencies from Tab.

Removing NavigationClient dependencies from Tab.

NavigationClient is a thin interface, which is replaced by
NavigationController and hence this thin layer is not required
in the Chrome layer.

BUG=398263
Committed: https://crrev.com/8d7a45820e23547fd3100221762e8fc177363aa8
Cr-Commit-Position: refs/heads/master@{#300915}

[AKVT, 2014-10-22 14:26:26]

ajith.v@samsung.com changed reviewers:
+ tedchoc@chromium.org, yfriedman@chromium.org

[AKVT, 2014-10-22 14:26:26]

@Yaron & Ted PTAL

[Ted C, 2014-10-22 17:41:34]

https://codereview.chromium.org/666673009/diff/40001/chrome/android/java/src/...
File chrome/android/java/src/org/chromium/chrome/browser/NavigationPopup.java
(right):

https://codereview.chromium.org/666673009/diff/40001/chrome/android/java/src/...
chrome/android/java/src/org/chromium/chrome/browser/NavigationPopup.java:57: *
@param navigationController The owner of the history being displayed.
I would update this comment

https://codereview.chromium.org/666673009/diff/40001/chrome/android/java/src/...
File chrome/android/java/src/org/chromium/chrome/browser/Tab.java (right):

https://codereview.chromium.org/666673009/diff/40001/chrome/android/java/src/...
chrome/android/java/src/org/chromium/chrome/browser/Tab.java:75: public class
Tab implements NavigationController {
We shouldn't be doing this.

Some of these methods are here for convenience, but we don't want to add all of
the rest.

It is find to just remove NavigationClient from Tab and leave the methods as
they were.

https://codereview.chromium.org/666673009/diff/40001/chrome/android/javatests...
File
chrome/android/javatests/src/org/chromium/chrome/browser/NavigationPopupTest.java
(right):

https://codereview.chromium.org/666673009/diff/40001/chrome/android/javatests...
chrome/android/javatests/src/org/chromium/chrome/browser/NavigationPopupTest.java:58:
private static class TestNavigationClient implements NavigationController {
TestNavigationController now

[Yaron, 2014-10-22 17:54:05]

https://codereview.chromium.org/666673009/diff/40001/chrome/android/java/src/...
File chrome/android/java/src/org/chromium/chrome/browser/Tab.java (right):

https://codereview.chromium.org/666673009/diff/40001/chrome/android/java/src/...
chrome/android/java/src/org/chromium/chrome/browser/Tab.java:75: public class
Tab implements NavigationController {
On 2014/10/22 17:41:33, Ted C wrote:
> We shouldn't be doing this.
> 
> Some of these methods are here for convenience, but we don't want to add all
of
> the rest.
> 
> It is find to just remove NavigationClient from Tab and leave the methods as
> they were.

Ya, I definitely agree with Ted here

[AKVT, 2014-10-23 13:34:27]

@Yaron & Ted PTAL

https://codereview.chromium.org/666673009/diff/40001/chrome/android/java/src/...
File chrome/android/java/src/org/chromium/chrome/browser/NavigationPopup.java
(right):

https://codereview.chromium.org/666673009/diff/40001/chrome/android/java/src/...
chrome/android/java/src/org/chromium/chrome/browser/NavigationPopup.java:57: *
@param navigationController The owner of the history being displayed.
On 2014/10/22 17:41:33, Ted C wrote:
> I would update this comment

Done.

https://codereview.chromium.org/666673009/diff/40001/chrome/android/java/src/...
File chrome/android/java/src/org/chromium/chrome/browser/Tab.java (right):

https://codereview.chromium.org/666673009/diff/40001/chrome/android/java/src/...
chrome/android/java/src/org/chromium/chrome/browser/Tab.java:75: public class
Tab implements NavigationController {
On 2014/10/22 17:41:33, Ted C wrote:
> We shouldn't be doing this.
> 
> Some of these methods are here for convenience, but we don't want to add all
of
> the rest.
> 
> It is find to just remove NavigationClient from Tab and leave the methods as
> they were.

Done. Thank you

https://codereview.chromium.org/666673009/diff/40001/chrome/android/java/src/...
chrome/android/java/src/org/chromium/chrome/browser/Tab.java:75: public class
Tab implements NavigationController {
On 2014/10/22 17:41:33, Ted C wrote:
> We shouldn't be doing this.
> 
> Some of these methods are here for convenience, but we don't want to add all
of
> the rest.
> 
> It is find to just remove NavigationClient from Tab and leave the methods as
> they were.

Done. Thank you

https://codereview.chromium.org/666673009/diff/40001/chrome/android/javatests...
File
chrome/android/javatests/src/org/chromium/chrome/browser/NavigationPopupTest.java
(right):

https://codereview.chromium.org/666673009/diff/40001/chrome/android/javatests...
chrome/android/javatests/src/org/chromium/chrome/browser/NavigationPopupTest.java:58:
private static class TestNavigationClient implements NavigationController {
On 2014/10/22 17:41:34, Ted C wrote:
> TestNavigationController now

Done.

[Ted C, 2014-10-23 16:23:27]

https://codereview.chromium.org/666673009/diff/60001/chrome/android/java/src/...
File chrome/android/java/src/org/chromium/chrome/browser/Tab.java (right):

https://codereview.chromium.org/666673009/diff/60001/chrome/android/java/src/...
chrome/android/java/src/org/chromium/chrome/browser/Tab.java:411: public
NavigationHistory getDirectedNavigationHistory(boolean isForward, int itemLimit)
{
I would go ahead and remove these two methods.  They were only used for the
navigation popup anyway.

https://codereview.chromium.org/666673009/diff/60001/chrome/android/javatests...
File
chrome/android/javatests/src/org/chromium/chrome/browser/NavigationPopupTest.java
(right):

https://codereview.chromium.org/666673009/diff/60001/chrome/android/javatests...
chrome/android/javatests/src/org/chromium/chrome/browser/NavigationPopupTest.java:133:

remove extra blank line

[AKVT, 2014-10-23 16:39:25]

@Ted & Yaron PTAL

https://codereview.chromium.org/666673009/diff/60001/chrome/android/java/src/...
File chrome/android/java/src/org/chromium/chrome/browser/Tab.java (right):

https://codereview.chromium.org/666673009/diff/60001/chrome/android/java/src/...
chrome/android/java/src/org/chromium/chrome/browser/Tab.java:411: public
NavigationHistory getDirectedNavigationHistory(boolean isForward, int itemLimit)
{
On 2014/10/23 16:23:27, Ted C wrote:
> I would go ahead and remove these two methods.  They were only used for the
> navigation popup anyway.

Done.

https://codereview.chromium.org/666673009/diff/60001/chrome/android/javatests...
File
chrome/android/javatests/src/org/chromium/chrome/browser/NavigationPopupTest.java
(right):

https://codereview.chromium.org/666673009/diff/60001/chrome/android/javatests...
chrome/android/javatests/src/org/chromium/chrome/browser/NavigationPopupTest.java:133:

On 2014/10/23 16:23:27, Ted C wrote:
> remove extra blank line

Done.

[Ted C, 2014-10-23 17:04:22]

lgtm

[AKVT, 2014-10-23 17:10:15]

The CQ bit was checked by ajith.v@samsung.com

[commit-bot: I haz the power, 2014-10-23 17:10:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/666673009/80001

[commit-bot: I haz the power, 2014-10-23 17:56:37]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2014-10-23 17:57:06]

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