Add canUpdateLayout & canHandleGestureEvent to WebViewClient.

Add canUpdateLayout & canHandleGestureEvent to WebViewClient.

This work is in preperation for work to make it illegal to pass a nullptr
for WebViewClient when instantiting a WebView object.

There are a handful of places in WebViewImpl where the logic changes based
on the nullness of WebViewClient. These changes will preserve this logic when
a non-null WebViewClient is supplied.

This include:
- Returning WebInputEventResult::NotHandled from handleGestureEvent if the
  client specifically opts out of handling the event.
- Not processing layout updates if the client opts out.

After removing null checks in WebViewImpl the requirement for these new methods 
are to be reviewed and they should be removed if possible.

Also, in preparation for forcing clients to pass a non null WebViewClient
I made the dtor for WebViewClient public so it can be instantiated without
derivation.

BUG=696895
Review-Url: https://codereview.chromium.org/2762553002
Cr-Commit-Position: refs/heads/master@{#460298}
Committed: https://chromium.googlesource.com/chromium/src/+/1a6375e628f5405f33f9061ee25644eaa90a81c2

[slangley, 2017-03-20 02:18:50]

slangley@chromium.org changed reviewers:
+ dcheng@chromium.org

[slangley, 2017-03-20 02:18:51]

dcheng@ - this is the first checking as an alternate to
https://codereview.chromium.org/2738133002 based on your feedback on that CL.

My preference would be to use https://codereview.chromium.org/2738133002 as that
change is simpler and more robust IMO.

WDYT?

[dcheng, 2017-03-20 03:40:07]

On 2017/03/20 02:18:51, slangley wrote:
> dcheng@ - this is the first checking as an alternate to
> https://codereview.chromium.org/2738133002 based on your feedback on that CL.
> 
> My preference would be to use https://codereview.chromium.org/2738133002 as
that
> change is simpler and more robust IMO.
> 
> WDYT?

Ah OK... I think I understand the confusion.

My intention behind requiring a non-null WebViewClient is to make it so we can
just delete most of the "m_client is null" branches, which should simplify the
logic.

It's true that the logic differs today when m_client is null, but I think that
it shouldn't be: if we have a clearly defined lifecycle for when m_client can
become null, then it should simplify a lot of this code (for example, we
shouldn't be trying to update layout after a WebView is closed, so we shouldn't
need to have the null check there at all). There will probably be a few places
that still need the null check (if it's possible to be called in some delayed
callback), but most of the null checks should simply disappear.

Does this make sense?

[slangley, 2017-03-20 04:38:09]

Sure I believe understand that :)

So the first question to solve is how users of WebView::Create() can
specify that they are not interested in callbacks.

Option 1: Provide an overload that specifies an API that does not take
WebViewClient as a parameters (https://codereview.chromium.org/2738133002)
Option 2: Make all call sites that previously passed nullptr to
WebView::Create() initiate some kind of derived instance  of WebViewClient
and manage the lifecycle of this object.
Option 3: Make the dtor() of WebViewClient public so that clients can use
it without needing to derive, but still need to manage the lifecycle.

I'd still like to advocate for (1) above, to me it seems the simplest
approach. However I'll make a pass at (2) to demonstrate the difference.

Once we have one of these in place then we can assume that if m_client is
null then the WebView has been closed and can act accordingly.

On Mon, Mar 20, 2017 at 2:40 PM, <dcheng@chromium.org> wrote:

> On 2017/03/20 02:18:51, slangley wrote:
> > dcheng@ - this is the first checking as an alternate to
> > https://codereview.chromium.org/2738133002 based on your feedback on
> that CL.
> >
> > My preference would be to use https://codereview.chromium.org/2738133002
> as
> that
> > change is simpler and more robust IMO.
> >
> > WDYT?
>
> Ah OK... I think I understand the confusion.
>
> My intention behind requiring a non-null WebViewClient is to make it so we
> can
> just delete most of the "m_client is null" branches, which should simplify
> the
> logic.
>
> It's true that the logic differs today when m_client is null, but I think
> that
> it shouldn't be: if we have a clearly defined lifecycle for when m_client
> can
> become null, then it should simplify a lot of this code (for example, we
> shouldn't be trying to update layout after a WebView is closed, so we
> shouldn't
> need to have the null check there at all). There will probably be a few
> places
> that still need the null check (if it's possible to be called in some
> delayed
> callback), but most of the null checks should simply disappear.
>
> Does this make sense?
>
> https://codereview.chromium.org/2762553002/
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[slangley, 2017-03-20 04:38:09]

Sure I believe understand that :)

So the first question to solve is how users of WebView::Create() can
specify that they are not interested in callbacks.

Option 1: Provide an overload that specifies an API that does not take
WebViewClient as a parameters (https://codereview.chromium.org/2738133002)
Option 2: Make all call sites that previously passed nullptr to
WebView::Create() initiate some kind of derived instance  of WebViewClient
and manage the lifecycle of this object.
Option 3: Make the dtor() of WebViewClient public so that clients can use
it without needing to derive, but still need to manage the lifecycle.

I'd still like to advocate for (1) above, to me it seems the simplest
approach. However I'll make a pass at (2) to demonstrate the difference.

Once we have one of these in place then we can assume that if m_client is
null then the WebView has been closed and can act accordingly.

On Mon, Mar 20, 2017 at 2:40 PM, <dcheng@chromium.org> wrote:

> On 2017/03/20 02:18:51, slangley wrote:
> > dcheng@ - this is the first checking as an alternate to
> > https://codereview.chromium.org/2738133002 based on your feedback on
> that CL.
> >
> > My preference would be to use https://codereview.chromium.org/2738133002
> as
> that
> > change is simpler and more robust IMO.
> >
> > WDYT?
>
> Ah OK... I think I understand the confusion.
>
> My intention behind requiring a non-null WebViewClient is to make it so we
> can
> just delete most of the "m_client is null" branches, which should simplify
> the
> logic.
>
> It's true that the logic differs today when m_client is null, but I think
> that
> it shouldn't be: if we have a clearly defined lifecycle for when m_client
> can
> become null, then it should simplify a lot of this code (for example, we
> shouldn't be trying to update layout after a WebView is closed, so we
> shouldn't
> need to have the null check there at all). There will probably be a few
> places
> that still need the null check (if it's possible to be called in some
> delayed
> callback), but most of the null checks should simply disappear.
>
> Does this make sense?
>
> https://codereview.chromium.org/2762553002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dglazkov, 2017-03-20 20:13:09]

dcheng@: If I hear you correctly, you're interested in close examination of each
null check and removing it by adjusting the lifecycle, right?

slangley@: If I understand the intent of your patch, you're looking to remove
the null-checks first by codifying remaining places where the behavior depends
on their nullness, and try to decide what to do with them afterwards. Did I get
that right?

If y'all are interested in amazing juicy yak shaves, the whole method
WebViewImpl::layoutUpdate is probably a bug and reflects that the layout
lifecycle isn't correctly captured somewhere around FrameView::layout.

[dcheng, 2017-03-21 06:25:25]

On 2017/03/20 20:13:09, dglazkov wrote:
> dcheng@: If I hear you correctly, you're interested in close examination of
each
> null check and removing it by adjusting the lifecycle, right?

Yeah, in my preferred world, we shouldn't have optional "client" pointers,
because it makes it hard to understand when a behavior is intentional (e.g.
something must not happen because we're shutting the view down) vs accidental
(embedders are allowed to pass null, so we must attempt to handle this
gracefully). For example: https://codereview.chromium.org/2423213002/ (though
never landed since I never got around to addressing the issues with workers)
gets rid of a ton of null checks that aren't actually necessary.

> 
> slangley@: If I understand the intent of your patch, you're looking to remove
> the null-checks first by codifying remaining places where the behavior depends
> on their nullness, and try to decide what to do with them afterwards. Did I
get
> that right?
> 
> If y'all are interested in amazing juicy yak shaves, the whole method
> WebViewImpl::layoutUpdate is probably a bug and reflects that the layout
> lifecycle isn't correctly captured somewhere around FrameView::layout.

Yeah I think this is a good long-term goal, but perhaps orthogonal. In my mind,
there are two things to work on here:

1. Simplify code by making lifetime of objects clearer. Today, we have lots of
code in Blink that null-checks a lot of different pointers, when it's often
unnecessary. This is because it's not documented at all how different objects
relate to each other/are affected by detaching a frame/navigation, etc. So when
traversing between classes, "just to be safe", we null-check every single
pointer along the way.

2. Figuring out the story behind layout updated callbacks. I didn't work on
WebKit as much of the time, but I think one of the reasons for this hook is the
'autoresize' feature of WebView, where we automatically try to size the WebView
container to best fit the contained content. This was used for things like
panels and HTML notifications: though I think both of these features are dead
now, it's sadly still being used for other stuff. I think we'd probably still
need this hook for things like that, but we should probably try to limit what
else it's used for.

[dglazkov, 2017-03-22 00:00:20]

On 2017/03/21 at 06:25:25, dcheng wrote:
> On 2017/03/20 20:13:09, dglazkov wrote:
> > dcheng@: If I hear you correctly, you're interested in close examination of
each
> > null check and removing it by adjusting the lifecycle, right?
> 
> Yeah, in my preferred world, we shouldn't have optional "client" pointers,
because it makes it hard to understand when a behavior is intentional (e.g.
something must not happen because we're shutting the view down) vs accidental
(embedders are allowed to pass null, so we must attempt to handle this
gracefully). For example: https://codereview.chromium.org/2423213002/ (though
never landed since I never got around to addressing the issues with workers)
gets rid of a ton of null checks that aren't actually necessary.
> 
> > 
> > slangley@: If I understand the intent of your patch, you're looking to
remove
> > the null-checks first by codifying remaining places where the behavior
depends
> > on their nullness, and try to decide what to do with them afterwards. Did I
get
> > that right?
> > 
> > If y'all are interested in amazing juicy yak shaves, the whole method
> > WebViewImpl::layoutUpdate is probably a bug and reflects that the layout
> > lifecycle isn't correctly captured somewhere around FrameView::layout.
> 
> Yeah I think this is a good long-term goal, but perhaps orthogonal. In my
mind, there are two things to work on here:
> 
> 1. Simplify code by making lifetime of objects clearer. Today, we have lots of
code in Blink that null-checks a lot of different pointers, when it's often
unnecessary. This is because it's not documented at all how different objects
relate to each other/are affected by detaching a frame/navigation, etc. So when
traversing between classes, "just to be safe", we null-check every single
pointer along the way.
> 
> 2. Figuring out the story behind layout updated callbacks. I didn't work on
WebKit as much of the time, but I think one of the reasons for this hook is the
'autoresize' feature of WebView, where we automatically try to size the WebView
container to best fit the contained content. This was used for things like
panels and HTML notifications: though I think both of these features are dead
now, it's sadly still being used for other stuff. I think we'd probably still
need this hook for things like that, but we should probably try to limit what
else it's used for.

Yep. One idea I had here is modifying FrameView::layout to always supply that
resulting box, since it's always computed at layout time. This way, you don't
need a hook -- just read it when you need it.

[slangley, 2017-03-22 03:38:18]

Description was changed from

==========
Add canUpdateLayout & canHandleGestureEvent to WebViewClient.

This work is in preperation for  work to make it illegal to pass a nullptr
for WebViewClient when instantiting a WebView object.

There are a handful of places in WebViewImpl where the logic changes based
on the nullness of WebViewClient. These changes will preserve this logic when
a non-null WebViewClient is supplied.

This include:
- Returning WebInputEventResult::NotHandled from handleGestureEvent if the
  client specifically opts out of handling the event.
- Not processing layout updates if the client opts out.

Note that I would prefer the approach in
https://codereview.chromium.org/2738133002 to this, where the default methods
return true and the clients then opt out, which is then simplified as all
callers would implicitly get a WebViewClient configured correctly.
==========

to

==========
Add canUpdateLayout & canHandleGestureEvent to WebViewClient.

This work is in preperation for  work to make it illegal to pass a nullptr
for WebViewClient when instantiting a WebView object.

There are a handful of places in WebViewImpl where the logic changes based
on the nullness of WebViewClient. These changes will preserve this logic when
a non-null WebViewClient is supplied.

This include:
- Returning WebInputEventResult::NotHandled from handleGestureEvent if the
  client specifically opts out of handling the event.
- Not processing layout updates if the client opts out.

Also, in preparation for forcing clients to pass a non null WebViewClient I made
the dtor for WebViewClient public so it can be instantiated without derivation.
==========

[slangley, 2017-03-22 03:43:51]

dcheng@ - ptal

As discussed offline, the intention is that this CL is the first in a series of
small steps to remove the conflation of what it means for m_client to be
nullptr.

The steps in order are roughly:

1. Make changes to WebViewClient so we can maintain the existing logic in
WebViewImpl where there currently is a switch in behavior when m_client == null.
(This patch).
2. Force all callers of WebView::create() to pass a valid WebViewClient (made
easier by the public dtor() in this patch).
3. Audit all use cases of m_client and CHECK() for nullness where it should not
be possible, and allow nullness where a delayed callback might have occurred. 
4. Remove the new methods in this patch if they no longer make sense after the
audit of !m_client usage.

Thanks! :)

[dcheng, 2017-03-22 03:51:38]

LGTM

Please update the CL description to also note that we're hoping to clean up
these behavior-preserving methods so we don't have so many branches.

[slangley, 2017-03-22 05:41:41]

Description was changed from

==========
Add canUpdateLayout & canHandleGestureEvent to WebViewClient.

This work is in preperation for  work to make it illegal to pass a nullptr
for WebViewClient when instantiting a WebView object.

There are a handful of places in WebViewImpl where the logic changes based
on the nullness of WebViewClient. These changes will preserve this logic when
a non-null WebViewClient is supplied.

This include:
- Returning WebInputEventResult::NotHandled from handleGestureEvent if the
  client specifically opts out of handling the event.
- Not processing layout updates if the client opts out.

Also, in preparation for forcing clients to pass a non null WebViewClient I made
the dtor for WebViewClient public so it can be instantiated without derivation.
==========

to

==========
Add canUpdateLayout & canHandleGestureEvent to WebViewClient.

This work is in preperation for  work to make it illegal to pass a nullptr
for WebViewClient when instantiting a WebView object.

There are a handful of places in WebViewImpl where the logic changes based
on the nullness of WebViewClient. These changes will preserve this logic when
a non-null WebViewClient is supplied.

This include:
- Returning WebInputEventResult::NotHandled from handleGestureEvent if the
  client specifically opts out of handling the event.
- Not processing layout updates if the client opts out.

After removing null checks in WebViewImpl the requirement for these new methods
are to be reviewed and they should be removed if possible.

Also, in preparation for forcing clients to pass a non null WebViewClient I made
the dtor for WebViewClient public so it can be instantiated without derivation.
==========

[slangley, 2017-03-22 05:41:57]

On 2017/03/22 at 03:51:38, dcheng wrote:
> LGTM
> 
> Please update the CL description to also note that we're hoping to clean up
these behavior-preserving methods so we don't have so many branches.

Done.

Thanks!

[slangley, 2017-03-22 05:42:14]

The CQ bit was checked by slangley@chromium.org

[commit-bot: I haz the power, 2017-03-22 05:42:35]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-22 05:48:43]

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

[commit-bot: I haz the power, 2017-03-22 05:48:44]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[slangley, 2017-03-22 22:37:01]

The CQ bit was checked by slangley@chromium.org

[commit-bot: I haz the power, 2017-03-22 22:38:10]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-22 22:49:42]

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

[commit-bot: I haz the power, 2017-03-22 22:49:43]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[slangley, 2017-03-22 23:32:59]

Description was changed from

==========
Add canUpdateLayout & canHandleGestureEvent to WebViewClient.

This work is in preperation for  work to make it illegal to pass a nullptr
for WebViewClient when instantiting a WebView object.

There are a handful of places in WebViewImpl where the logic changes based
on the nullness of WebViewClient. These changes will preserve this logic when
a non-null WebViewClient is supplied.

This include:
- Returning WebInputEventResult::NotHandled from handleGestureEvent if the
  client specifically opts out of handling the event.
- Not processing layout updates if the client opts out.

After removing null checks in WebViewImpl the requirement for these new methods
are to be reviewed and they should be removed if possible.

Also, in preparation for forcing clients to pass a non null WebViewClient I made
the dtor for WebViewClient public so it can be instantiated without derivation.
==========

to

==========
Add canUpdateLayout & canHandleGestureEvent to WebViewClient.

This work is in preperation for  work to make it illegal to pass a nullptr
for WebViewClient when instantiting a WebView object.

There are a handful of places in WebViewImpl where the logic changes based
on the nullness of WebViewClient. These changes will preserve this logic when
a non-null WebViewClient is supplied.

This include:
- Returning WebInputEventResult::NotHandled from handleGestureEvent if the
  client specifically opts out of handling the event.
- Not processing layout updates if the client opts out.

After removing null checks in WebViewImpl the requirement for these new methods
are to be reviewed and they should be removed if possible.

Also, in preparation for forcing clients to pass a non null WebViewClient I made
the dtor for WebViewClient public so it can be instantiated without derivation.

BUG=696895
==========

[slangley, 2017-03-22 23:36:05]

slangley@chromium.org changed reviewers:
+ tommycli@chromium.org

[slangley, 2017-03-22 23:36:06]

+tommycli for webview_plugin OWNERS

[slangley, 2017-03-22 23:37:13]

slangley@chromium.org changed reviewers:
+ esprehn@chromium.org

[slangley, 2017-03-22 23:37:14]

+esprehn - for content/* OWNERS.

[tommycli, 2017-03-23 19:15:39]

On 2017/03/22 23:37:14, slangley wrote:
> +esprehn - for content/* OWNERS.

lgtm webview plugin

[slangley, 2017-03-27 22:52:45]

esprehn@ - ping

[dglazkov, 2017-03-27 22:56:01]

dglazkov@chromium.org changed reviewers:
+ dglazkov@chromium.org

[dglazkov, 2017-03-27 22:56:02]

lgtm

[slangley, 2017-03-27 23:02:58]

The CQ bit was checked by slangley@chromium.org

[commit-bot: I haz the power, 2017-03-27 23:03:35]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-27 23:13:31]

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

[commit-bot: I haz the power, 2017-03-27 23:13:33]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[esprehn, 2017-03-28 21:42:45]

Description was changed from

==========
Add canUpdateLayout & canHandleGestureEvent to WebViewClient.

This work is in preperation for  work to make it illegal to pass a nullptr
for WebViewClient when instantiting a WebView object.

There are a handful of places in WebViewImpl where the logic changes based
on the nullness of WebViewClient. These changes will preserve this logic when
a non-null WebViewClient is supplied.

This include:
- Returning WebInputEventResult::NotHandled from handleGestureEvent if the
  client specifically opts out of handling the event.
- Not processing layout updates if the client opts out.

After removing null checks in WebViewImpl the requirement for these new methods
are to be reviewed and they should be removed if possible.

Also, in preparation for forcing clients to pass a non null WebViewClient I made
the dtor for WebViewClient public so it can be instantiated without derivation.

BUG=696895
==========

to

==========
Add canUpdateLayout & canHandleGestureEvent to WebViewClient.

This work is in preperation for work to make it illegal to pass a nullptr
for WebViewClient when instantiting a WebView object.

There are a handful of places in WebViewImpl where the logic changes based
on the nullness of WebViewClient. These changes will preserve this logic when
a non-null WebViewClient is supplied.

This include:
- Returning WebInputEventResult::NotHandled from handleGestureEvent if the
  client specifically opts out of handling the event.
- Not processing layout updates if the client opts out.

After removing null checks in WebViewImpl the requirement for these new methods
are to be reviewed and they should be removed if possible.

Also, in preparation for forcing clients to pass a non null WebViewClient I made
the dtor for WebViewClient public so it can be instantiated without derivation.

BUG=696895
==========

[esprehn, 2017-03-28 21:43:04]

lgtm

[esprehn, 2017-03-28 21:43:26]

Description was changed from

==========
Add canUpdateLayout & canHandleGestureEvent to WebViewClient.

This work is in preperation for work to make it illegal to pass a nullptr
for WebViewClient when instantiting a WebView object.

There are a handful of places in WebViewImpl where the logic changes based
on the nullness of WebViewClient. These changes will preserve this logic when
a non-null WebViewClient is supplied.

This include:
- Returning WebInputEventResult::NotHandled from handleGestureEvent if the
  client specifically opts out of handling the event.
- Not processing layout updates if the client opts out.

After removing null checks in WebViewImpl the requirement for these new methods
are to be reviewed and they should be removed if possible.

Also, in preparation for forcing clients to pass a non null WebViewClient I made
the dtor for WebViewClient public so it can be instantiated without derivation.

BUG=696895
==========

to

==========
Add canUpdateLayout & canHandleGestureEvent to WebViewClient.

This work is in preperation for work to make it illegal to pass a nullptr
for WebViewClient when instantiting a WebView object.

There are a handful of places in WebViewImpl where the logic changes based
on the nullness of WebViewClient. These changes will preserve this logic when
a non-null WebViewClient is supplied.

This include:
- Returning WebInputEventResult::NotHandled from handleGestureEvent if the
  client specifically opts out of handling the event.
- Not processing layout updates if the client opts out.

After removing null checks in WebViewImpl the requirement for these new methods 
are to be reviewed and they should be removed if possible.

Also, in preparation for forcing clients to pass a non null WebViewClient
I made the dtor for WebViewClient public so it can be instantiated without
derivation.

BUG=696895
==========

[slangley, 2017-03-28 22:49:16]

The CQ bit was checked by slangley@chromium.org

[commit-bot: I haz the power, 2017-03-28 22:50:02]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-28 23:33:36]

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

[commit-bot: I haz the power, 2017-03-28 23:33:39]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[slangley, 2017-03-29 03:47:43]

The CQ bit was checked by slangley@chromium.org

[slangley, 2017-03-29 03:47:43]

The patchset sent to the CQ was uploaded after l-g-t-m from
tommycli@chromium.org, esprehn@chromium.org, dglazkov@chromium.org,
dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2762553002/#ps40001
(title: "Fix merge issues.")

[commit-bot: I haz the power, 2017-03-29 03:48:44]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-29 03:57:37]

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

[commit-bot: I haz the power, 2017-03-29 03:57:38]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[slangley, 2017-03-29 05:53:49]

The CQ bit was unchecked by slangley@chromium.org

[slangley, 2017-03-29 05:54:22]

slangley@chromium.org changed reviewers:
+ tkent@chromium.org

[slangley, 2017-03-29 05:54:22]

+tkent for content/shell/test_runner/OWNERS

[tkent, 2017-03-29 05:57:24]

lgtm

[slangley, 2017-03-29 05:58:12]

The CQ bit was checked by slangley@chromium.org

[commit-bot: I haz the power, 2017-03-29 05:58:36]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-29 06:03:27]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490767092450570,
"parent_rev": "e5a4554fd2c8c34234f0e301b6855536b3a36009", "commit_rev":
"00d6516398ec823f42052f3c514d97929589f684"}

[commit-bot: I haz the power, 2017-03-29 06:09:30]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490767092450570,
"parent_rev": "9c9082282cb7d4cb1e44ac93dd6ff18780ccb538", "commit_rev":
"1a6375e628f5405f33f9061ee25644eaa90a81c2"}

[commit-bot: I haz the power, 2017-03-29 06:10:55]

Description was changed from

==========
Add canUpdateLayout & canHandleGestureEvent to WebViewClient.

This work is in preperation for work to make it illegal to pass a nullptr
for WebViewClient when instantiting a WebView object.

There are a handful of places in WebViewImpl where the logic changes based
on the nullness of WebViewClient. These changes will preserve this logic when
a non-null WebViewClient is supplied.

This include:
- Returning WebInputEventResult::NotHandled from handleGestureEvent if the
  client specifically opts out of handling the event.
- Not processing layout updates if the client opts out.

After removing null checks in WebViewImpl the requirement for these new methods 
are to be reviewed and they should be removed if possible.

Also, in preparation for forcing clients to pass a non null WebViewClient
I made the dtor for WebViewClient public so it can be instantiated without
derivation.

BUG=696895
==========

to

==========
Add canUpdateLayout & canHandleGestureEvent to WebViewClient.

This work is in preperation for work to make it illegal to pass a nullptr
for WebViewClient when instantiting a WebView object.

There are a handful of places in WebViewImpl where the logic changes based
on the nullness of WebViewClient. These changes will preserve this logic when
a non-null WebViewClient is supplied.

This include:
- Returning WebInputEventResult::NotHandled from handleGestureEvent if the
  client specifically opts out of handling the event.
- Not processing layout updates if the client opts out.

After removing null checks in WebViewImpl the requirement for these new methods 
are to be reviewed and they should be removed if possible.

Also, in preparation for forcing clients to pass a non null WebViewClient
I made the dtor for WebViewClient public so it can be instantiated without
derivation.

BUG=696895

Review-Url: https://codereview.chromium.org/2762553002
Cr-Commit-Position: refs/heads/master@{#460298}
Committed:
https://chromium.googlesource.com/chromium/src/+/1a6375e628f5405f33f9061ee256...
==========

[commit-bot: I haz the power, 2017-03-29 06:10:56]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/1a6375e628f5405f33f9061ee256...