Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2017923002:
Unbreak release component builds on Windows after https://codereview.chromium.org/2014343002/ (Closed)
|
Created:
4 years, 6 months ago by Nico Modified:
4 years, 6 months ago Reviewers:
Raymond Toy CC:
chromium-reviews, blink-reviews, haraken, hongchan, Raymond Toy Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionUnbreak release component builds on Windows after https://codereview.chromium.org/2014343002/
DCHECK expands to
#define DCHECK(condition) \
LAZY_STREAM(LOG_STREAM(DCHECK), DCHECK_IS_ON() ? !(condition) : false) \
<< "Check failed: " #condition ". "
which requires calls in condition to exist even in non-dcheck builds,
they're just stripped later because DCHECK_IS_ON() is 0 there.
Hence, https://codereview.chromium.org/2014343002/ moved
isGraphOwner() out of ENABLE(ASSERT), but forgot to do the same
in the cpp file. This causes link errors. However, removing the macro
there too requires removing it in a few other places too.
To unbreak bots and devs, just replace the DCHECKs added in that CL with
ASSERTs for now.
BUG=615093, 613332
TBR=rtoy
Committed: https://crrev.com/c117666ede47a2ee7c7e403ef43b3718b9d7c716
Cr-Commit-Position: refs/heads/master@{#396462}
Patch Set 1 #Patch Set 2 : . #Patch Set 3 : hmm2 #
Messages
Total messages: 24 (10 generated)
The CQ bit was checked by thakis@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2017923002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2017923002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
Description was changed from ========== Unbreak release component builds on Windows after https://codereview.chromium.org/2014343002/ BUG=615093, 613332 ========== to ========== Unbreak release component builds on Windows after https://codereview.chromium.org/2014343002/ DCHECK expands to #define DCHECK(condition) \ LAZY_STREAM(LOG_STREAM(DCHECK), DCHECK_IS_ON() ? !(condition) : false) \ << "Check failed: " #condition ". " which requires calls in condition to exist even in non-dcheck builds, they're just stripped later because DCHECK_IS_ON() is 0 there. Hence, https://codereview.chromium.org/2014343002/ moved isGraphOwner() out of ENABLE(ASSERT), but forgot to do the same in the cpp file. BUG=615093, 613332 ==========
Description was changed from ========== Unbreak release component builds on Windows after https://codereview.chromium.org/2014343002/ DCHECK expands to #define DCHECK(condition) \ LAZY_STREAM(LOG_STREAM(DCHECK), DCHECK_IS_ON() ? !(condition) : false) \ << "Check failed: " #condition ". " which requires calls in condition to exist even in non-dcheck builds, they're just stripped later because DCHECK_IS_ON() is 0 there. Hence, https://codereview.chromium.org/2014343002/ moved isGraphOwner() out of ENABLE(ASSERT), but forgot to do the same in the cpp file. BUG=615093, 613332 ========== to ========== Unbreak release component builds on Windows after https://codereview.chromium.org/2014343002/ DCHECK expands to #define DCHECK(condition) \ LAZY_STREAM(LOG_STREAM(DCHECK), DCHECK_IS_ON() ? !(condition) : false) \ << "Check failed: " #condition ". " which requires calls in condition to exist even in non-dcheck builds, they're just stripped later because DCHECK_IS_ON() is 0 there. Hence, https://codereview.chromium.org/2014343002/ moved isGraphOwner() out of ENABLE(ASSERT), but forgot to do the same in the cpp file. BUG=615093, 613332 TBR=rtoy ==========
thakis@chromium.org changed reviewers: + rtoy@chromium.org
The CQ bit was checked by thakis@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2017923002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2017923002/20001
ah hmm, this doesn't build either:
../../third_party/WebKit/Source/modules/webaudio/DeferredTaskHandler.cpp:73:32:
error: 'class WTF::RecursiveMutex' has no member named 'locked'
return m_contextGraphMutex.locked();
^
because of
#if ENABLE(ASSERT)
bool locked() { return m_mutex.m_recursionCount > 0; }
#endif
in ThreadingPrimitives.h.
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
Description was changed from ========== Unbreak release component builds on Windows after https://codereview.chromium.org/2014343002/ DCHECK expands to #define DCHECK(condition) \ LAZY_STREAM(LOG_STREAM(DCHECK), DCHECK_IS_ON() ? !(condition) : false) \ << "Check failed: " #condition ". " which requires calls in condition to exist even in non-dcheck builds, they're just stripped later because DCHECK_IS_ON() is 0 there. Hence, https://codereview.chromium.org/2014343002/ moved isGraphOwner() out of ENABLE(ASSERT), but forgot to do the same in the cpp file. BUG=615093, 613332 TBR=rtoy ========== to ========== Unbreak release component builds on Windows after https://codereview.chromium.org/2014343002/ DCHECK expands to #define DCHECK(condition) \ LAZY_STREAM(LOG_STREAM(DCHECK), DCHECK_IS_ON() ? !(condition) : false) \ << "Check failed: " #condition ". " which requires calls in condition to exist even in non-dcheck builds, they're just stripped later because DCHECK_IS_ON() is 0 there. Hence, https://codereview.chromium.org/2014343002/ moved isGraphOwner() out of ENABLE(ASSERT), but forgot to do the same in the cpp file. This causes link errors. However, removing the macro there too requires removing it in a few other places too. To unbreak bots and devs, just replace the DCHECKs added in that CL with ASSERTs for now. BUG=615093, 613332 TBR=rtoy ==========
ok, ptal ps 3
Go ahead and revert the original CL. I'll fix it up better. On May 27, 2016 6:57 AM, <thakis@chromium.org> wrote: > ah hmm, this doesn't build either: > > > ../../third_party/WebKit/Source/modules/webaudio/DeferredTaskHandler.cpp:73:32: > error: 'class WTF::RecursiveMutex' has no member named 'locked' > return m_contextGraphMutex.locked(); > ^ > > because of > > #if ENABLE(ASSERT) > bool locked() { return m_mutex.m_recursionCount > 0; } > #endif > > in ThreadingPrimitives.h. > > https://codereview.chromium.org/2017923002/ > -- 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.
Go ahead and revert the original CL. I'll fix it up better. On May 27, 2016 6:57 AM, <thakis@chromium.org> wrote: > ah hmm, this doesn't build either: > > > ../../third_party/WebKit/Source/modules/webaudio/DeferredTaskHandler.cpp:73:32: > error: 'class WTF::RecursiveMutex' has no member named 'locked' > return m_contextGraphMutex.locked(); > ^ > > because of > > #if ENABLE(ASSERT) > bool locked() { return m_mutex.m_recursionCount > 0; } > #endif > > in ThreadingPrimitives.h. > > https://codereview.chromium.org/2017923002/ > -- 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.
Oh. Patch 3 lgtm. On May 27, 2016 7:01 AM, <thakis@chromium.org> wrote: > ok, ptal ps 3 > > https://codereview.chromium.org/2017923002/ > -- 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.
Oh. Patch 3 lgtm. On May 27, 2016 7:01 AM, <thakis@chromium.org> wrote: > ok, ptal ps 3 > > https://codereview.chromium.org/2017923002/ > -- 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.
lgtm
Description was changed from ========== Unbreak release component builds on Windows after https://codereview.chromium.org/2014343002/ DCHECK expands to #define DCHECK(condition) \ LAZY_STREAM(LOG_STREAM(DCHECK), DCHECK_IS_ON() ? !(condition) : false) \ << "Check failed: " #condition ". " which requires calls in condition to exist even in non-dcheck builds, they're just stripped later because DCHECK_IS_ON() is 0 there. Hence, https://codereview.chromium.org/2014343002/ moved isGraphOwner() out of ENABLE(ASSERT), but forgot to do the same in the cpp file. This causes link errors. However, removing the macro there too requires removing it in a few other places too. To unbreak bots and devs, just replace the DCHECKs added in that CL with ASSERTs for now. BUG=615093, 613332 TBR=rtoy ========== to ========== Unbreak release component builds on Windows after https://codereview.chromium.org/2014343002/ DCHECK expands to #define DCHECK(condition) \ LAZY_STREAM(LOG_STREAM(DCHECK), DCHECK_IS_ON() ? !(condition) : false) \ << "Check failed: " #condition ". " which requires calls in condition to exist even in non-dcheck builds, they're just stripped later because DCHECK_IS_ON() is 0 there. Hence, https://codereview.chromium.org/2014343002/ moved isGraphOwner() out of ENABLE(ASSERT), but forgot to do the same in the cpp file. This causes link errors. However, removing the macro there too requires removing it in a few other places too. To unbreak bots and devs, just replace the DCHECKs added in that CL with ASSERTs for now. BUG=615093, 613332 TBR=rtoy ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) to pending queue manually as 61bd11a05087e41ec5ecf03fe2d72f07a2377978 (presubmit successful).
Message was sent while issue was closed.
Description was changed from ========== Unbreak release component builds on Windows after https://codereview.chromium.org/2014343002/ DCHECK expands to #define DCHECK(condition) \ LAZY_STREAM(LOG_STREAM(DCHECK), DCHECK_IS_ON() ? !(condition) : false) \ << "Check failed: " #condition ". " which requires calls in condition to exist even in non-dcheck builds, they're just stripped later because DCHECK_IS_ON() is 0 there. Hence, https://codereview.chromium.org/2014343002/ moved isGraphOwner() out of ENABLE(ASSERT), but forgot to do the same in the cpp file. This causes link errors. However, removing the macro there too requires removing it in a few other places too. To unbreak bots and devs, just replace the DCHECKs added in that CL with ASSERTs for now. BUG=615093, 613332 TBR=rtoy ========== to ========== Unbreak release component builds on Windows after https://codereview.chromium.org/2014343002/ DCHECK expands to #define DCHECK(condition) \ LAZY_STREAM(LOG_STREAM(DCHECK), DCHECK_IS_ON() ? !(condition) : false) \ << "Check failed: " #condition ". " which requires calls in condition to exist even in non-dcheck builds, they're just stripped later because DCHECK_IS_ON() is 0 there. Hence, https://codereview.chromium.org/2014343002/ moved isGraphOwner() out of ENABLE(ASSERT), but forgot to do the same in the cpp file. This causes link errors. However, removing the macro there too requires removing it in a few other places too. To unbreak bots and devs, just replace the DCHECKs added in that CL with ASSERTs for now. BUG=615093, 613332 TBR=rtoy Committed: https://crrev.com/c117666ede47a2ee7c7e403ef43b3718b9d7c716 Cr-Commit-Position: refs/heads/master@{#396462} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/c117666ede47a2ee7c7e403ef43b3718b9d7c716 Cr-Commit-Position: refs/heads/master@{#396462} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
