Add missing DCHECK to validate HR.

Add missing DCHECK to validate HR.

This issue was found by a linter.

The value of 'hr' is ignored. Probably a nice idea to validate the CreateInstance success.

R=amit@chromium.org,tommi@chromium.org
BUG=

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

[etienneb, 2013-08-02 18:29:07]

PTAL.

[robertshield, 2013-08-02 18:35:42]

https://chromiumcodereview.appspot.com/21948002/diff/3001/chrome_frame/urlmon...
File chrome_frame/urlmon_bind_status_callback.cc (right):

https://chromiumcodereview.appspot.com/21948002/diff/3001/chrome_frame/urlmon...
chrome_frame/urlmon_bind_status_callback.cc:323: if (hr != S_OK)
FAILED(hr)

[tommi (sloooow) - chröme, 2013-08-04 12:43:12]

https://chromiumcodereview.appspot.com/21948002/diff/3001/chrome_frame/urlmon...
File chrome_frame/urlmon_bind_status_callback.cc (right):

https://chromiumcodereview.appspot.com/21948002/diff/3001/chrome_frame/urlmon...
chrome_frame/urlmon_bind_status_callback.cc:323: if (hr != S_OK)
On 2013/08/02 18:35:42, robertshield wrote:
> FAILED(hr)

No chance of S_FALSE?

https://chromiumcodereview.appspot.com/21948002/diff/3001/chrome_frame/urlmon...
File chrome_frame/urlmon_moniker.cc (right):

https://chromiumcodereview.appspot.com/21948002/diff/3001/chrome_frame/urlmon...
chrome_frame/urlmon_moniker.cc:246: DCHECK(SUCCEEDED(hr));
I think this is equivalent to checking the return value of new(), so I don't
think this is needed.

[etienneb, 2013-08-05 13:56:26]

so, what do you think?

https://codereview.chromium.org/21948002/diff/3001/chrome_frame/urlmon_bind_s...
File chrome_frame/urlmon_bind_status_callback.cc (right):

https://codereview.chromium.org/21948002/diff/3001/chrome_frame/urlmon_bind_s...
chrome_frame/urlmon_bind_status_callback.cc:323: if (hr != S_OK)
I don't think S_FALSE can be here.
At some place, the returns value of MayPlayBack is just ignored.

  if (!is_interesting) {
    // Play back report progress so far.
    MayPlayBack(flags);
    return CallbackImpl::OnDataAvailable(flags, size, format_etc, stgmed);
  }

On 2013/08/04 12:43:12, tommi wrote:
> On 2013/08/02 18:35:42, robertshield wrote:
> > FAILED(hr)
> 
> No chance of S_FALSE?

https://codereview.chromium.org/21948002/diff/3001/chrome_frame/urlmon_monike...
File chrome_frame/urlmon_moniker.cc (right):

https://codereview.chromium.org/21948002/diff/3001/chrome_frame/urlmon_monike...
chrome_frame/urlmon_moniker.cc:246: DCHECK(SUCCEEDED(hr));
I that case, why assign it to 'hr'?
The linter complains that 'hr' is assigned twice.

On 2013/08/04 12:43:12, tommi wrote:
> I think this is equivalent to checking the return value of new(), so I don't
> think this is needed.

[tommi (sloooow) - chröme, 2013-08-05 15:10:54]

lgtm

https://codereview.chromium.org/21948002/diff/3001/chrome_frame/urlmon_monike...
File chrome_frame/urlmon_moniker.cc (right):

https://codereview.chromium.org/21948002/diff/3001/chrome_frame/urlmon_monike...
chrome_frame/urlmon_moniker.cc:246: DCHECK(SUCCEEDED(hr));
On 2013/08/05 13:56:26, etienneb wrote:
> I that case, why assign it to 'hr'?
> The linter complains that 'hr' is assigned twice.
> 
> On 2013/08/04 12:43:12, tommi wrote:
> > I think this is equivalent to checking the return value of new(), so I don't
> > think this is needed.
> 

Keeping the DCHECK is ok with me, I just don't think it's necessary.  You can
either skip assigning to hr or add the DCHECK. Up to you.

[commit-bot: I haz the power, 2013-08-20 05:34:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/etienneb@chromium.org/21948002/17001

[commit-bot: I haz the power, 2013-08-21 03:17:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/etienneb@chromium.org/21948002/17001

[commit-bot: I haz the power, 2013-08-21 06:47:37]

Change committed as 218662