[Android WebView] Provide more information about WebView for web debugging

[Android WebView] Provide more information about WebView for web debugging

Using only page title is not enough for identifying a WebView.
This change adds to the remote web debugging landing page more
information about debuggable WebViews.

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

[mnaganov (inactive), 2013-03-08 15:09:36]

This patch requires https://codereview.chromium.org/12472004/ to land first.

[joth, 2013-03-08 23:23:46]

https://codereview.chromium.org/12697002/diff/1/android_webview/browser/brows...
File android_webview/browser/browser_view_renderer.h (right):

https://codereview.chromium.org/12697002/diff/1/android_webview/browser/brows...
android_webview/browser/browser_view_renderer.h:41: virtual bool
isAttachedToViewHierarchy() = 0;
both these methods: leading capital letter in C++

https://codereview.chromium.org/12697002/diff/1/android_webview/browser/brows...
android_webview/browser/browser_view_renderer.h:106: virtual gfx::Rect
getScreenRect() = 0;
ditto

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

https://codereview.chromium.org/12697002/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:964:
private boolean isAttachedToViewHierarchy() {
shouldn't need this method: we already pushed the window attachment down to
native via nativeOnAttachedToWindow

https://codereview.chromium.org/12697002/diff/1/android_webview/native/aw_con...
File android_webview/native/aw_contents.cc (right):

https://codereview.chromium.org/12697002/diff/1/android_webview/native/aw_con...
android_webview/native/aw_contents.cc:115: BrowserViewRenderer*
BrowserViewRenderer::FromWebContents(
this is really weird here!

BrowserViewRenderer is already holding a lot of responsibility for the draw path
so maybe confusing it to pull it into the dev tools flow too... but I see
there's nothing more convenient in browser/ for you to use at this point. 

AwContentsClientBridgeBase almost works, but isn't ideal as you don't really
want to pop out via the AwContentsClient.

If we keep with BrowserViewRenderer, I think the 'correct' solution is to have
it register itself with the WebContents::SetUserData() and then it can implement
its own FromWebContents() method. (And that would also help tackle the lifetime
ambiguity that currently exists between the two)

[mnaganov (inactive), 2013-03-11 14:39:19]

https://codereview.chromium.org/12697002/diff/1/android_webview/browser/brows...
File android_webview/browser/browser_view_renderer.h (right):

https://codereview.chromium.org/12697002/diff/1/android_webview/browser/brows...
android_webview/browser/browser_view_renderer.h:41: virtual bool
isAttachedToViewHierarchy() = 0;
On 2013/03/08 23:23:46, joth wrote:
> both these methods: leading capital letter in C++

This is what happens when you mix Java and C++ :( Fixed.

https://codereview.chromium.org/12697002/diff/1/android_webview/browser/brows...
android_webview/browser/browser_view_renderer.h:106: virtual gfx::Rect
getScreenRect() = 0;
On 2013/03/08 23:23:46, joth wrote:
> ditto

Done.

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

https://codereview.chromium.org/12697002/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:964:
private boolean isAttachedToViewHierarchy() {
On 2013/03/08 23:23:46, joth wrote:
> shouldn't need this method: we already pushed the window attachment down to
> native via nativeOnAttachedToWindow

Right. When I first tried using these notifications, I've got an impression that
the detachment event is only called on view deletion, but now I rechecked, and
it looks like the state obtained from the event is consistent with the state
calculated by this function. Removed.

https://codereview.chromium.org/12697002/diff/1/android_webview/native/aw_con...
File android_webview/native/aw_contents.cc (right):

https://codereview.chromium.org/12697002/diff/1/android_webview/native/aw_con...
android_webview/native/aw_contents.cc:115: BrowserViewRenderer*
BrowserViewRenderer::FromWebContents(
On 2013/03/08 23:23:46, joth wrote:
> this is really weird here!
> 
> BrowserViewRenderer is already holding a lot of responsibility for the draw
path
> so maybe confusing it to pull it into the dev tools flow too... but I see
> there's nothing more convenient in browser/ for you to use at this point. 
> 
> AwContentsClientBridgeBase almost works, but isn't ideal as you don't really
> want to pop out via the AwContentsClient.
> 
> If we keep with BrowserViewRenderer, I think the 'correct' solution is to have
> it register itself with the WebContents::SetUserData() and then it can
implement
> its own FromWebContents() method. (And that would also help tackle the
lifetime
> ambiguity that currently exists between the two)

I would be using AwContents directly (because I'm viewing it as an extension of
WebContents), but this is prohibited by our architecture.

BrowserViewRenderer was the second natural choice, because it was only missing
the screen location data.

I did this hack as it required the least amount of lines. Switched to the
userdata approach.

[joth, 2013-03-11 16:36:12]

lgtm although I've caused you to need a rebase..

https://codereview.chromium.org/12697002/diff/5001/android_webview/browser/br...
File android_webview/browser/browser_view_renderer_impl.cc (right):

https://codereview.chromium.org/12697002/diff/5001/android_webview/browser/br...
android_webview/browser/browser_view_renderer_impl.cc:111: class
BrowserViewRendererUserData : public base::SupportsUserData::Data {
ah no - afraid we duplicated work here..!
Mine's already in CQ so I think you'll need to rebase.
https://codereview.chromium.org/12668003/diff/11001/android_webview/browser/b...

[mnaganov (inactive), 2013-03-12 09:51:27]

On 2013/03/11 16:36:12, joth wrote:
> lgtm although I've caused you to need a rebase..
> 
>
https://codereview.chromium.org/12697002/diff/5001/android_webview/browser/br...
> File android_webview/browser/browser_view_renderer_impl.cc (right):
> 
>
https://codereview.chromium.org/12697002/diff/5001/android_webview/browser/br...
> android_webview/browser/browser_view_renderer_impl.cc:111: class
> BrowserViewRendererUserData : public base::SupportsUserData::Data {
> ah no - afraid we duplicated work here..!
> Mine's already in CQ so I think you'll need to rebase.
>
https://codereview.chromium.org/12668003/diff/11001/android_webview/browser/b...

Thanks! Rebased, committing...

[commit-bot: I haz the power, 2013-03-12 09:51:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mnaganov@chromium.org/12697002/10001

[commit-bot: I haz the power, 2013-03-12 12:03:05]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[mnaganov (inactive), 2013-03-12 13:23:58]

Committed patchset #3 manually as r187573 (presubmit successful).