New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 612069 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug-Regression


Sign in to add a comment

arm (peach, veyron, oak): thumbnails on NTP corrupted; page captured from extension corrupted

Project Member Reported by djkurtz@chromium.org, May 16 2016

Issue description

Originally reported as:
 http://crosbug.com/p/52346

Chrome Version: 52.0.2734.0
Chrome OS Version: 8328.0.0
Chrome OS Platform: peach_pi, veyron_minnie, oak

Steps To Reproduce issue:
(1) Log in as a (test) user
(2) close all tabs and windows
(3) navigate to chrome://history:
(4) clear [Browsing history ; Download history ; Cached images and files]
(5) log out
(6) log back in
(7) browse to any page, eg:
  https://en.wikipedia.org/wiki/List_of_law_clerks_of_the_Supreme_Court_of_the_United_States
(8) open new tab page
(9) Open chrome://thumbnails
(10) Install the "Awesome Screenshot" extension:
 https://chrome.google.com/webstore/detail/awesome-screenshot-screen/nlipoenfbbikpbjkfpfillcgkoblgpmj
(10.a) [Googlers w/ Enterprise Enrolled Chromebooks can also repro w/ go/snipit: 
 https://chrome.google.com/webstore/detail/snipit/ehoadneljpdggcbbknedodolkkjodefl ]
(11) Navigate to any page, eg:
  https://en.wikipedia.org/wiki/List_of_law_clerks_of_the_Supreme_Court_of_the_United_States
(12) Press "Ctrl-Shift-E" to take a snapshot of page contents

Expected Result:

(8) => Accurate thumbnail of page (7)
(9) => Accurate thumbnail of page (7)
(12) => Accurate screenshot of page (11)


Actual Result:

(8) => Corrupted thumbnail of page from (7)
(9) => Corrupted thumbnail of page from (7)
(12) => Corrupted screenshot of page (11)

How frequently does this problem reproduce? (Always, sometimes, hard to
reproduce?)

Always

What is the impact to the user, and is there a workaround? If so, what is
it?

Some thumbnails on New Tab Page are corrupted.
Cannot use extensions to capture tab content.

Please provide any additional information below. Attach a screen shot or
log if possible.

From /var/log/ui/ui.LATEST:

[20251:20251:0516/112905:ERROR:gles2_cmd_decoder.cc(2236)] [.CompositorWorker-0xb8f3d400]GL ERROR :GL_INVALID_OPERATION : GLES2DecoderImpl::DoBindTexImage2DCHROMIUM: <- error from previous GL command


Note: this issue reproduces on at least peach, veyron and oak.
Note: for screenshots and more analysis, see original report: http://crosbug.com/p/52346
 
It turns out the GL error is caused by the GLES driver rejecting Chrome's glReadPixels() call to fetch pixels for the extension/thumbnail.  In fact the GLES library doesn't like the parameters we are passing, and just errors out.  Chrome does not do any error checking near this glReadPixels, and hence it happily processes garbage pixels.  However, at some unlucky point later, some code checks glGetError(), which reports that at some previous time, there had been a GL_INVALID_OPERATION.  In the trace above, the unlucky function was 
doBindTexImage2DCHROMIUM.

This is what a call looks like:
glReadPixels(0, 0, 424, 284, GL_BGRA_EXT, GL_UNSIGNED_BYTE, 0);

This call, and its format are ultimately initiated :

(a) for tab capture extensions:
bool WebContentsCaptureClient::CaptureAsync(
...
  host->CopyFromBackingStore(gfx::Rect(view_size), bitmap_size, callback,
                             kN32_SkColorType);

(b) for thumbnails:

void ThumbnailTabHelper::AsyncProcessThumbnail(
...
  render_widget_host->CopyFromBackingStore(
      copy_rect,
      thumbnailing_context_->requested_copy_size,
      base::Bind(&ThumbnailTabHelper::ProcessCapturedBitmap,
                 weak_factory_.GetWeakPtr(),
                 algorithm),
      kN32_SkColorType);


In both cases, the color format is: kN32_SkColorType, which is defined as:

#if SK_PMCOLOR_BYTE_ORDER(B,G,R,A)
    kN32_SkColorType = kBGRA_8888_SkColorType,
#elif SK_PMCOLOR_BYTE_ORDER(R,G,B,A)
    kN32_SkColorType = kRGBA_8888_SkColorType,
#else
    #error "SK_*32_SHFIT values must correspond to BGRA or RGBA byte order"
#endif

For non Android builds, in skia/config/SkUserConfig.h Chromium sets:

#if !defined(ANDROID)   // On Android, we use the skia default settings.
#define SK_A32_SHIFT    24
#define SK_R32_SHIFT    16
#define SK_G32_SHIFT    8
#define SK_B32_SHIFT    0
#endif

For arm SK_CPU_LENDIAN is defined, so:

#ifdef SK_CPU_BENDIAN
#  define SK_PMCOLOR_BYTE_ORDER(C0, C1, C2, C3)     \
        (SK_ ## C3 ## 32_SHIFT == 0  &&             \
         SK_ ## C2 ## 32_SHIFT == 8  &&             \
         SK_ ## C1 ## 32_SHIFT == 16 &&             \
         SK_ ## C0 ## 32_SHIFT == 24)
#else
#  define SK_PMCOLOR_BYTE_ORDER(C0, C1, C2, C3)     \
        (SK_ ## C0 ## 32_SHIFT == 0  &&             \
         SK_ ## C1 ## 32_SHIFT == 8  &&             \
         SK_ ## C2 ## 32_SHIFT == 16 &&             \
         SK_ ## C3 ## 32_SHIFT == 24)
#endif


Therefore, SK_PMCOLOR_BYTE_ORDER(B,G,R,A) == true, 
and kN32_SkColorType = kBGRA_8888_SkColorType

CopyFromBackingStore() eventually winds its way down to CopyFromCompositingSurface(), which has a somewhat misleading comment (since kN32_SkColorType is not necessarily ARGB888):

void DelegatedFrameHost::CopyFromCompositingSurface(
    const gfx::Rect& src_subrect,
    const gfx::Size& output_size,
    const ReadbackRequestCallback& callback,
    const SkColorType preferred_color_type) {
  // Only ARGB888 and RGB565 supported as of now.
  bool format_support = ((preferred_color_type == kAlpha_8_SkColorType) ||
                         (preferred_color_type == kRGB_565_SkColorType) ||
                         (preferred_color_type == kN32_SkColorType));

This format winds its way through many layers, eventually getting converted to a gles format:

void GLHelper::CopyTextureToImpl::CropScaleReadbackAndCleanTexture(
{
...
  FormatSupport supported = GetReadbackConfig(readback_color_type, true,
                                              &format, &type, &bytes_per_pixel);

...
    case kBGRA_8888_SkColorType:
      *format = GL_BGRA_EXT;


So:
 (1) Chrome overrides Skia's default settings to use BGRA instead of RGBA
 (2) Chrome then tries to use GL_BGRA_EXT for glReadPixels()
 (3) The GLES drivers reject GL_BGRA_EXT for glReadPixels()
 (4) Chrome doesn't check for errors reported by the driver
 (5) Chrome detects the GL error later, but doesn't know where it came from, so just prints and otherwise ignores it
 (5) Chrome uses whatever garbage was in the buffer when glReadPixel() fails as the thumbnail/screenshot

Two questions remain:
 (1) Why are the ARM GLES drivers rejecting glReadPixels() w/ format = GL_BGRA_EXT?
 (2) This used to work..  What introduced this regression?
Blockedon: chrome-os-partner:52346 chrome-os-partner:53381
Mali specific bug opened here: crosbug.com/p/53381
Cc: siev...@chromium.org
Labels: OS-All
Labels: -OS-All

Comment 5 by aa...@rumbold.me.uk, May 19 2016

This is also affecting the current BETA release of Chrome 51, running on ARM Platform
Status: Fixed (was: Started)
Fixed on oak as of 8341.0.0
Fixed on daisy/peach/veyron as of 8350.0.0.
Labels: VerifyIn-53
Labels: VerifyIn-54

Comment 9 by ka...@chromium.org, Aug 31 2016

Labels: Bulk-Verified
Status: Verified (was: Fixed)
Components: -Internals>Graphics Internals>GPU
Moving old issues out of Internal>Graphics to delete this obsolete component ( crbug.com/685425  for details)

Sign in to add a comment