New issue
Advanced search Search tips

Issue 800658 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Files: Duplicated notification and a crash under the presence of Incognito browser window

Project Member Reported by yamaguchi@chromium.org, Jan 10 2018

Issue description

Chrome Version: 63.0.3239.11

Steps To Reproduce:
(1) Open Chrome browser.
(2) Open an incognito window of the browser.
(3) Plug a USB drive.
(4) Check the system notification and see "removable device detected" message(s).
(5) Open the files app from (either of) the notification.
(6) Close the incognito browser window.


Expected Result:
(4) See only one notification telling removable device detected.
(6) Nothing else happens.

Actual Result:
(4) There are 2 identical notifications telling removable device detected.
(6) The session crashes. Screen blacks out for a while and comes back with a single browser window.

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?
I also confirmed this happens on the latest dev.
IIUC this has been happening in older versions since the time when it started to notify insertion of a removable device.

Issue 800543 should essentially be caused by the same mechanism.
 
Description: Show this description
Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 12 2018

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

commit e9b2e312171933e64ed390c7b369ae37c98ab219
Author: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Date: Fri Jan 12 06:40:12 2018

Stop Files app in the incognito context to react to device events.

While 1+ incognito browser windows are opened, it'll have separate Files
app extension background and foreground scripts running. The ones in
the incognito context should only be visible by the file picker window
invoked by the browser, but should not show fullscreen app windows or
notifications.

- Prevent duplicated "Removable device detected" notifications
  (when no DCIM folder in it)
- Prevent duplicated Files app windows popup
  (when plugging a disk with /DCIM directory)
- Files app fullscreen mode opened with incognito context (which caused
  some errors), when clicking the "show in folder" link in "Removable
  device detected" notification.
- Other duplicated notifications such as "formatting finished".

Bug:  800658 
Test: browser_tests --gtest_filter=FileManagerJsTest.DeviceHandler*
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I6dc1bc650e1b5cb669e17907c34b680535d0b532
Reviewed-on: https://chromium-review.googlesource.com/861589
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Daichi Hirono <hirono@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528896}
[modify] https://crrev.com/e9b2e312171933e64ed390c7b369ae37c98ab219/ui/file_manager/file_manager/background/js/background.js
[modify] https://crrev.com/e9b2e312171933e64ed390c7b369ae37c98ab219/ui/file_manager/file_manager/background/js/device_handler.js
[modify] https://crrev.com/e9b2e312171933e64ed390c7b369ae37c98ab219/ui/file_manager/file_manager/background/js/device_handler_unittest.js

Status: Fixed (was: Started)
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 15 2018

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

commit ba542af24ceb899798628bd7eb29dddcf37bfb75
Author: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Date: Mon Jan 15 05:37:34 2018

Revert "Stop Files app in the incognito context to react to device events."

This reverts commit e9b2e312171933e64ed390c7b369ae37c98ab219.

Reason for revert: It caused a regression in the Guest mode.

Original change's description:
> Stop Files app in the incognito context to react to device events.
> 
> While 1+ incognito browser windows are opened, it'll have separate Files
> app extension background and foreground scripts running. The ones in
> the incognito context should only be visible by the file picker window
> invoked by the browser, but should not show fullscreen app windows or
> notifications.
> 
> - Prevent duplicated "Removable device detected" notifications
>   (when no DCIM folder in it)
> - Prevent duplicated Files app windows popup
>   (when plugging a disk with /DCIM directory)
> - Files app fullscreen mode opened with incognito context (which caused
>   some errors), when clicking the "show in folder" link in "Removable
>   device detected" notification.
> - Other duplicated notifications such as "formatting finished".
> 
> Bug:  800658 
> Test: browser_tests --gtest_filter=FileManagerJsTest.DeviceHandler*
> Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
> Change-Id: I6dc1bc650e1b5cb669e17907c34b680535d0b532
> Reviewed-on: https://chromium-review.googlesource.com/861589
> Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
> Reviewed-by: Daichi Hirono <hirono@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#528896}

TBR=hirono@chromium.org,yamaguchi@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  800658 
Change-Id: Ifcf3e4996543b7d3a07b8892069d58e5779ff5df
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Reviewed-on: https://chromium-review.googlesource.com/866353
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Daichi Hirono <hirono@chromium.org>
Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529203}
[modify] https://crrev.com/ba542af24ceb899798628bd7eb29dddcf37bfb75/ui/file_manager/file_manager/background/js/background.js
[modify] https://crrev.com/ba542af24ceb899798628bd7eb29dddcf37bfb75/ui/file_manager/file_manager/background/js/device_handler.js
[modify] https://crrev.com/ba542af24ceb899798628bd7eb29dddcf37bfb75/ui/file_manager/file_manager/background/js/device_handler_unittest.js

Status: Assigned (was: Fixed)
Reopening as the fix caused regression; We need to make sure the fix doesn't break the guest mode.
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 30 2018

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

commit c3951527be651a695ea440e684478eec4186a254
Author: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Date: Tue Jan 30 05:09:45 2018

Stop Files app in the incognito context to react to device events.

While 1+ incognito browser windows are opened, it'll have separate Files
app extension background and foreground scripts running. The ones in
the incognito context should only be visible by the file picker window
invoked by the browser, but should not show fullscreen app windows or
notifications.

- Prevent duplicated "Removable device detected" notifications
  (when no DCIM folder in it)
- Prevent duplicated Files app windows popup
  (when plugging a disk with /DCIM directory)
- Files app fullscreen mode opened with incognito context (which caused
  some errors), when clicking the "show in folder" link in "Removable
  device detected" notification.
- Other duplicated notifications such as "formatting finished".

This is reland of crrev.com/c/861589 with some tweak.

Bug:  800658 
Test: manually tested as noted above
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I7c9fbfb4bbf25d40228895d5f5836bb9caa1f5fa
Reviewed-on: https://chromium-review.googlesource.com/886661
Reviewed-by: Daichi Hirono <hirono@chromium.org>
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532765}
[modify] https://crrev.com/c3951527be651a695ea440e684478eec4186a254/ui/file_manager/file_manager/background/js/device_handler.js
[modify] https://crrev.com/c3951527be651a695ea440e684478eec4186a254/ui/file_manager/file_manager/background/js/device_handler_unittest.js
[modify] https://crrev.com/c3951527be651a695ea440e684478eec4186a254/ui/file_manager/file_manager/common/js/util.js

Status: Fixed (was: Assigned)
The fix should also cover this scenario:

(1) Open incognito window of Chrome browser
(2) [ctrl]+[s] to save the page as a file under Downloads. (for example, NewTab.mhtml) See the notification message "Download complete".
(3) click the "SHOW IN FOLDER" link in the notification. See the Files app open.
(4) Leaving the Files app window opened at (3) opened, close the incognito browser window.

Expected:
The incognito window closes and nothing else happen (i.e. no crash).

Actual:
Chrome process crashes. The screen turns black for a while and then comes back with a new browser window.
(Also the Files app window opened at step #3 has problem opening Zip files similar to Issue 800543.)
Labels: M-65 Merge-Request-65
Status: Assigned (was: Fixed)
I would like to have this fix cherry picked to M65 because this is dependency for Issue 800543.
> I would like to have this fix cherry picked to M65 because this is dependency for Issue 800543.
I mean https://chromium-review.googlesource.com/886661 by "this fix".
Cc: weifangsun@chromium.org
Project Member

Comment 12 by sheriffbot@chromium.org, Jan 31 2018

Labels: -Merge-Request-65 Merge-Review-65 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Review -Merge-Review-65 Merge-Approved-65
Project Member

Comment 14 by bugdroid1@chromium.org, Feb 1 2018

Labels: -merge-approved-65 merge-merged-3325
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/53fd29285bb50a54601087aa877a8e4b06161ff8

commit 53fd29285bb50a54601087aa877a8e4b06161ff8
Author: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Date: Thu Feb 01 01:22:19 2018

Stop Files app in the incognito context to react to device events.

While 1+ incognito browser windows are opened, it'll have separate Files
app extension background and foreground scripts running. The ones in
the incognito context should only be visible by the file picker window
invoked by the browser, but should not show fullscreen app windows or
notifications.

- Prevent duplicated "Removable device detected" notifications
  (when no DCIM folder in it)
- Prevent duplicated Files app windows popup
  (when plugging a disk with /DCIM directory)
- Files app fullscreen mode opened with incognito context (which caused
  some errors), when clicking the "show in folder" link in "Removable
  device detected" notification.
- Other duplicated notifications such as "formatting finished".

This is reland of crrev.com/c/861589 with some tweak.

Bug:  800658 
Test: manually tested as noted above
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I7c9fbfb4bbf25d40228895d5f5836bb9caa1f5fa
Reviewed-on: https://chromium-review.googlesource.com/886661
Reviewed-by: Daichi Hirono <hirono@chromium.org>
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#532765}(cherry picked from commit c3951527be651a695ea440e684478eec4186a254)
Reviewed-on: https://chromium-review.googlesource.com/896902
Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#223}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/53fd29285bb50a54601087aa877a8e4b06161ff8/ui/file_manager/file_manager/background/js/device_handler.js
[modify] https://crrev.com/53fd29285bb50a54601087aa877a8e4b06161ff8/ui/file_manager/file_manager/background/js/device_handler_unittest.js
[modify] https://crrev.com/53fd29285bb50a54601087aa877a8e4b06161ff8/ui/file_manager/file_manager/common/js/util.js

Status: Fixed (was: Assigned)

Sign in to add a comment