Removing ContentViewCore dependencies from few functions which acts as direct wrapper to WebContents

Removing ContentViewCore dependencies from few functions which acts as direct wrapper to WebContents

It's a follow up patch to https://codereview.chromium.org/414423002/. We already restructured WebContents
functionalities from ContentViewCore. In this patch we are removing unwanted ContentViewCore methods,
so that ContentViewCore stake holders can directly call these methods.

BUG=398263
Committed: https://crrev.com/7f5d0b808652fe088a46631948d3deea164fd677
Cr-Commit-Position: refs/heads/master@{#294746}

[AKVT, 2014-08-18 14:34:50]

@Yaron, boliu and avi PTAL this change.

[Avi (use Gerrit), 2014-08-18 14:52:51]

On 2014/08/18 14:34:50, AKV wrote:
> @Yaron, boliu and avi PTAL this change.

I don't know the guts of the Java side of things, so I can't comment
intelligently. I will stamp once the experts in the area approve.

[boliu, 2014-08-18 15:38:15]

https://codereview.chromium.org/481803004/diff/1/android_webview/java/src/org...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/481803004/diff/1/android_webview/java/src/org...
android_webview/java/src/org/chromium/android_webview/AwContents.java:1072:
params.getUrl().equals(mContentViewCore.getWebContents().getUrl()) &&
Add a mWebContents member in AwContents so we don't have to call
mContentViewCore.getWebContents() all over the place in this class.

https://codereview.chromium.org/481803004/diff/1/android_webview/javatests/sr...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java
(right):

https://codereview.chromium.org/481803004/diff/1/android_webview/javatests/sr...
android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java:358:
return awContents.getContentViewCore().getWebContents().getTitle();
can this just be awContents.getTitle()?

https://codereview.chromium.org/481803004/diff/1/android_webview/javatests/sr...
File
android_webview/javatests/src/org/chromium/android_webview/test/NavigationHistoryTest.java
(right):

https://codereview.chromium.org/481803004/diff/1/android_webview/javatests/sr...
android_webview/javatests/src/org/chromium/android_webview/test/NavigationHistoryTest.java:291:
String title = mAwContents.getContentViewCore().getWebContents().getTitle();
Same comment in this file

https://codereview.chromium.org/481803004/diff/1/android_webview/javatests/sr...
File
android_webview/javatests/src/org/chromium/android_webview/test/SaveRestoreStateTest.java
(right):

https://codereview.chromium.org/481803004/diff/1/android_webview/javatests/sr...
android_webview/javatests/src/org/chromium/android_webview/test/SaveRestoreStateTest.java:139:
return
TITLES[0].equals(restoredVars.contentViewCore.getWebContents().getTitle()) &&
same comment

https://codereview.chromium.org/481803004/diff/1/android_webview/javatests/sr...
File
android_webview/javatests/src/org/chromium/android_webview/test/WebKitHitTestTest.java
(right):

https://codereview.chromium.org/481803004/diff/1/android_webview/javatests/sr...
android_webview/javatests/src/org/chromium/android_webview/test/WebKitHitTestTest.java:403:
return
mAwContents.getContentViewCore().getWebContents().getTitle().equals(title);
same comment

https://codereview.chromium.org/481803004/diff/1/content/public/android/java/...
File
content/public/android/java/src/org/chromium/content/browser/accessibility/AccessibilityInjector.java
(right):

https://codereview.chromium.org/481803004/diff/1/content/public/android/java/...
content/public/android/java/src/org/chromium/content/browser/accessibility/AccessibilityInjector.java:322:
ex.printStackTrace();
Remove these new prints please. They are not part of this refactor.

[AKVT, 2014-08-18 17:03:03]

Thanks boliu. I am fixing the comments. PTAL my inline comments about
ex.printStacktrace() and share your opinion.

https://codereview.chromium.org/481803004/diff/1/android_webview/java/src/org...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/481803004/diff/1/android_webview/java/src/org...
android_webview/java/src/org/chromium/android_webview/AwContents.java:1072:
params.getUrl().equals(mContentViewCore.getWebContents().getUrl()) &&
On 2014/08/18 15:38:15, boliu wrote:
> Add a mWebContents member in AwContents so we don't have to call
> mContentViewCore.getWebContents() all over the place in this class.

Done.

https://codereview.chromium.org/481803004/diff/1/android_webview/javatests/sr...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java
(right):

https://codereview.chromium.org/481803004/diff/1/android_webview/javatests/sr...
android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java:358:
return awContents.getContentViewCore().getWebContents().getTitle();
On 2014/08/18 15:38:15, boliu wrote:
> can this just be awContents.getTitle()?

Done. Thanks

https://codereview.chromium.org/481803004/diff/1/android_webview/javatests/sr...
File
android_webview/javatests/src/org/chromium/android_webview/test/NavigationHistoryTest.java
(right):

https://codereview.chromium.org/481803004/diff/1/android_webview/javatests/sr...
android_webview/javatests/src/org/chromium/android_webview/test/NavigationHistoryTest.java:291:
String title = mAwContents.getContentViewCore().getWebContents().getTitle();
On 2014/08/18 15:38:15, boliu wrote:
> Same comment in this file

Done.

https://codereview.chromium.org/481803004/diff/1/android_webview/javatests/sr...
File
android_webview/javatests/src/org/chromium/android_webview/test/SaveRestoreStateTest.java
(right):

https://codereview.chromium.org/481803004/diff/1/android_webview/javatests/sr...
android_webview/javatests/src/org/chromium/android_webview/test/SaveRestoreStateTest.java:139:
return
TITLES[0].equals(restoredVars.contentViewCore.getWebContents().getTitle()) &&
On 2014/08/18 15:38:15, boliu wrote:
> same comment

Done.

https://codereview.chromium.org/481803004/diff/1/android_webview/javatests/sr...
File
android_webview/javatests/src/org/chromium/android_webview/test/WebKitHitTestTest.java
(right):

https://codereview.chromium.org/481803004/diff/1/android_webview/javatests/sr...
android_webview/javatests/src/org/chromium/android_webview/test/WebKitHitTestTest.java:403:
return
mAwContents.getContentViewCore().getWebContents().getTitle().equals(title);
On 2014/08/18 15:38:15, boliu wrote:
> same comment

Done.

https://codereview.chromium.org/481803004/diff/1/content/public/android/java/...
File
content/public/android/java/src/org/chromium/content/browser/accessibility/AccessibilityInjector.java
(right):

https://codereview.chromium.org/481803004/diff/1/content/public/android/java/...
content/public/android/java/src/org/chromium/content/browser/accessibility/AccessibilityInjector.java:322:
ex.printStackTrace();
On 2014/08/18 15:38:15, boliu wrote:
> Remove these new prints please. They are not part of this refactor.
These things I added due to presubmit warnings. I am OK to remove it. Please
suggest.

[boliu, 2014-08-18 17:08:47]

https://codereview.chromium.org/481803004/diff/1/content/public/android/java/...
File
content/public/android/java/src/org/chromium/content/browser/accessibility/AccessibilityInjector.java
(right):

https://codereview.chromium.org/481803004/diff/1/content/public/android/java/...
content/public/android/java/src/org/chromium/content/browser/accessibility/AccessibilityInjector.java:322:
ex.printStackTrace();
On 2014/08/18 17:03:03, AKV wrote:
> On 2014/08/18 15:38:15, boliu wrote:
> > Remove these new prints please. They are not part of this refactor.
> These things I added due to presubmit warnings. I am OK to remove it. Please
> suggest.
> 

The warning must have been added *after* this code was written then. I think you
can get pass it by just adding a comment like

// Intentional no-op.

Although I don't know this code well enough if these really are intentional
no-ops. Looks to me the try block is doing way too much.

[AKVT, 2014-08-18 17:21:11]

@boliu and Yaron PTAL

[boliu, 2014-08-18 17:37:27]

android_webview lgtm

https://codereview.chromium.org/481803004/diff/20001/android_webview/java/src...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/481803004/diff/20001/android_webview/java/src...
android_webview/java/src/org/chromium/android_webview/AwContents.java:779:
mWebContentsObserver = new AwWebContentsObserver(mWebContents,
nit: this can fit on one line now

[AKVT, 2014-08-18 18:22:03]

Thanks boliu. @Yaron and Avi PTAL.

https://codereview.chromium.org/481803004/diff/20001/android_webview/java/src...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/481803004/diff/20001/android_webview/java/src...
android_webview/java/src/org/chromium/android_webview/AwContents.java:779:
mWebContentsObserver = new AwWebContentsObserver(mWebContents,
On 2014/08/18 17:37:27, boliu wrote:
> nit: this can fit on one line now

Done.

[Avi (use Gerrit), 2014-08-18 19:17:38]

You have LGTM from me once you have all the Android experts happy.

[Yaron, 2014-08-18 23:16:24]

https://codereview.chromium.org/481803004/diff/40001/chrome/android/shell/jav...
File chrome/android/shell/java/src/org/chromium/chrome/shell/ChromeShellTab.java
(right):

https://codereview.chromium.org/481803004/diff/40001/chrome/android/shell/jav...
chrome/android/shell/java/src/org/chromium/chrome/shell/ChromeShellTab.java:65:
ContentViewCore contentViewCore = getContentViewCore();
We should remove the CVC dependency entirely when
https://codereview.chromium.org/406023002/ lands. You could use just WebContents
and NavigationController.

https://codereview.chromium.org/481803004/diff/40001/chrome/android/shell/jav...
File
chrome/android/shell/java/src/org/chromium/chrome/shell/ChromeShellToolbar.java
(right):

https://codereview.chromium.org/481803004/diff/40001/chrome/android/shell/jav...
chrome/android/shell/java/src/org/chromium/chrome/shell/ChromeShellToolbar.java:80:
mUrlTextView.setText(mTab.getContentViewCore().getWebContents().getUrl());
Tab exposes getWebContents. Please use directly (throughout this CL)

https://codereview.chromium.org/481803004/diff/40001/content/public/android/j...
File
content/public/android/java/src/org/chromium/content/browser/accessibility/AccessibilityInjector.java
(right):

https://codereview.chromium.org/481803004/diff/40001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/accessibility/AccessibilityInjector.java:308:
if (mContentViewCore.getWebContents().getUrl() == null) {
So this is probably a few steps down the road (i.e. a few blocking issues in the
way) but I'm curious whether we'd be able to have some of these classes only
depend on WebContents and lower layers. What would be preventing us from doing
that? It seems like we need the Context and the ContainerView, the latter is
supposed to be available from webcontents based on web_contents.h
(GetNativeView).

https://codereview.chromium.org/481803004/diff/40001/content/public/test/andr...
File
content/public/test/android/javatests/src/org/chromium/content/browser/test/util/HistoryUtils.java
(right):

https://codereview.chromium.org/481803004/diff/40001/content/public/test/andr...
content/public/test/android/javatests/src/org/chromium/content/browser/test/util/HistoryUtils.java:109:
public static String getUrlOnUiThread(Instrumentation instrumentation,
This class could also have the dependency removed when your other CL lands.
Maybe finish that one first?

[AKVT, 2014-08-19 14:25:47]

On 2014/08/18 23:16:24, Yaron wrote:
>
https://codereview.chromium.org/481803004/diff/40001/chrome/android/shell/jav...
> File
chrome/android/shell/java/src/org/chromium/chrome/shell/ChromeShellTab.java
> (right):
> 
>
https://codereview.chromium.org/481803004/diff/40001/chrome/android/shell/jav...
>
chrome/android/shell/java/src/org/chromium/chrome/shell/ChromeShellTab.java:65:
> ContentViewCore contentViewCore = getContentViewCore();
> We should remove the CVC dependency entirely when
> https://codereview.chromium.org/406023002/ lands. You could use just
WebContents
> and NavigationController.
> 
>
https://codereview.chromium.org/481803004/diff/40001/chrome/android/shell/jav...
> File
>
chrome/android/shell/java/src/org/chromium/chrome/shell/ChromeShellToolbar.java
> (right):
> 
>
https://codereview.chromium.org/481803004/diff/40001/chrome/android/shell/jav...
>
chrome/android/shell/java/src/org/chromium/chrome/shell/ChromeShellToolbar.java:80:
> mUrlTextView.setText(mTab.getContentViewCore().getWebContents().getUrl());
> Tab exposes getWebContents. Please use directly (throughout this CL)
> 
>
https://codereview.chromium.org/481803004/diff/40001/content/public/android/j...
> File
>
content/public/android/java/src/org/chromium/content/browser/accessibility/AccessibilityInjector.java
> (right):
> 
>
https://codereview.chromium.org/481803004/diff/40001/content/public/android/j...
>
content/public/android/java/src/org/chromium/content/browser/accessibility/AccessibilityInjector.java:308:
> if (mContentViewCore.getWebContents().getUrl() == null) {
> So this is probably a few steps down the road (i.e. a few blocking issues in
the
> way) but I'm curious whether we'd be able to have some of these classes only
> depend on WebContents and lower layers. What would be preventing us from doing
> that? It seems like we need the Context and the ContainerView, the latter is
> supposed to be available from webcontents based on web_contents.h
> (GetNativeView).
> 
>
https://codereview.chromium.org/481803004/diff/40001/content/public/test/andr...
> File
>
content/public/test/android/javatests/src/org/chromium/content/browser/test/util/HistoryUtils.java
> (right):
> 
>
https://codereview.chromium.org/481803004/diff/40001/content/public/test/andr...
>
content/public/test/android/javatests/src/org/chromium/content/browser/test/util/HistoryUtils.java:109:
> public static String getUrlOnUiThread(Instrumentation instrumentation,
> This class could also have the dependency removed when your other CL lands.
> Maybe finish that one first?

@Yaron - Thanks, I am in progress of uploading the corrected patch for
https://codereview.chromium.org/406023002/. But due to aosp integrated build
issues, I am stucked a bit. Once I resolve it, I will upload it.

[AKVT, 2014-09-11 17:19:42]

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

[AKVT, 2014-09-11 17:19:43]

@Yaron, boliu, Avi, tedchoc PTAL.

https://codereview.chromium.org/481803004/diff/40001/chrome/android/shell/jav...
File chrome/android/shell/java/src/org/chromium/chrome/shell/ChromeShellTab.java
(right):

https://codereview.chromium.org/481803004/diff/40001/chrome/android/shell/jav...
chrome/android/shell/java/src/org/chromium/chrome/shell/ChromeShellTab.java:65:
ContentViewCore contentViewCore = getContentViewCore();
On 2014/08/18 23:16:24, Yaron wrote:
> We should remove the CVC dependency entirely when
> https://codereview.chromium.org/406023002/ lands. You could use just
WebContents
> and NavigationController.

Done. Thanks

https://codereview.chromium.org/481803004/diff/40001/chrome/android/shell/jav...
File
chrome/android/shell/java/src/org/chromium/chrome/shell/ChromeShellToolbar.java
(right):

https://codereview.chromium.org/481803004/diff/40001/chrome/android/shell/jav...
chrome/android/shell/java/src/org/chromium/chrome/shell/ChromeShellToolbar.java:80:
mUrlTextView.setText(mTab.getContentViewCore().getWebContents().getUrl());
On 2014/08/18 23:16:24, Yaron wrote:
> Tab exposes getWebContents. Please use directly (throughout this CL)

Done.

https://codereview.chromium.org/481803004/diff/40001/content/public/android/j...
File
content/public/android/java/src/org/chromium/content/browser/accessibility/AccessibilityInjector.java
(right):

https://codereview.chromium.org/481803004/diff/40001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/accessibility/AccessibilityInjector.java:308:
if (mContentViewCore.getWebContents().getUrl() == null) {
On 2014/08/18 23:16:24, Yaron wrote:
> So this is probably a few steps down the road (i.e. a few blocking issues in
the
> way) but I'm curious whether we'd be able to have some of these classes only
> depend on WebContents and lower layers. What would be preventing us from doing
> that? It seems like we need the Context and the ContainerView, the latter is
> supposed to be available from webcontents based on web_contents.h
> (GetNativeView). 

@Yaron, Can we do this with a follow up patch ? as it needs more changes

https://codereview.chromium.org/481803004/diff/40001/content/public/test/andr...
File
content/public/test/android/javatests/src/org/chromium/content/browser/test/util/HistoryUtils.java
(right):

https://codereview.chromium.org/481803004/diff/40001/content/public/test/andr...
content/public/test/android/javatests/src/org/chromium/content/browser/test/util/HistoryUtils.java:109:
public static String getUrlOnUiThread(Instrumentation instrumentation,
On 2014/08/18 23:16:24, Yaron_OOO_until_9-22 wrote:
> This class could also have the dependency removed when your other CL lands.
> Maybe finish that one first?

Done. Thanks

[AKVT, 2014-09-11 17:54:12]

PTAL PS6.

[Avi (use Gerrit), 2014-09-11 18:06:45]

On 2014/09/11 17:54:12, AKV wrote:
> PTAL PS6.

I am not a Java person, and I don't know how anything works on there. Again, you
may rely on my LGTM once all the Android experts are happy.

[AKVT, 2014-09-11 18:22:18]

On 2014/09/11 18:06:45, Avi wrote:
> On 2014/09/11 17:54:12, AKV wrote:
> > PTAL PS6.
> 
> I am not a Java person, and I don't know how anything works on there. Again,
you
> may rely on my LGTM once all the Android experts are happy.

Thanks Avi. @Ted Please look into it as Yaron is OOO for few days.

[Ted C, 2014-09-12 00:40:37]

A few style nits.  I need to write up a downstream change to clean up the
references to these calls, but I'll try to do that first thing tomorrow and I'll
send this through the commit queue.

By the way, thanks for this awesome cleanup!

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

https://codereview.chromium.org/481803004/diff/100001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/Tab.java:403: if
(getWebContents() != null) getWebContents().getNavigationController().
The statement and condition all need to fit on a single line, so add braces
here.

Also when wrapping at the '.', put it on the start of the next line.  Typically,
it is just the assignment operators that wrap on the previous line (=).

This comment applies to a few places in this file.

https://codereview.chromium.org/481803004/diff/100001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/test/ModalDialogTest.java
(right):

https://codereview.chromium.org/481803004/diff/100001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/test/ModalDialogTest.java:187:
assertEquals(BEFORE_UNLOAD_URL, getActivity().getActiveContentViewCore().
same dot wrapping comment here

https://codereview.chromium.org/481803004/diff/100001/chrome/android/shell/ja...
File
chrome/android/shell/javatests/src/org/chromium/chrome/shell/ChromeShellTestBase.java
(right):

https://codereview.chromium.org/481803004/diff/100001/chrome/android/shell/ja...
chrome/android/shell/javatests/src/org/chromium/chrome/shell/ChromeShellTestBase.java:99:
getWebContents().getUrl()));
here too

https://codereview.chromium.org/481803004/diff/100001/chrome/android/shell/ja...
File
chrome/android/shell/javatests/src/org/chromium/chrome/shell/ChromeShellUrlTest.java
(right):

https://codereview.chromium.org/481803004/diff/100001/chrome/android/shell/ja...
chrome/android/shell/javatests/src/org/chromium/chrome/shell/ChromeShellUrlTest.java:67:
isShowingInterstitialPage.set(contentViewCore.get().getWebContents().
same :-)

I'll just say look at all following files for this situation again

[AKVT, 2014-09-12 14:37:11]

@Ted PTAL. I corrected line breaks.

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

https://codereview.chromium.org/481803004/diff/100001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/Tab.java:403: if
(getWebContents() != null) getWebContents().getNavigationController().
On 2014/09/12 00:40:36, Ted C wrote:
> The statement and condition all need to fit on a single line, so add braces
> here.
> 
> Also when wrapping at the '.', put it on the start of the next line. 
Typically,
> it is just the assignment operators that wrap on the previous line (=).
> 
> This comment applies to a few places in this file.

Done. Thanks

https://codereview.chromium.org/481803004/diff/100001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/test/ModalDialogTest.java
(right):

https://codereview.chromium.org/481803004/diff/100001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/test/ModalDialogTest.java:187:
assertEquals(BEFORE_UNLOAD_URL, getActivity().getActiveContentViewCore().
On 2014/09/12 00:40:36, Ted C wrote:
> same dot wrapping comment here

Done.

https://codereview.chromium.org/481803004/diff/100001/chrome/android/shell/ja...
File
chrome/android/shell/javatests/src/org/chromium/chrome/shell/ChromeShellTestBase.java
(right):

https://codereview.chromium.org/481803004/diff/100001/chrome/android/shell/ja...
chrome/android/shell/javatests/src/org/chromium/chrome/shell/ChromeShellTestBase.java:99:
getWebContents().getUrl()));
On 2014/09/12 00:40:36, Ted C wrote:
> here too

Done.

https://codereview.chromium.org/481803004/diff/100001/chrome/android/shell/ja...
File
chrome/android/shell/javatests/src/org/chromium/chrome/shell/ChromeShellUrlTest.java
(right):

https://codereview.chromium.org/481803004/diff/100001/chrome/android/shell/ja...
chrome/android/shell/javatests/src/org/chromium/chrome/shell/ChromeShellUrlTest.java:67:
isShowingInterstitialPage.set(contentViewCore.get().getWebContents().
On 2014/09/12 00:40:36, Ted C wrote:
> same :-)
> 
> I'll just say look at all following files for this situation again

Done.

[Ted C, 2014-09-12 16:06:19]

lgtm

[Ted C, 2014-09-12 16:06:25]

The CQ bit was checked by tedchoc@chromium.org

[commit-bot: I haz the power, 2014-09-12 16:08:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/481803004/120001

[boliu, 2014-09-12 16:10:06]

The CQ bit was unchecked by boliu@chromium.org

[boliu, 2014-09-12 16:10:43]

On 2014/09/12 16:08:52, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patchset/481803004/120001

Unchecked cq.

I still haven't had time to look over the changes in android_webview

[boliu, 2014-09-12 17:13:47]

On 2014/09/12 16:10:43, boliu wrote:
> On 2014/09/12 16:08:52, I haz the power (commit-bot) wrote:
> > CQ is trying da patch. Follow status at
> >  https://chromium-cq-status.appspot.com/patchset/481803004/120001
> 
> Unchecked cq.
> 
> I still haven't had time to look over the changes in android_webview

Heh, I was checking initialization order, then noticed 90% of android webview
instrumentation tests failed with "[FATAL:web_contents_observer_android.cc(44)]
Check failed: web_contents."

[AKVT, 2014-09-12 19:50:44]

@boliu, PTAL, I fixed the crashes.

[boliu, 2014-09-13 00:38:54]

lgtm after a bunch of clean ups

https://codereview.chromium.org/481803004/diff/160001/android_webview/java/sr...
File
android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java
(right):

https://codereview.chromium.org/481803004/diff/160001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java:138:
public void showRepostFormWarningDialog(final ContentViewCore contentViewCore) {
Content should pass up a NavigationController instead. Should clean this up in a
follow up.

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwSettingsTest.java
(right):

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/AwSettingsTest.java:1678:
final WebContents webContents =
testContainerView.getContentViewCore().getWebContents();
awContents.getWebContents()

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java
(right):

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java:262:
awContents.getContentViewCore().getWebContents().getNavigationController()
awContents.getNativationController

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/ClearHistoryTest.java
(right):

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/ClearHistoryTest.java:39:
final WebContents webContents =
testContainerView.getContentViewCore().getWebContents();
awContents.getWebContents

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/LoadDataWithBaseUrlTest.java
(right):

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/LoadDataWithBaseUrlTest.java:41:
mWebContents = testContainerView.getContentViewCore().getWebContents();
ditto

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/LoadUrlTest.java
(right):

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/LoadUrlTest.java:326:
awContents.getContentViewCore().getWebContents(),
ditto

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/NavigationHistoryTest.java
(right):

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/NavigationHistoryTest.java:174:
HistoryUtils.goBackSync(getInstrumentation(), mAwContents.getContentViewCore()
ditto

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/NavigationHistoryTest.java:308:
HistoryUtils.goBackSync(getInstrumentation(), mAwContents.getContentViewCore()
ditto

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/SaveRestoreStateTest.java
(right):

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/SaveRestoreStateTest.java:29:
public final WebContents webContents;
Not needed. Remove this and the import

[AKVT, 2014-09-13 11:30:50]

https://codereview.chromium.org/481803004/diff/160001/android_webview/java/sr...
File
android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java
(right):

https://codereview.chromium.org/481803004/diff/160001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java:138:
public void showRepostFormWarningDialog(final ContentViewCore contentViewCore) {
On 2014/09/13 00:38:53, boliu wrote:
> Content should pass up a NavigationController instead. Should clean this up in
a
> follow up.

@boliu. I will correct in follow up patch.

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwSettingsTest.java
(right):

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/AwSettingsTest.java:1678:
final WebContents webContents =
testContainerView.getContentViewCore().getWebContents();
On 2014/09/13 00:38:53, boliu wrote:
> awContents.getWebContents()

Done. Thanks

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java
(right):

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java:262:
awContents.getContentViewCore().getWebContents().getNavigationController()
On 2014/09/13 00:38:53, boliu wrote:
> awContents.getNativationController

Done.

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/ClearHistoryTest.java
(right):

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/ClearHistoryTest.java:39:
final WebContents webContents =
testContainerView.getContentViewCore().getWebContents();
On 2014/09/13 00:38:53, boliu wrote:
> awContents.getWebContents

Done.

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/LoadDataWithBaseUrlTest.java
(right):

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/LoadDataWithBaseUrlTest.java:41:
mWebContents = testContainerView.getContentViewCore().getWebContents();
On 2014/09/13 00:38:53, boliu wrote:
> ditto

Done.

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/LoadUrlTest.java
(right):

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/LoadUrlTest.java:326:
awContents.getContentViewCore().getWebContents(),
On 2014/09/13 00:38:53, boliu wrote:
> ditto

Done.

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/NavigationHistoryTest.java
(right):

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/NavigationHistoryTest.java:174:
HistoryUtils.goBackSync(getInstrumentation(), mAwContents.getContentViewCore()
On 2014/09/13 00:38:54, boliu wrote:
> ditto

Done.

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/NavigationHistoryTest.java:308:
HistoryUtils.goBackSync(getInstrumentation(), mAwContents.getContentViewCore()
On 2014/09/13 00:38:53, boliu wrote:
> ditto

Done.

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/SaveRestoreStateTest.java
(right):

https://codereview.chromium.org/481803004/diff/160001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/SaveRestoreStateTest.java:29:
public final WebContents webContents;
On 2014/09/13 00:38:54, boliu wrote:
> Not needed. Remove this and the import

Done. Thanks

[AKVT, 2014-09-13 12:45:29]

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

[commit-bot: I haz the power, 2014-09-13 12:46:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/481803004/180001

[commit-bot: I haz the power, 2014-09-13 13:28:17]

Committed patchset #10 (id:180001) as 3cd082f82afd6a4d30c35f26f65d49d682b2bb16

[commit-bot: I haz the power, 2014-09-13 13:32:25]

Patchset 10 (id:??) landed as
https://crrev.com/7f5d0b808652fe088a46631948d3deea164fd677
Cr-Commit-Position: refs/heads/master@{#294746}

[AKVT, 2014-10-13 12:25:05]

@Yaron/Ted Please respond to my inline queries

https://codereview.chromium.org/481803004/diff/40001/content/public/android/j...
File
content/public/android/java/src/org/chromium/content/browser/accessibility/AccessibilityInjector.java
(right):

https://codereview.chromium.org/481803004/diff/40001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/accessibility/AccessibilityInjector.java:308:
if (mContentViewCore.getWebContents().getUrl() == null) {
On 2014/09/11 17:19:43, AKV wrote:
> On 2014/08/18 23:16:24, Yaron wrote:
> > So this is probably a few steps down the road (i.e. a few blocking issues in
> the
> > way) but I'm curious whether we'd be able to have some of these classes only
> > depend on WebContents and lower layers. What would be preventing us from
doing
> > that? It seems like we need the Context and the ContainerView, the latter is
> > supposed to be available from webcontents based on web_contents.h
> > (GetNativeView). 
> 
> @Yaron, Can we do this with a follow up patch ? as it needs more changes

@Yaron, I would like to take a follow up to this CL.
Please help me to address following issues.

How we can place addJavascriptInterface() ? Can we move the native part
including java_bridge_dispatcher_host_ to WebContents ?

ContentSettings can we manage inside WebContents instead of CVC ?

[Yaron, 2014-10-15 20:26:25]

On 2014/10/13 12:25:05, AKV wrote:
> @Yaron/Ted Please respond to my inline queries
> 
>
https://codereview.chromium.org/481803004/diff/40001/content/public/android/j...
> File
>
content/public/android/java/src/org/chromium/content/browser/accessibility/AccessibilityInjector.java
> (right):
> 
>
https://codereview.chromium.org/481803004/diff/40001/content/public/android/j...
>
content/public/android/java/src/org/chromium/content/browser/accessibility/AccessibilityInjector.java:308:
> if (mContentViewCore.getWebContents().getUrl() == null) {
> On 2014/09/11 17:19:43, AKV wrote:
> > On 2014/08/18 23:16:24, Yaron wrote:
> > > So this is probably a few steps down the road (i.e. a few blocking issues
in
> > the
> > > way) but I'm curious whether we'd be able to have some of these classes
only
> > > depend on WebContents and lower layers. What would be preventing us from
> doing
> > > that? It seems like we need the Context and the ContainerView, the latter
is
> > > supposed to be available from webcontents based on web_contents.h
> > > (GetNativeView). 
> > 
> > @Yaron, Can we do this with a follow up patch ? as it needs more changes
> 
> @Yaron, I would like to take a follow up to this CL.
> Please help me to address following issues.
> 
> How we can place addJavascriptInterface() ? Can we move the native part
> including java_bridge_dispatcher_host_ to WebContents ?
> 

Is it purely WebContents-based? I'm not familiar with it.
> 
> ContentSettings can we manage inside WebContents instead of CVC ?

Ya, ContentSettings looks like it doesn't need to depend on CVC. Remember we
also talked about removing the NavigationClient interface and replacing it with
NavigationController