Change assert() to DCHECK() in scoped_ptr.h

Change assert() to DCHECK() in scoped_ptr.h

The motivation for making this change is that Chromium's CQ tests run in
release mode with DCHECK() turned on but assert() compiled out. For more
context, see
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/IIkOmt66zEc.

[blundell, 2015-09-03 16:14:35]

blundell@chromium.org changed reviewers:
+ danakj@chromium.org, dcheng@chromium.org

[blundell, 2015-09-03 17:04:18]

Whoops, turns out it's not that simple. Will look at failures tomorrow.

[dcheng, 2015-09-03 17:15:10]

As I recall, Blink pulls in this header. Pulling in logging.h into this
header causes colliding macro definitions. That's why I used assert.

On Thu, Sep 3, 2015, 10:04  <blundell@chromium.org> wrote:

> Whoops, turns out it's not that simple. Will look at failures tomorrow.
>
> https://codereview.chromium.org/1305973011/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[danakj, 2015-09-03 17:16:27]

https://codereview.chromium.org/1305973011/diff/1/base/memory/scoped_ptr.h
File base/memory/scoped_ptr.h (right):

https://codereview.chromium.org/1305973011/diff/1/base/memory/scoped_ptr.h#ne...
base/memory/scoped_ptr.h:91: #include "base/logging.h"
This will include logging in a lot of places. Does LOG still conflict with blink
macros?

[danakj, 2015-09-03 17:57:26]

On 2015/09/03 17:16:27, danakj wrote:
> https://codereview.chromium.org/1305973011/diff/1/base/memory/scoped_ptr.h
> File base/memory/scoped_ptr.h (right):
> 
>
https://codereview.chromium.org/1305973011/diff/1/base/memory/scoped_ptr.h#ne...
> base/memory/scoped_ptr.h:91: #include "base/logging.h"
> This will include logging in a lot of places. Does LOG still conflict with
blink
> macros?

Oh apparently that is fixed :D. And one of the scoped_ptr unit tests DCHECKs
lol.

[blundell, 2015-09-04 09:14:43]

On 2015/09/03 17:57:26, danakj wrote:
> On 2015/09/03 17:16:27, danakj wrote:
> > https://codereview.chromium.org/1305973011/diff/1/base/memory/scoped_ptr.h
> > File base/memory/scoped_ptr.h (right):
> > 
> >
>
https://codereview.chromium.org/1305973011/diff/1/base/memory/scoped_ptr.h#ne...
> > base/memory/scoped_ptr.h:91: #include "base/logging.h"
> > This will include logging in a lot of places. Does LOG still conflict with
> blink
> > macros?
> 
> Oh apparently that is fixed :D. And one of the scoped_ptr unit tests DCHECKs
> lol.

As I added to the chromium-dev@ thread, I think that in the end this patch might
still not be workable due to it adding a dependency from //third_party/WebKit to
//base.

[danakj, 2015-09-04 18:07:15]

On Fri, Sep 4, 2015 at 2:14 AM, <blundell@chromium.org> wrote:

> On 2015/09/03 17:57:26, danakj wrote:
>
>> On 2015/09/03 17:16:27, danakj wrote:
>> >
>> https://codereview.chromium.org/1305973011/diff/1/base/memory/scoped_ptr.h
>> > File base/memory/scoped_ptr.h (right):
>> >
>> >
>>
>
>
>
https://codereview.chromium.org/1305973011/diff/1/base/memory/scoped_ptr.h#ne...
>
>> > base/memory/scoped_ptr.h:91: #include "base/logging.h"
>> > This will include logging in a lot of places. Does LOG still conflict
>> with
>> blink
>> > macros?
>>
>
> Oh apparently that is fixed :D. And one of the scoped_ptr unit tests
>> DCHECKs
>> lol.
>>
>
> As I added to the chromium-dev@ thread, I think that in the end this
> patch might
> still not be workable due to it adding a dependency from
> //third_party/WebKit to
> //base.
>

Blink shouldn't be using base, Jeremy has nicely removed that use of
scoped_ptr in blink.


>
> https://codereview.chromium.org/1305973011/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.