New issue
Advanced search Search tips

Issue 608895 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: May 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Chrome OS Crashes When Programmatically Changing Wallpaper with Chrome.wallpaper API

Reported by dr.steph...@gmail.com, May 3 2016

Issue description

UserAgent: Mozilla/5.0 (X11; CrOS armv7l 7978.66.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/50.0.2661.91 Safari/537.36
Platform: 7978.66.0 (Official Build) stable-channel nyan_big

Steps to reproduce the problem:
1. Run this function in an extension:
function test() {
  chrome.wallpaper.setWallpaper(
    {
      'url': "http://apod.nasa.gov/apod/image/1605/AuroraSweden_Strand_1500.jpg",
      'layout': "CENTER_CROPPED",
      'filename': 'apod_wallpaper',
      'thumbnail': true
    }, 
    function(thumb) {
      chrome.notifications.create(
        "",
        {
          "type" : "image",
          "title" : "Some Text",
          "message" : "Some More Text",
          "iconUrl" : "/assets/icon_128.png",
          "imageUrl" : "data:image/jpeg;base64," + btoa(String.fromCharCode.apply(null, new Uint8Array(thumb))),
          "isClickable" : true
        }
      );
    }
  );
}
2. See Chromebook crash

What is the expected behavior?
ChromeOS wallpaper should change

What went wrong?
ChromeOS crashes and restarts. No error logs, sluggish until full reboot.

Crashed report ID: 576d2a6c00000000

How much crashed? Whole browser

Is it a problem with a plugin? No 

Did this work before? Yes Any time before Chrome 49 (i.e. 43 - 48)

Chrome version: 50.0.2661.91  Channel: stable
OS Version: 7978.66.0
Flash Version: Shockwave Flash 21.0 r0

Limited troubleshooting possible, but it looks like something is cycling and using all the memory. Very careful observation suggests that the crash occurs a few moments after code execution, i.e. not within the function at all.

There is an extension in the web store which uses this:
https://chrome.google.com/webstore/detail/apod-wallpaper-for-chrome/canadhjkgpkaokimkakhinigmigajddg

Anybody installing this will crash their Chromebook.
 
Components: UI>Shell>Wallpaper
Labels: -Pri-2 M-50 Pri-1
Owner: x...@chromium.org
Status: Assigned (was: Unconfirmed)
Project Member

Comment 3 by bugdroid1@chromium.org, May 17 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/72f2683a0d2175b27d43f8fc91357af138a872c4

commit 72f2683a0d2175b27d43f8fc91357af138a872c4
Author: xdai <xdai@chromium.org>
Date: Tue May 17 18:00:19 2016

Fix the browser crash when changing wallpaper with chrome.wallpaper API.

The crash is caused by a pointer ownership issue.

BUG= 608895 

Review-Url: https://codereview.chromium.org/1986583002
Cr-Commit-Position: refs/heads/master@{#394160}

[modify] https://crrev.com/72f2683a0d2175b27d43f8fc91357af138a872c4/chrome/browser/chromeos/extensions/wallpaper_api.cc
[modify] https://crrev.com/72f2683a0d2175b27d43f8fc91357af138a872c4/chrome/test/data/extensions/api_test/wallpaper/test.js

Project Member

Comment 4 by bugdroid1@chromium.org, May 17 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0d83958df92063615a84019f689985c74da54f8b

commit 0d83958df92063615a84019f689985c74da54f8b
Author: xdai <xdai@chromium.org>
Date: Tue May 17 22:51:34 2016

Revert "Fix the browser crash when changing wallpaper with chrome.wallpaper API."

This reverts commit 72f2683a0d2175b27d43f8fc91357af138a872c4.

Revert reason: Detect memory leak in the test ExtensionApiTest.Wallpaper.

BUG= 608895 
TBR=bshe@chromium.org

Review-Url: https://codereview.chromium.org/1984953005
Cr-Commit-Position: refs/heads/master@{#394253}

[modify] https://crrev.com/0d83958df92063615a84019f689985c74da54f8b/chrome/browser/chromeos/extensions/wallpaper_api.cc
[modify] https://crrev.com/0d83958df92063615a84019f689985c74da54f8b/chrome/test/data/extensions/api_test/wallpaper/test.js

Comment 5 by x...@chromium.org, May 17 2016

A retry CL is in the review process https://codereview.chromium.org/1989653002/.
Project Member

Comment 6 by bugdroid1@chromium.org, May 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fffb22c1b808e247ce67cb35793b57ddb35bf1d0

commit fffb22c1b808e247ce67cb35793b57ddb35bf1d0
Author: xdai <xdai@chromium.org>
Date: Fri May 27 06:48:20 2016

[Retry] Fix the browser crash when changing wallpaper with chrome.wallpaper API.

The original CL https://codereview.chromium.org/1986583002/ was reverted in
https://codereview.chromium.org/1984953005/ because of memory leak detected in
the test function getThumbnailAferSetWallpaper().

BUG= 608895 

Review-Url: https://codereview.chromium.org/1989653002
Cr-Commit-Position: refs/heads/master@{#396411}

[modify] https://crrev.com/fffb22c1b808e247ce67cb35793b57ddb35bf1d0/chrome/browser/chromeos/extensions/wallpaper_api.cc
[modify] https://crrev.com/fffb22c1b808e247ce67cb35793b57ddb35bf1d0/chrome/test/data/extensions/api_test/wallpaper/test.js

Comment 7 by x...@chromium.org, May 27 2016

Status: Fixed (was: Assigned)
Status: Verified (was: Fixed)
Not fixed in Version 51.0.2704.79

Comment 10 by x...@chromium.org, Jun 8 2016

The fix is in M53 and later. It's too late to request a merge to M51 now and I can try to request to merge the fix to M52.

Comment 11 by x...@chromium.org, Jun 8 2016

Labels: Merge-Request-52

Comment 12 by tin...@google.com, Jun 8 2016

Labels: -Merge-Request-52 Merge-Approved-52 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M52 (branch: 2743)
Excellent News!
Project Member

Comment 14 by bugdroid1@chromium.org, Jun 10 2016

Labels: -merge-approved-52 merge-merged-2743
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/59e1abe6af3205eea84d0bb6fb3890ca051e6b1d

commit 59e1abe6af3205eea84d0bb6fb3890ca051e6b1d
Author: xdai <xdai@chromium.org>
Date: Fri Jun 10 16:04:40 2016

[Merge to M52] [Retry] Fix the browser crash when changing wallpaper with chrome.wallpaper API.

The original CL https://codereview.chromium.org/1986583002/ was reverted in
https://codereview.chromium.org/1984953005/ because of memory leak detected in
the test function getThumbnailAferSetWallpaper().

BUG= 608895 
TBR=bshe@chromium.org

Review-Url: https://codereview.chromium.org/1989653002
Cr-Commit-Position: refs/heads/master@{#396411}
(cherry picked from commit fffb22c1b808e247ce67cb35793b57ddb35bf1d0)

Review URL: https://codereview.chromium.org/2058833002 .

Cr-Commit-Position: refs/branch-heads/2743@{#315}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/59e1abe6af3205eea84d0bb6fb3890ca051e6b1d/chrome/browser/chromeos/extensions/wallpaper_api.cc
[modify] https://crrev.com/59e1abe6af3205eea84d0bb6fb3890ca051e6b1d/chrome/test/data/extensions/api_test/wallpaper/test.js

Project Member

Comment 15 by bugdroid1@chromium.org, Jun 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/59e1abe6af3205eea84d0bb6fb3890ca051e6b1d

commit 59e1abe6af3205eea84d0bb6fb3890ca051e6b1d
Author: xdai <xdai@chromium.org>
Date: Fri Jun 10 16:04:40 2016

[Merge to M52] [Retry] Fix the browser crash when changing wallpaper with chrome.wallpaper API.

The original CL https://codereview.chromium.org/1986583002/ was reverted in
https://codereview.chromium.org/1984953005/ because of memory leak detected in
the test function getThumbnailAferSetWallpaper().

BUG= 608895 
TBR=bshe@chromium.org

Review-Url: https://codereview.chromium.org/1989653002
Cr-Commit-Position: refs/heads/master@{#396411}
(cherry picked from commit fffb22c1b808e247ce67cb35793b57ddb35bf1d0)

Review URL: https://codereview.chromium.org/2058833002 .

Cr-Commit-Position: refs/branch-heads/2743@{#315}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/59e1abe6af3205eea84d0bb6fb3890ca051e6b1d/chrome/browser/chromeos/extensions/wallpaper_api.cc
[modify] https://crrev.com/59e1abe6af3205eea84d0bb6fb3890ca051e6b1d/chrome/test/data/extensions/api_test/wallpaper/test.js

Sign in to add a comment