Revert of tryAllocPixels returns bool, allocPixels requires success

Revert of tryAllocPixels returns bool, allocPixels requires success (patchset #6 id:100001 of https://codereview.chromium.org/545513002/)

Reason for revert:
Broke Android Tests (dbg)

C  427.574s Main  [FAIL] org.chromium.content.browser.WebContentsObserverAndroidTest#testDidFirstVisuallyNonEmptyPaint:
C  427.574s Main  java.util.concurrent.TimeoutException: waitForCallback timed out!
C  427.574s Main  	at org.chromium.content.browser.test.util.CallbackHelper.waitForCallback(CallbackHelper.java:183)
C  427.574s Main  	at org.chromium.content.browser.test.util.CallbackHelper.waitForCallback(CallbackHelper.java:191)
C  427.574s Main  	at org.chromium.content.browser.test.util.CallbackHelper.waitForCallback(CallbackHelper.java:197)
C  427.574s Main  	at org.chromium.content.browser.WebContentsObserverAndroidTest.testDidFirstVisuallyNonEmptyPaint(WebContentsObserverAndroidTest.java:70)
C  427.574s Main  	at java.lang.reflect.Method.invokeNative(Native Method)
C  427.574s Main  	at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:214)
C  427.574s Main  	at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:199)
C  427.574s Main  	at android.test.ActivityInstrumentationTestCase2.runTest(ActivityInstrumentationTestCase2.java:192)
C  427.574s Main  	at org.chromium.content_shell_apk.ContentShellTestBase.runTest(ContentShellTestBase.java:227)
C  427.574s Main  	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:191)
C  427.574s Main  	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:176)
C  427.574s Main  	at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:554)
C  427.575s Main  	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1701)

Original issue's description:
> tryAllocPixels returns bool, allocPixels requires success
> 
> depends on blink CL https://codereview.chromium.org/540033002/
> 
> TBR=jochen
> 
> Committed: https://chromium.googlesource.com/chromium/src/+/c2e4854d5092fd31e295fdfe06c2463f00c3b0ba

TBR=danakj@chromium.org,jochen@chromium.org,reed@chromium.org,reed@google.com
NOTREECHECKS=true
NOTRY=true

[arv (Not doing code reviews), 2014-09-08 15:37:17]

Created Revert of tryAllocPixels returns bool, allocPixels requires success

[arv (Not doing code reviews), 2014-09-08 15:37:49]

arv@chromium.org changed reviewers:
+ keishi@chromium.org

[arv (Not doing code reviews), 2014-09-08 15:37:50]

FYI keishi

[commit-bot: I haz the power, 2014-09-08 15:38:09]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/arv@chromium.org/556463002/1

[commit-bot: I haz the power, 2014-09-08 15:38:55]

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

[commit-bot: I haz the power, 2014-09-08 15:38:56]

Failed to apply patch for skia/config/SkUserConfig.h:
While running git apply --index -p1;
  error: patch failed: skia/config/SkUserConfig.h:257
  error: skia/config/SkUserConfig.h: patch does not apply

Patch:       skia/config/SkUserConfig.h
Index: skia/config/SkUserConfig.h
diff --git a/skia/config/SkUserConfig.h b/skia/config/SkUserConfig.h
index
c45b8533ba184a50bf6a71736d4b62fac4b5d695..78dd44596c024cd580076c908231128d29733f2c
100644
--- a/skia/config/SkUserConfig.h
+++ b/skia/config/SkUserConfig.h
@@ -257,8 +257,8 @@
     #define SK_SUPPORT_LEGACY_GETDEVICE
 #endif
 
-#ifdef SK_SUPPORT_LEGACY_ALLOCPIXELS_BOOL
-    #undef SK_SUPPORT_LEGACY_ALLOCPIXELS_BOOL
+#ifndef SK_SUPPORT_LEGACY_ALLOCPIXELS_BOOL
+    #define SK_SUPPORT_LEGACY_ALLOCPIXELS_BOOL
 #endif
 
 // Turns SkPicture::clone() into a simple "return SkRef(this);" as a way to

[arv (Not doing code reviews), 2014-09-08 15:47:47]

On 2014/09/08 15:38:56, I haz the power (commit-bot) wrote:
> Failed to apply patch for skia/config/SkUserConfig.h:

I'll disable the test instead then.

[arv (Not doing code reviews), 2014-09-08 15:59:20]

On 2014/09/08 15:47:47, arv wrote:
> On 2014/09/08 15:38:56, I haz the power (commit-bot) wrote:
> > Failed to apply patch for skia/config/SkUserConfig.h:
> 
> I'll disable the test instead then.

https://codereview.chromium.org/554713002/

[reed1, 2014-09-08 18:26:54]

Do we think that possibly the change here is problematic?

content/browser/android/system_ui_resource_manager_impl.cc : 43

I interpreted LOGFATAL to be the same as "crash", which is what allocPixels now
will do if it fails. If my assumption is wrong, then I can certainly revert this
one-line change, and just call "tryAllocPixels" instead...

[danakj, 2014-09-08 19:07:17]

LOG(FATAL) does indeed crash.


On Mon, Sep 8, 2014 at 2:26 PM, <reed@google.com> wrote:

> Do we think that possibly the change here is problematic?
>
> content/browser/android/system_ui_resource_manager_impl.cc : 43
>
> I interpreted LOGFATAL to be the same as "crash", which is what
> allocPixels now
> will do if it fails. If my assumption is wrong, then I can certainly
> revert this
> one-line change, and just call "tryAllocPixels" instead...
>
> https://codereview.chromium.org/556463002/
>

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

[chromium-reviews, 2014-09-08 20:59:00]

ah, ok, good.


On Mon, Sep 8, 2014 at 3:06 PM, Dana Jansens <danakj@chromium.org> wrote:

> LOG(FATAL) does indeed crash.
>
>
> On Mon, Sep 8, 2014 at 2:26 PM, <reed@google.com> wrote:
>
>> Do we think that possibly the change here is problematic?
>>
>> content/browser/android/system_ui_resource_manager_impl.cc : 43
>>
>> I interpreted LOGFATAL to be the same as "crash", which is what
>> allocPixels now
>> will do if it fails. If my assumption is wrong, then I can certainly
>> revert this
>> one-line change, and just call "tryAllocPixels" instead...
>>
>> https://codereview.chromium.org/556463002/
>>
>
>

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