[android] Resize the android_webview if it's 0x0 initially.

[android] Resize the android_webview if it's 0x0 initially.

This changes the content size update path for android_webview to use
the preferred size RenderView mechanism instead of the
CompositorFrameMetadata.
The reason for the change is due to the fact that the CompositorFrameMetadata
is not updated when the view size is 0x0, which is a common use case
for the WebView when it's layout mode is set to "wrap content".

BUG=b/8187850
TEST=AndroidWebViewTests

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

[mkosiba (inactive), 2013-03-26 18:57:52]

Needs a bit more work on the test side (I'd appreciate suggestions on how to
inject a PreferredSizeChangedListener into AwContents) but this is pretty much
it, so PTAL.

[joth, 2013-03-26 22:12:49]

general comments - structurally seems good to me. didn't read the tests in
detail...

I didn't quite follow the question on injecting a preferred size listener...
what breaks if you have setPreferredSizeListener[ForTest] ?

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

https://codereview.chromium.org/12567020/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:258:
mLayoutSizer.onPageScaleChanged(newScale);
my gut instinct is these two calls should be in the other order: generally
retains invariants more reliably to update your own state, before calling out to
another layer.

if it's intentionally this way around, comment why.
(if this was always this way, fine to change in separate CL)

https://codereview.chromium.org/12567020/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:267:
mLayoutSizer.onContentSizeChanged(widthCss, heightCss);
would it be evil for LayoutSize to just implement
AwWebContentsDelegateAdapter.PreferredSizeChangedListener directly?

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

https://codereview.chromium.org/12567020/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwContentsClient.java:38:
public abstract class AwContentsClient extends ContentViewClient {
I'm thinking one of these days we should probably break this inheritance too?

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

https://codereview.chromium.org/12567020/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java:19:
* This class also serves a secondary function of routing certain callbacks to
specific listener
certain callbacks _from the content layer_ to specific...

https://codereview.chromium.org/12567020/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java:25:
public interface PreferredSizeChangedListener {
docs

https://codereview.chromium.org/12567020/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java:30:
PreferredSizeChangedListener mPreferredSizeChangedListener;
both final?

https://codereview.chromium.org/12567020/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java:98:
// This is intentionally not part of mCallbackHelper as that class is intended
for
mCallbackHelper? orphaned comment.

https://codereview.chromium.org/12567020/diff/1/android_webview/native/aw_set...
File android_webview/native/aw_settings.cc (right):

https://codereview.chromium.org/12567020/diff/1/android_webview/native/aw_set...
android_webview/native/aw_settings.cc:106:
render_view_host->EnablePreferredSizeMode();
for consistency I wonder about having UpdatePreferredSizeMode(), called here and
in SetWebContents

That way there's a single place to look for the policy about what it is set to
(i.e. always enabled).

also, I now wonder if the other UpdateFooBar methods need calling from the
SetWebContents? maybe check with mikhail/ben (it's mostly relevant for the
pop-up window case)

https://codereview.chromium.org/12567020/diff/1/components/web_contents_deleg...
File
components/web_contents_delegate_android/android/java/src/org/chromium/components/web_contents_delegate_android/WebContentsDelegateAndroid.java
(right):

https://codereview.chromium.org/12567020/diff/1/components/web_contents_deleg...
components/web_contents_delegate_android/android/java/src/org/chromium/components/web_contents_delegate_android/WebContentsDelegateAndroid.java:125:
public void updatePreferredSize(int widthCss, int heightCss) {
move to AwWebContentsDelegate unless needed in chrome too.

[mnaganov (inactive), 2013-03-27 11:06:31]

https://codereview.chromium.org/12567020/diff/1/android_webview/native/aw_set...
File android_webview/native/aw_settings.cc (right):

https://codereview.chromium.org/12567020/diff/1/android_webview/native/aw_set...
android_webview/native/aw_settings.cc:106:
render_view_host->EnablePreferredSizeMode();
On 2013/03/26 22:12:49, joth wrote:
> for consistency I wonder about having UpdatePreferredSizeMode(), called here
and
> in SetWebContents
> 
> That way there's a single place to look for the policy about what it is set to
> (i.e. always enabled).
> 
> also, I now wonder if the other UpdateFooBar methods need calling from the
> SetWebContents? maybe check with mikhail/ben (it's mostly relevant for the
> pop-up window case)

Yes, a good point! I have completely missed the pop-up case here. We also need
to be able to copy those setting from the embedder-provided WebView. Currently,
we only do that for the ContentSettings part.

I will shortly turn to refactoring ContentSettings / AwSettings mayhem and will
sort the things out.

[mkosiba (inactive), 2013-03-27 16:46:41]

+ yfriedman for content/public/test/android/... OWNERS approval

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

https://codereview.chromium.org/12567020/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:258:
mLayoutSizer.onPageScaleChanged(newScale);
On 2013/03/26 22:12:49, joth wrote:
> my gut instinct is these two calls should be in the other order: generally
> retains invariants more reliably to update your own state, before calling out
to
> another layer.
> 
> if it's intentionally this way around, comment why.
> (if this was always this way, fine to change in separate CL)

Good find!

https://codereview.chromium.org/12567020/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:267:
mLayoutSizer.onContentSizeChanged(widthCss, heightCss);
On 2013/03/26 22:12:49, joth wrote:
> would it be evil for LayoutSize to just implement
> AwWebContentsDelegateAdapter.PreferredSizeChangedListener directly?

not terribly, no. The only reason I wanted to keep this separate was the method
naming - AwLayoutSizer.onContentSizeChange reads a lot better in unittests
compared to AwLayoutSizer.updatePreferredSize.

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

https://codereview.chromium.org/12567020/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwContentsClient.java:38:
public abstract class AwContentsClient extends ContentViewClient {
On 2013/03/26 22:12:49, joth wrote:
> I'm thinking one of these days we should probably break this inheritance too?

probably. ContentViewClient is pretty thin these days though.

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

https://codereview.chromium.org/12567020/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java:19:
* This class also serves a secondary function of routing certain callbacks to
specific listener
On 2013/03/26 22:12:49, joth wrote:
> certain callbacks _from the content layer_ to specific...

Done.

https://codereview.chromium.org/12567020/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java:25:
public interface PreferredSizeChangedListener {
On 2013/03/26 22:12:49, joth wrote:
> docs

Done.

https://codereview.chromium.org/12567020/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java:30:
PreferredSizeChangedListener mPreferredSizeChangedListener;
On 2013/03/26 22:12:49, joth wrote:
> both final?

Done.

https://codereview.chromium.org/12567020/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java:98:
// This is intentionally not part of mCallbackHelper as that class is intended
for
On 2013/03/26 22:12:49, joth wrote:
> mCallbackHelper? orphaned comment.

Done.

[mkosiba (inactive), 2013-03-28 14:20:36]

+CC:aelias (fyi)

Ping?

https://codereview.chromium.org/12567020/diff/11018/android_webview/native/aw...
File android_webview/native/aw_settings.cc (right):

https://codereview.chromium.org/12567020/diff/11018/android_webview/native/aw...
android_webview/native/aw_settings.cc:128:
DCHECK(web_contents()->GetRenderViewHost() == render_view_host);
this fails on some tests, which most likely means we're not always pushing the
AwSettings to the right RenderView! I'll take this DCHECK out for the moment and
work with Mikhail to identify and fix the problem.

[Yaron, 2013-03-28 22:11:05]

content/public/test/android lgtm

[joth, 2013-03-28 22:51:08]

mostly nits, other than moving updatePreferredSize out of the shared component
into aw/ ....

https://codereview.chromium.org/12567020/diff/1/components/web_contents_deleg...
File
components/web_contents_delegate_android/android/java/src/org/chromium/components/web_contents_delegate_android/WebContentsDelegateAndroid.java
(right):

https://codereview.chromium.org/12567020/diff/1/components/web_contents_deleg...
components/web_contents_delegate_android/android/java/src/org/chromium/components/web_contents_delegate_android/WebContentsDelegateAndroid.java:125:
public void updatePreferredSize(int widthCss, int heightCss) {
On 2013/03/26 22:12:49, joth wrote:
> move to AwWebContentsDelegate unless needed in chrome too.

Looks like you missed this comment ?

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

https://codereview.chromium.org/12567020/diff/11018/android_webview/java/src/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:281: */
comment this c'tor uses default versions of the otherwise injectable parts

https://codereview.chromium.org/12567020/diff/11018/android_webview/java/src/...
File android_webview/java/src/org/chromium/android_webview/AwLayoutSizer.java
(right):

https://codereview.chromium.org/12567020/diff/11018/android_webview/java/src/...
android_webview/java/src/org/chromium/android_webview/AwLayoutSizer.java:79:
boolean anyMeasurementNotFixed = !mWidthMeasurementIsFixed ||
!mHeightMeasurementIsFixed;
any summery of the intended behavior of these expressions you can add?

https://codereview.chromium.org/12567020/diff/11018/android_webview/javatests...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwLayoutSizerTest.java
(right):

https://codereview.chromium.org/12567020/diff/11018/android_webview/javatests...
android_webview/javatests/src/org/chromium/android_webview/test/AwLayoutSizerTest.java:43:
private static final double initialPageScale = 1.0;
hmm as these are at class scope they should be MACRO_STYLE

https://codereview.chromium.org/12567020/diff/11018/android_webview/native/aw...
File android_webview/native/aw_settings.cc (right):

https://codereview.chromium.org/12567020/diff/11018/android_webview/native/aw...
android_webview/native/aw_settings.cc:128:
DCHECK(web_contents()->GetRenderViewHost() == render_view_host);
On 2013/03/28 14:20:37, Martin Kosiba wrote:
> this fails on some tests, which most likely means we're not always pushing the
> AwSettings to the right RenderView! I'll take this DCHECK out for the moment
and
> work with Mikhail to identify and fix the problem.

There's a tricky dance that happens on cross site navigations, with the RVH
being created and only being made "current" later on (when provisional load
committed, right?) you're probably tickling that.
It does indeed sound like RVHExt is either flawed in this scenario, or at very
least, badly named. I haven't quite got my head around what the 'right' way to
deal with this is.

[aelias_OOO_until_Jul13, 2013-03-28 23:01:14]

Hmm, pageScaleFactor comes in the metadata so you're mixing information from two
sources.  That makes the original approach seem more reliable.  For example, if
you haven't heard the value of pageScaleFactor yet but get preferredSize, you
probably assume 1 but that may not be in sync with what the viewport tag
initial-scale says.

I was thinking in any case that for WebView, we could move to a "poll" model for
the CompositorFrameMetadata.  You could call a method on the OutputSurfaceClient
at any time and it would give you the current content size and page scale.  That
seems cleaner than either of the approaches so far?

[mkosiba (inactive), 2013-04-03 16:25:28]

@alex - Agree about mixing stuff from cc/ and RenderView being wrong. I've
uploaded an alternative proposal which is boils down to getting the pageScale
from the RenderView.
After fiddling with this implementation it turns out that the gesture detector
gets super-confused when you change the size of the view mid-way through the
gesture. This is probably why WebViewClassic only does the re-layout after the
pinch-zoom gesture ends (after we get the UP touch event).

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

https://codereview.chromium.org/12567020/diff/11018/android_webview/java/src/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:281: */
On 2013/03/28 22:51:08, joth wrote:
> comment this c'tor uses default versions of the otherwise injectable parts

Done.

https://codereview.chromium.org/12567020/diff/11018/android_webview/java/src/...
File android_webview/java/src/org/chromium/android_webview/AwLayoutSizer.java
(right):

https://codereview.chromium.org/12567020/diff/11018/android_webview/java/src/...
android_webview/java/src/org/chromium/android_webview/AwLayoutSizer.java:79:
boolean anyMeasurementNotFixed = !mWidthMeasurementIsFixed ||
!mHeightMeasurementIsFixed;
On 2013/03/28 22:51:08, joth wrote:
> any summery of the intended behavior of these expressions you can add?

Done.

https://codereview.chromium.org/12567020/diff/11018/android_webview/javatests...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwLayoutSizerTest.java
(right):

https://codereview.chromium.org/12567020/diff/11018/android_webview/javatests...
android_webview/javatests/src/org/chromium/android_webview/test/AwLayoutSizerTest.java:43:
private static final double initialPageScale = 1.0;
On 2013/03/28 22:51:08, joth wrote:
> hmm as these are at class scope they should be MACRO_STYLE

Done.

https://codereview.chromium.org/12567020/diff/11018/android_webview/native/aw...
File android_webview/native/aw_settings.cc (right):

https://codereview.chromium.org/12567020/diff/11018/android_webview/native/aw...
android_webview/native/aw_settings.cc:128:
DCHECK(web_contents()->GetRenderViewHost() == render_view_host);
On 2013/03/28 22:51:08, joth wrote:
> On 2013/03/28 14:20:37, Martin Kosiba wrote:
> > this fails on some tests, which most likely means we're not always pushing
the
> > AwSettings to the right RenderView! I'll take this DCHECK out for the moment
> and
> > work with Mikhail to identify and fix the problem.
> 
> There's a tricky dance that happens on cross site navigations, with the RVH
> being created and only being made "current" later on (when provisional load
> committed, right?) you're probably tickling that.
> It does indeed sound like RVHExt is either flawed in this scenario, or at very
> least, badly named. I haven't quite got my head around what the 'right' way to
> deal with this is.

So the way I see it is that we've got an actual problem, since this means that
whenever the DCHECK fails we don't propagate the settings to the correct
RenderView. Essentially we'd need to register for a RenderViewMadeCurrent
notification or something.

[aelias_OOO_until_Jul13, 2013-04-04 04:44:37]

Hmm, based on jreck's email feedback pinch-zoom for resizable views never worked
very well anyway and was discouraged.  So maybe we don't need to worry that much
about metadata synchronization after all.  Doing the relayout only when the
pinch gesture ends seems reasonable, are you going with that?

[mkosiba (inactive), 2013-04-04 15:53:24]

On 2013/04/04 04:44:37, aelias wrote:
> Hmm, based on jreck's email feedback pinch-zoom for resizable views never
worked
> very well anyway and was discouraged.  So maybe we don't need to worry that
much
> about metadata synchronization after all.  Doing the relayout only when the
> pinch gesture ends seems reasonable, are you going with that?

Yes, I'd like to go with that (but in a follow-up CL). I'd like some
confirmation on whether polling the pageScaleFactor in
RenderViewObserver::DidCommitCompositorFrame is a reasonable approach.

[mkosiba (inactive), 2013-04-05 15:47:42]

ok, turns out pausing layout for the duration of a pinch gesture wasn't that
complex so I just added it to this CL.

@joth - PTAL.

[joth, 2013-04-10 22:06:23]

(sorry I read this code several times but never got the comments sent for some
reason)

minor suggestions, but I'm mostly out of my depth on the finer details of the
correctness of the scaling logic... defer to mikhail and alex on that.
also, alex, could you check the updatePinchGestureStateListener in
ContentViewCore makes sense as an API from your point of view?

Thanks

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

https://codereview.chromium.org/12567020/diff/42001/android_webview/java/src/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:279:
public static class DependencyFactory {
this class only exists to unravel the construction dependency (can't make
AwLayoutSizer without the delegate, but in normal usage the caller is the
delegate?)
The more normal chrome pattern is to add a setDelegate() to the sizer instead of
pass it on c'tor, and then you just pass the layoutsizer in and avoid the need
for a domain specific dependency factory.

https://codereview.chromium.org/12567020/diff/42001/android_webview/java/src/...
File android_webview/java/src/org/chromium/android_webview/AwLayoutSizer.java
(right):

https://codereview.chromium.org/12567020/diff/42001/android_webview/java/src/...
android_webview/java/src/org/chromium/android_webview/AwLayoutSizer.java:142:
int contentWidthPix = (int) (mContentWidthCss * mPageScaleFactor * mDIPScale);
I feel I'd benefit from a comment, but actually the code probably says it all.
(Think I'm just looking for some reassurance that we're not double correcting
these factors..)

https://codereview.chromium.org/12567020/diff/42001/android_webview/native/aw...
File android_webview/native/aw_contents.cc (right):

https://codereview.chromium.org/12567020/diff/42001/android_webview/native/aw...
android_webview/native/aw_contents.cc:635: if (obj.is_null()) return;
nit: \n before return in c++

https://codereview.chromium.org/12567020/diff/42001/android_webview/native/aw...
File android_webview/native/aw_settings.cc (right):

https://codereview.chromium.org/12567020/diff/42001/android_webview/native/aw...
android_webview/native/aw_settings.cc:130: // the RenderViewHostExtSettings to
the wrong RenderView.
I forget the outcome... but was there a bug for this?

[mkosiba (inactive), 2013-04-12 16:42:19]

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

https://codereview.chromium.org/12567020/diff/42001/android_webview/java/src/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:279:
public static class DependencyFactory {
On 2013/04/10 22:06:23, joth wrote:
> this class only exists to unravel the construction dependency (can't make
> AwLayoutSizer without the delegate, but in normal usage the caller is the
> delegate?)
> The more normal chrome pattern is to add a setDelegate() to the sizer instead
of
> pass it on c'tor, and then you just pass the layoutsizer in and avoid the need
> for a domain specific dependency factory. 

Done, although the code feels less clean now. Having the setDelegate method
creates a logical dependency (the caller has to 'remember' to call setDelegate
and setDIPScale) that was not previously present.

https://codereview.chromium.org/12567020/diff/42001/android_webview/native/aw...
File android_webview/native/aw_contents.cc (right):

https://codereview.chromium.org/12567020/diff/42001/android_webview/native/aw...
android_webview/native/aw_contents.cc:635: if (obj.is_null()) return;
On 2013/04/10 22:06:23, joth wrote:
> nit: \n before return in c++

Done.

https://codereview.chromium.org/12567020/diff/42001/android_webview/native/aw...
File android_webview/native/aw_settings.cc (right):

https://codereview.chromium.org/12567020/diff/42001/android_webview/native/aw...
android_webview/native/aw_settings.cc:130: // the RenderViewHostExtSettings to
the wrong RenderView.
On 2013/04/10 22:06:23, joth wrote:
> I forget the outcome... but was there a bug for this?

I thought Mikhail's patch to not swap RVH's out fixes this. I did a test pass
with the DCHECK in and it seems to work fine.

[mnaganov (inactive), 2013-04-14 20:53:22]

On 2013/04/12 16:42:19, mkosiba wrote:
>
https://codereview.chromium.org/12567020/diff/42001/android_webview/java/src/...
> File android_webview/java/src/org/chromium/android_webview/AwContents.java
> (right):
> 
>
https://codereview.chromium.org/12567020/diff/42001/android_webview/java/src/...
> android_webview/java/src/org/chromium/android_webview/AwContents.java:279:
> public static class DependencyFactory {
> On 2013/04/10 22:06:23, joth wrote:
> > this class only exists to unravel the construction dependency (can't make
> > AwLayoutSizer without the delegate, but in normal usage the caller is the
> > delegate?)
> > The more normal chrome pattern is to add a setDelegate() to the sizer
instead
> of
> > pass it on c'tor, and then you just pass the layoutsizer in and avoid the
need
> > for a domain specific dependency factory. 
> 
> Done, although the code feels less clean now. Having the setDelegate method
> creates a logical dependency (the caller has to 'remember' to call setDelegate
> and setDIPScale) that was not previously present.
> 
>
https://codereview.chromium.org/12567020/diff/42001/android_webview/native/aw...
> File android_webview/native/aw_contents.cc (right):
> 
>
https://codereview.chromium.org/12567020/diff/42001/android_webview/native/aw...
> android_webview/native/aw_contents.cc:635: if (obj.is_null()) return;
> On 2013/04/10 22:06:23, joth wrote:
> > nit: \n before return in c++
> 
> Done.
> 
>
https://codereview.chromium.org/12567020/diff/42001/android_webview/native/aw...
> File android_webview/native/aw_settings.cc (right):
> 
>
https://codereview.chromium.org/12567020/diff/42001/android_webview/native/aw...
> android_webview/native/aw_settings.cc:130: // the RenderViewHostExtSettings to
> the wrong RenderView.
> On 2013/04/10 22:06:23, joth wrote:
> > I forget the outcome... but was there a bug for this?
> 
> I thought Mikhail's patch to not swap RVH's out fixes this. I did a test pass
> with the DCHECK in and it seems to work fine.

Correct. According to creis@, apart from the swapping that I disabled for the
single process mode, the remaining cases are for special occasions like WebUI,
extensions, and view-source.

[joth, 2013-04-17 00:41:50]

lgtm

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

https://codereview.chromium.org/12567020/diff/86001/android_webview/java/src/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:342: 
nit: spurious \n

https://codereview.chromium.org/12567020/diff/86001/android_webview/java/src/...
File android_webview/java/src/org/chromium/android_webview/AwLayoutSizer.java
(right):

https://codereview.chromium.org/12567020/diff/86001/android_webview/java/src/...
android_webview/java/src/org/chromium/android_webview/AwLayoutSizer.java:45:
public AwLayoutSizer() {
maybe just add a comment that setDelegate must be called before this class is
ready for use

[mkosiba (inactive), 2013-04-17 10:15:44]

+jln for android_webview/common/render_view_messages.h

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

https://codereview.chromium.org/12567020/diff/86001/android_webview/java/src/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:342: 
On 2013/04/17 00:41:50, joth wrote:
> nit: spurious \n

Done.

https://codereview.chromium.org/12567020/diff/86001/android_webview/java/src/...
File android_webview/java/src/org/chromium/android_webview/AwLayoutSizer.java
(right):

https://codereview.chromium.org/12567020/diff/86001/android_webview/java/src/...
android_webview/java/src/org/chromium/android_webview/AwLayoutSizer.java:45:
public AwLayoutSizer() {
On 2013/04/17 00:41:50, joth wrote:
> maybe just add a comment that setDelegate must be called before this class is
> ready for use

Done.

[joth, 2013-04-17 20:39:00]

+palmer for android_webview/common/render_**view_messages.h as I think he
is most familiar with the android-y side of things.



On 17 April 2013 03:15, <mkosiba@chromium.org> wrote:

> +jln for android_webview/common/render_**view_messages.h
>
>
>
> https://codereview.chromium.**org/12567020/diff/86001/**
>
android_webview/java/src/org/**chromium/android_webview/**AwContents.java<https://codereview.chromium.org/12567020/diff/86001/android_webview/java/src/org/chromium/android_webview/AwContents.java>
> File
> android_webview/java/src/org/**chromium/android_webview/**AwContents.java
> (right):
>
> https://codereview.chromium.**org/12567020/diff/86001/**
> android_webview/java/src/org/**chromium/android_webview/**
>
AwContents.java#newcode342<https://codereview.chromium.org/12567020/diff/86001/android_webview/java/src/org/chromium/android_webview/AwContents.java#newcode342>
> android_webview/java/src/org/**chromium/android_webview/**
> AwContents.java:342:
>
> On 2013/04/17 00:41:50, joth wrote:
>
>> nit: spurious \n
>>
>
> Done.
>
>
> https://codereview.chromium.**org/12567020/diff/86001/**
> android_webview/java/src/org/**chromium/android_webview/**
>
AwLayoutSizer.java<https://codereview.chromium.org/12567020/diff/86001/android_webview/java/src/org/chromium/android_webview/AwLayoutSizer.java>
> File
> android_webview/java/src/org/**chromium/android_webview/**
> AwLayoutSizer.java
> (right):
>
> https://codereview.chromium.**org/12567020/diff/86001/**
> android_webview/java/src/org/**chromium/android_webview/**
>
AwLayoutSizer.java#newcode45<https://codereview.chromium.org/12567020/diff/86001/android_webview/java/src/org/chromium/android_webview/AwLayoutSizer.java#newcode45>
> android_webview/java/src/org/**chromium/android_webview/**
> AwLayoutSizer.java:45:
> public AwLayoutSizer() {
> On 2013/04/17 00:41:50, joth wrote:
>
>> maybe just add a comment that setDelegate must be called before this
>>
> class is
>
>> ready for use
>>
>
> Done.
>
>
https://codereview.chromium.**org/12567020/<https://codereview.chromium.org/1...
>

[mkosiba (inactive), 2013-04-18 13:22:44]

+palmer,-jln for android_webview/common/render_**view_messages.h

[palmer, 2013-04-18 19:21:59]

IPC security LGTM

[commit-bot: I haz the power, 2013-04-19 08:35:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mkosiba@chromium.org/12567020/99001

[commit-bot: I haz the power, 2013-04-19 08:37:27]

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=ios_dbg_si...

[commit-bot: I haz the power, 2013-04-19 08:52:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mkosiba@chromium.org/12567020/99001

[commit-bot: I haz the power, 2013-04-19 12:46:40]

Change committed as 195135