Make the WebWidgetClient parameter a ref so it's not legal to pass a nullptr.

Ensure that the WebWidgetClient cannot be a nullptr by adding DCHECK()s.

There was only one call site in print_web_view_helper where a nullptr was being passed instead of a valid WebWidgetClient, to fix this the following changes were also made:

* Make WebWidgetClient dtor public so the class can be instantiated.
* Pass a valid WebWidgetClient in the print_web_view_helper

A subsequent CL will analyze and remove m_client null checks in WebFrameWidgetImpl.

I left WebFrameViewClient as a ref as this appears to be internal to blink and will adhere to blink standards.

BUG=696895
Review-Url: https://codereview.chromium.org/2773003002
Cr-Commit-Position: refs/heads/master@{#460235}
Committed: https://chromium.googlesource.com/chromium/src/+/9df2c54436a76ebaf6bfcc317815e776e4a0a7f5

[slangley, 2017-03-24 02:39:21]

Description was changed from

==========
Make the WebWidgetClient parameter a ref so it's not legal to pass a nullptr.

Make WebWidgetClient dtor public so the class can be instantiated.

Pass a valid WebWidgetClient in the print_web_view_helper




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

to

==========
Make the WebWidgetClient parameter a ref so it's not legal to pass a nullptr.

There was only one call site in print_web_view_helper where a nullptr was being
passed instead of a valid WebWidgetClient, to fix this the following changes
were also made:

* Make WebWidgetClient dtor public so the class can be instantiated.
* Pass a valid WebWidgetClient in the print_web_view_helper

A subsequent CL will analyze and remove m_client null checks in
WebFrameWidgetImpl.

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

[slangley, 2017-03-24 02:40:10]

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

[dcheng, 2017-03-24 04:32:59]

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

[dcheng, 2017-03-24 04:32:59]

So I know I said that we use non-mutable references in Blink for things that
should never be null... however, it's relatively rare to do that in the public
API since that interfaces with Chrome:
https://cs.chromium.org/search/?q=file:WebKit/public+%5B(,%5DWeb%5B%5E+%5D%2B...

For consistency with WebLocalFrame/WebRemoteFrame and the rest of the public
API, I am inclined to just DCHECK that the client parameter is not null, if
you're OK with that?

+haraken as well to see what he thinks

[haraken, 2017-03-24 04:39:26]

On 2017/03/24 04:32:59, dcheng wrote:
> So I know I said that we use non-mutable references in Blink for things that
> should never be null... however, it's relatively rare to do that in the public
> API since that interfaces with Chrome:
>
https://cs.chromium.org/search/?q=file:WebKit/public+%5B(,%5DWeb%5B%5E+%5D%2B...
> 
> For consistency with WebLocalFrame/WebRemoteFrame and the rest of the public
> API, I am inclined to just DCHECK that the client parameter is not null, if
> you're OK with that?
> 
> +haraken as well to see what he thinks

(I don't have any strong opinion on it :-)

[slangley, 2017-03-24 04:55:58]

TBH I'm strongly opposed to using non const ref arguments (I need to
respond to that other mail) so I'm happy to revert the majority of this
change. :)

On Fri, Mar 24, 2017 at 3:39 PM, <haraken@chromium.org> wrote:

> On 2017/03/24 04:32:59, dcheng wrote:
> > So I know I said that we use non-mutable references in Blink for things
> that
> > should never be null... however, it's relatively rare to do that in the
> public
> > API since that interfaces with Chrome:
> >
> https://cs.chromium.org/search/?q=file:WebKit/public+%
> 5B(,%5DWeb%5B%5E+%5D%2B%5C%26%5B,+%5D+package:%5Echromium$&type=cs
> >
> > For consistency with WebLocalFrame/WebRemoteFrame and the rest of the
> public
> > API, I am inclined to just DCHECK that the client parameter is not null,
> if
> > you're OK with that?
> >
> > +haraken as well to see what he thinks
>
> (I don't have any strong opinion on it :-)
>
>
> https://codereview.chromium.org/2773003002/
>

-- 
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-24 04:55:58]

TBH I'm strongly opposed to using non const ref arguments (I need to
respond to that other mail) so I'm happy to revert the majority of this
change. :)

On Fri, Mar 24, 2017 at 3:39 PM, <haraken@chromium.org> wrote:

> On 2017/03/24 04:32:59, dcheng wrote:
> > So I know I said that we use non-mutable references in Blink for things
> that
> > should never be null... however, it's relatively rare to do that in the
> public
> > API since that interfaces with Chrome:
> >
> https://cs.chromium.org/search/?q=file:WebKit/public+%
> 5B(,%5DWeb%5B%5E+%5D%2B%5C%26%5B,+%5D+package:%5Echromium$&type=cs
> >
> > For consistency with WebLocalFrame/WebRemoteFrame and the rest of the
> public
> > API, I am inclined to just DCHECK that the client parameter is not null,
> if
> > you're OK with that?
> >
> > +haraken as well to see what he thinks
>
> (I don't have any strong opinion on it :-)
>
>
> https://codereview.chromium.org/2773003002/
>

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

[slangley, 2017-03-24 05:33:09]

Description was changed from

==========
Make the WebWidgetClient parameter a ref so it's not legal to pass a nullptr.

There was only one call site in print_web_view_helper where a nullptr was being
passed instead of a valid WebWidgetClient, to fix this the following changes
were also made:

* Make WebWidgetClient dtor public so the class can be instantiated.
* Pass a valid WebWidgetClient in the print_web_view_helper

A subsequent CL will analyze and remove m_client null checks in
WebFrameWidgetImpl.

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

to

==========
Ensure that the WebWidgetClient cannot be a nullptr by adding DCHECK()s.

There was only one call site in print_web_view_helper where a nullptr was being
passed instead of a valid WebWidgetClient, to fix this the following changes
were also made:

* Make WebWidgetClient dtor public so the class can be instantiated.
* Pass a valid WebWidgetClient in the print_web_view_helper

A subsequent CL will analyze and remove m_client null checks in
WebFrameWidgetImpl.

I left WebFrameViewClient as a ref as this appears to be internal to blink and
will adhere to blink standards.

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

[slangley, 2017-03-24 05:33:24]

ptal

[dcheng, 2017-03-24 05:35:47]

LGTM

[haraken, 2017-03-24 05:36:04]

LGTM

[slangley, 2017-03-28 00:07:09]

The CQ bit was checked by slangley@chromium.org

[slangley, 2017-03-28 00:07:09]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2773003002/#ps40001
(title: "Break dependancy on upstream branch.")

[commit-bot: I haz the power, 2017-03-28 00:07:47]

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 00:16:07]

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

[commit-bot: I haz the power, 2017-03-28 00:16:08]

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-28 00:23:06]

slangley@chromium.org changed reviewers:
+ dpapad@chromium.org, mkwst@chromium.org

[slangley, 2017-03-28 00:23:07]

+mkwst@ for content/shell OWNERS
+dpapad@ for components/printing/renderer OWNERS

[dpapad, 2017-03-28 01:19:42]

dpapad@chromium.org changed reviewers:
+ thestig@chromium.org
- dpapad@chromium.org

[dpapad, 2017-03-28 01:20:08]

On 2017/03/28 at 00:23:07, slangley wrote:
> +mkwst@ for content/shell OWNERS
> +dpapad@ for components/printing/renderer OWNERS

Added thesting@ for printing/.

[slangley, 2017-03-28 01:50:41]

The CQ bit was checked by slangley@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-28 01:51:01]

Dry run: 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 03:20:02]

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

[commit-bot: I haz the power, 2017-03-28 03:20:04]

Dry run: This issue passed the CQ dry run.

[Mike West, 2017-03-28 10:11:37]

//content/shell LGTM.

[Lei Zhang, 2017-03-28 17:41:54]

lgtm assuming this doesn't change the blink::WebFrameWidget::create() behavior
w.r.t. printing.

[slangley, 2017-03-28 22:57:59]

The CQ bit was checked by slangley@chromium.org

[slangley, 2017-03-28 22:57:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2773003002/#ps80001
(title: "WebViewClient dtor now overrides the virtual dtor in WebWidgetClient.")

[commit-bot: I haz the power, 2017-03-28 22:58:41]

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:20:17]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1490741879422530,
"parent_rev": "aa93df3943c70cef817a33f3b2fdb377598774ba", "commit_rev":
"9df2c54436a76ebaf6bfcc317815e776e4a0a7f5"}

[commit-bot: I haz the power, 2017-03-28 23:21:51]

Description was changed from

==========
Ensure that the WebWidgetClient cannot be a nullptr by adding DCHECK()s.

There was only one call site in print_web_view_helper where a nullptr was being
passed instead of a valid WebWidgetClient, to fix this the following changes
were also made:

* Make WebWidgetClient dtor public so the class can be instantiated.
* Pass a valid WebWidgetClient in the print_web_view_helper

A subsequent CL will analyze and remove m_client null checks in
WebFrameWidgetImpl.

I left WebFrameViewClient as a ref as this appears to be internal to blink and
will adhere to blink standards.

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

to

==========
Ensure that the WebWidgetClient cannot be a nullptr by adding DCHECK()s.

There was only one call site in print_web_view_helper where a nullptr was being
passed instead of a valid WebWidgetClient, to fix this the following changes
were also made:

* Make WebWidgetClient dtor public so the class can be instantiated.
* Pass a valid WebWidgetClient in the print_web_view_helper

A subsequent CL will analyze and remove m_client null checks in
WebFrameWidgetImpl.

I left WebFrameViewClient as a ref as this appears to be internal to blink and
will adhere to blink standards.

BUG=696895

Review-Url: https://codereview.chromium.org/2773003002
Cr-Commit-Position: refs/heads/master@{#460235}
Committed:
https://chromium.googlesource.com/chromium/src/+/9df2c54436a76ebaf6bfcc317815...
==========

[commit-bot: I haz the power, 2017-03-28 23:21:53]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/9df2c54436a76ebaf6bfcc317815...