Migration of code from ContentViewCore to NavigationController to avoid ContentViewCore abstraction…

Migration of code from ContentViewCore to NavigationController to avoid ContentViewCore abstractions.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=281695

[AKVT, 2014-07-02 05:13:54]

PTAL

[Yaron, 2014-07-02 17:32:40]

Thanks for the change.

In the future, please pick a minimal set of owners to cover the directories and
don't include all owners.

https://codereview.chromium.org/369503003/diff/1/content/browser/frame_host/n...
File content/browser/frame_host/navigation_controller_android.cc (right):

https://codereview.chromium.org/369503003/diff/1/content/browser/frame_host/n...
content/browser/frame_host/navigation_controller_android.cc:76: void
NavigationControllerAndroid::Reload(JNIEnv* env, jobject obj,
nit: all params on one line or separate lines for each param

https://codereview.chromium.org/369503003/diff/1/content/browser/frame_host/n...
content/browser/frame_host/navigation_controller_android.cc:82: jobject obj,
jboolean check_for_repost) {
same

https://codereview.chromium.org/369503003/diff/1/content/public/android/java/...
File
content/public/android/java/src/org/chromium/content/browser/framehost/NavigationControllerImpl.java
(right):

https://codereview.chromium.org/369503003/diff/1/content/public/android/java/...
content/public/android/java/src/org/chromium/content/browser/framehost/NavigationControllerImpl.java:84:
*/
Please remove function comments and replace them with @Override (for all new
functions)

https://codereview.chromium.org/369503003/diff/1/content/public/android/java/...
File
content/public/android/java/src/org/chromium/content_public/browser/NavigationController.java
(right):

https://codereview.chromium.org/369503003/diff/1/content/public/android/java/...
content/public/android/java/src/org/chromium/content_public/browser/NavigationController.java:54:
public void loadIfNecessary();
I think you should update the CVC equivalents to call these and remove the code
from ContentViewCoreImpl in the same changelist.

In a follow-up we can see about removing the Java CVC intermediary.

[AKVT, 2014-07-05 10:32:17]

@Yaron - I fixed the review comments. PTAL

https://codereview.chromium.org/369503003/diff/1/content/browser/frame_host/n...
File content/browser/frame_host/navigation_controller_android.cc (right):

https://codereview.chromium.org/369503003/diff/1/content/browser/frame_host/n...
content/browser/frame_host/navigation_controller_android.cc:76: void
NavigationControllerAndroid::Reload(JNIEnv* env, jobject obj,
On 2014/07/02 17:32:39, Yaron wrote:
> nit: all params on one line or separate lines for each param

Done.

https://codereview.chromium.org/369503003/diff/1/content/browser/frame_host/n...
content/browser/frame_host/navigation_controller_android.cc:82: jobject obj,
jboolean check_for_repost) {
On 2014/07/02 17:32:39, Yaron wrote:
> same

Done.

https://codereview.chromium.org/369503003/diff/1/content/public/android/java/...
File
content/public/android/java/src/org/chromium/content/browser/framehost/NavigationControllerImpl.java
(right):

https://codereview.chromium.org/369503003/diff/1/content/public/android/java/...
content/public/android/java/src/org/chromium/content/browser/framehost/NavigationControllerImpl.java:84:
*/
On 2014/07/02 17:32:39, Yaron wrote:
> Please remove function comments and replace them with @Override (for all new
> functions)

Done.

https://codereview.chromium.org/369503003/diff/1/content/public/android/java/...
File
content/public/android/java/src/org/chromium/content_public/browser/NavigationController.java
(right):

https://codereview.chromium.org/369503003/diff/1/content/public/android/java/...
content/public/android/java/src/org/chromium/content_public/browser/NavigationController.java:54:
public void loadIfNecessary();
On 2014/07/02 17:32:40, Yaron wrote:
> I think you should update the CVC equivalents to call these and remove the
code
> from ContentViewCoreImpl in the same changelist.
> 
> In a follow-up we can see about removing the Java CVC intermediary.

Done.

[AKVT, 2014-07-07 06:29:10]

PTAL

[Yaron, 2014-07-07 16:01:18]

lgtm

[AKVT, 2014-07-07 17:00:11]

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

[commit-bot: I haz the power, 2014-07-07 17:01:14]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ajith.v@samsung.com/369503003/40001

[commit-bot: I haz the power, 2014-07-07 18:51:04]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[AKVT, 2014-07-07 18:51:38]

@nasko PTAL

[commit-bot: I haz the power, 2014-07-07 18:56:54]

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

[commit-bot: I haz the power, 2014-07-07 18:56:56]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[AKVT, 2014-07-08 06:22:55]

On 2014/07/07 18:56:56, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on tryserver.chromium
>
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

@nasko PTAL build is failing due to OWNER lgtm for
content/browser/frame_host/navigation_controller_android.cc and
content/browser/frame_host/navigation_controller_android.h files.

[nasko, 2014-07-08 06:28:58]

LGTM with the style fixed.

https://codereview.chromium.org/369503003/diff/40001/content/browser/frame_ho...
File content/browser/frame_host/navigation_controller_android.cc (right):

https://codereview.chromium.org/369503003/diff/40001/content/browser/frame_ho...
content/browser/frame_host/navigation_controller_android.cc:82: void
NavigationControllerAndroid::ReloadIgnoringCache(JNIEnv* env,
style: all params should start on new line

[AKVT, 2014-07-08 06:38:41]

@nasko PTAL

https://codereview.chromium.org/369503003/diff/40001/content/browser/frame_ho...
File content/browser/frame_host/navigation_controller_android.cc (right):

https://codereview.chromium.org/369503003/diff/40001/content/browser/frame_ho...
content/browser/frame_host/navigation_controller_android.cc:82: void
NavigationControllerAndroid::ReloadIgnoringCache(JNIEnv* env,
On 2014/07/08 06:28:58, nasko wrote:
> style: all params should start on new line

Done.

[nasko, 2014-07-08 06:39:24]

On 2014/07/08 06:38:41, AKV wrote:
> @nasko PTAL
> 
>
https://codereview.chromium.org/369503003/diff/40001/content/browser/frame_ho...
> File content/browser/frame_host/navigation_controller_android.cc (right):
> 
>
https://codereview.chromium.org/369503003/diff/40001/content/browser/frame_ho...
> content/browser/frame_host/navigation_controller_android.cc:82: void
> NavigationControllerAndroid::ReloadIgnoringCache(JNIEnv* env,
> On 2014/07/08 06:28:58, nasko wrote:
> > style: all params should start on new line
> 
> Done.

Still LGTM

[AKVT, 2014-07-08 06:40:07]

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

[commit-bot: I haz the power, 2014-07-08 06:40:50]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ajith.v@samsung.com/369503003/60001

[commit-bot: I haz the power, 2014-07-08 08:23:33]

Change committed as 281695