New issue
Advanced search Search tips

Issue 734677 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Feature

Blocked on:
issue 753632



Sign in to add a comment

Chrome Cleaner UI: Try to prompt later when a good browser is not available at first attemp

Project Member Reported by alito@chromium.org, Jun 19 2017

Issue description

Currently, if we fail to find a suitable browser window to show the Chrome Cleaner prompt in, we just skip showing the prompt. We could instead register ourselves to be notifed when a suitable browser does appear and prompt the user then.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 4 2017

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

commit df950cc336fda4298d6dd2c587c9a9c4d47b9874
Author: Juan Jose Lopez Jaimez <jjlopezjaimez@google.com>
Date: Fri Aug 04 23:57:47 2017

Add the dialog controller as an observer so we can prompt the user if no browser is available
Bug:   http://crbug.com/734677 

Change-Id: Iec5a1e85bd4887dac5a821d2bb96d6db70f33cb6
Reviewed-on: https://chromium-review.googlesource.com/593967
Reviewed-by: Robert Shield <robertshield@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Ali Tofigh <alito@chromium.org>
Commit-Queue: Juan José Lopez Jaimez <jjlopezjaimez@google.com>
Cr-Commit-Position: refs/heads/master@{#492183}
[add] https://crrev.com/df950cc336fda4298d6dd2c587c9a9c4d47b9874/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_dialog_controller_impl_browsertest_win.cc
[modify] https://crrev.com/df950cc336fda4298d6dd2c587c9a9c4d47b9874/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_dialog_controller_impl_win.cc
[modify] https://crrev.com/df950cc336fda4298d6dd2c587c9a9c4d47b9874/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_dialog_controller_impl_win.h
[modify] https://crrev.com/df950cc336fda4298d6dd2c587c9a9c4d47b9874/chrome/browser/safe_browsing/chrome_cleaner/srt_field_trial_win.cc
[modify] https://crrev.com/df950cc336fda4298d6dd2c587c9a9c4d47b9874/chrome/browser/safe_browsing/chrome_cleaner/srt_field_trial_win.h
[modify] https://crrev.com/df950cc336fda4298d6dd2c587c9a9c4d47b9874/chrome/browser/safe_browsing/chrome_cleaner/srt_global_error_win.cc
[modify] https://crrev.com/df950cc336fda4298d6dd2c587c9a9c4d47b9874/chrome/test/BUILD.gn
[modify] https://crrev.com/df950cc336fda4298d6dd2c587c9a9c4d47b9874/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/df950cc336fda4298d6dd2c587c9a9c4d47b9874/tools/metrics/histograms/histograms.xml

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 7 2017

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

commit e2b3698c962e14be9675d533d179d4a5335cd40e
Author: Takashi Sakamoto <tasak@google.com>
Date: Mon Aug 07 04:48:57 2017

Revert "Add the dialog controller as an observer so we can prompt the user if no browser is available"

This reverts commit df950cc336fda4298d6dd2c587c9a9c4d47b9874.

Reason for revert: 
failures:
ChromeCleanerPromptUserTest.OnInfectedBrowserNotAvailable
ChromeCleanerPromptUserTest.AllBrowsersClosed
ChromeCleanerPromptUserTest.OnInfectedBrowserAvailable
 
[ RUN      ] ChromeCleanerPromptUserTest.OnInfectedBrowserNotAvailable
GMOCK WARNING:
Uninteresting mock function call - returning directly.
    Function call: AddObserver(00000000025C5C18)
NOTE: You can safely ignore the above warning unless this call should not happen.  Do not suppress it by blindly adding an EXPECT_CALL() if you don't mean to enforce the call.  See https://github.com/google/googletest/blob/master/googlemock/docs/CookBook.md#knowing-when-to-expect for details.
../../chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_dialog_controller_impl_browsertest_win.cc(59): error: Actual function call count doesn't match EXPECT_CALL(mock_cleaner_controller_, state())...
         Expected: to be called once
           Actual: never called - unsatisfied and active

Original change's description:
> Add the dialog controller as an observer so we can prompt the user if no browser is available
> Bug:   http://crbug.com/734677 
> 
> Change-Id: Iec5a1e85bd4887dac5a821d2bb96d6db70f33cb6
> Reviewed-on: https://chromium-review.googlesource.com/593967
> Reviewed-by: Robert Shield <robertshield@chromium.org>
> Reviewed-by: Jesse Doherty <jwd@chromium.org>
> Reviewed-by: Ali Tofigh <alito@chromium.org>
> Commit-Queue: Juan José Lopez Jaimez <jjlopezjaimez@google.com>
> Cr-Commit-Position: refs/heads/master@{#492183}

TBR=jwd@chromium.org,robertshield@chromium.org,joenotcharles@google.com,ftirelo@chromium.org,alito@chromium.org,jjlopezjaimez@google.com

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

Bug:  http://crbug.com/734677 
Change-Id: If5c3cda43b284860a5ef3bfc541813925ff491d0
Reviewed-on: https://chromium-review.googlesource.com/602833
Reviewed-by: Takashi Sakamoto <tasak@google.com>
Commit-Queue: Takashi Sakamoto <tasak@google.com>
Cr-Commit-Position: refs/heads/master@{#492266}
[delete] https://crrev.com/cfd7e50f2eb11851dc562c826cba85a028280618/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_dialog_controller_impl_browsertest_win.cc
[modify] https://crrev.com/e2b3698c962e14be9675d533d179d4a5335cd40e/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_dialog_controller_impl_win.cc
[modify] https://crrev.com/e2b3698c962e14be9675d533d179d4a5335cd40e/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_dialog_controller_impl_win.h
[modify] https://crrev.com/e2b3698c962e14be9675d533d179d4a5335cd40e/chrome/browser/safe_browsing/chrome_cleaner/srt_field_trial_win.cc
[modify] https://crrev.com/e2b3698c962e14be9675d533d179d4a5335cd40e/chrome/browser/safe_browsing/chrome_cleaner/srt_field_trial_win.h
[modify] https://crrev.com/e2b3698c962e14be9675d533d179d4a5335cd40e/chrome/browser/safe_browsing/chrome_cleaner/srt_global_error_win.cc
[modify] https://crrev.com/e2b3698c962e14be9675d533d179d4a5335cd40e/chrome/test/BUILD.gn
[modify] https://crrev.com/e2b3698c962e14be9675d533d179d4a5335cd40e/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/e2b3698c962e14be9675d533d179d4a5335cd40e/tools/metrics/histograms/histograms.xml

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 8 2017

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

commit fec79bd98fcdfa1e42fefa875c598ffa61a82d72
Author: Juan Jose Lopez Jaimez <jjlopezjaimez@google.com>
Date: Tue Aug 08 16:36:54 2017

Reland of Add the dialog controller as an observer 
Bug:   http://crbug.com/734677 

Change-Id: Iec5a1e85bd4887dac5a821d2bb96d6db70f33cb6
Reviewed-on: https://chromium-review.googlesource.com/593967
Reviewed-by: Robert Shield <robertshield@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Ali Tofigh <alito@chromium.org>
Commit-Queue: Juan José Lopez Jaimez <jjlopezjaimez@google.com>
Cr-Commit-Position: refs/heads/master@{#492183}

Reverted here:
https://chromium-review.googlesource.com/c/602833

Because we were expecting a call that was being made inside a DCHECK therefore making the tests fail
on release builds.

Change-Id: Icf9c45af48295d15c6aff6ce6282db8381214f73
Reviewed-on: https://chromium-review.googlesource.com/603879
Reviewed-by: Ali Tofigh <alito@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Robert Shield <robertshield@chromium.org>
Commit-Queue: Juan José Lopez Jaimez <jjlopezjaimez@google.com>
Cr-Commit-Position: refs/heads/master@{#492653}
[add] https://crrev.com/fec79bd98fcdfa1e42fefa875c598ffa61a82d72/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_dialog_controller_impl_browsertest_win.cc
[modify] https://crrev.com/fec79bd98fcdfa1e42fefa875c598ffa61a82d72/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_dialog_controller_impl_win.cc
[modify] https://crrev.com/fec79bd98fcdfa1e42fefa875c598ffa61a82d72/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_dialog_controller_impl_win.h
[modify] https://crrev.com/fec79bd98fcdfa1e42fefa875c598ffa61a82d72/chrome/browser/safe_browsing/chrome_cleaner/srt_field_trial_win.cc
[modify] https://crrev.com/fec79bd98fcdfa1e42fefa875c598ffa61a82d72/chrome/browser/safe_browsing/chrome_cleaner/srt_field_trial_win.h
[modify] https://crrev.com/fec79bd98fcdfa1e42fefa875c598ffa61a82d72/chrome/browser/safe_browsing/chrome_cleaner/srt_global_error_win.cc
[modify] https://crrev.com/fec79bd98fcdfa1e42fefa875c598ffa61a82d72/chrome/test/BUILD.gn
[modify] https://crrev.com/fec79bd98fcdfa1e42fefa875c598ffa61a82d72/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/fec79bd98fcdfa1e42fefa875c598ffa61a82d72/tools/metrics/histograms/histograms.xml

Blockedon: 753632
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 9 2017

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

commit 69f7ddbe92ad5372d2c263d930ee20cfb1c3528f
Author: Makoto Shimazu <shimazu@chromium.org>
Date: Wed Aug 09 01:54:22 2017

Disabled flaky ChromeCleanerPromptUserTest.OnInfectedBrowserNotAvailable

TBR=robertshield@chromium.org,jjlopezjaimez@google.com
NOTRY=true

Bug: 753632,  734677 
Change-Id: I1405aed369a9e59cd2d45ff57b0359809d24c11c
Reviewed-on: https://chromium-review.googlesource.com/607689
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492804}
[modify] https://crrev.com/69f7ddbe92ad5372d2c263d930ee20cfb1c3528f/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_dialog_controller_impl_browsertest_win.cc

Comment 6 by alito@chromium.org, Nov 6 2017

Status: Fixed (was: Assigned)

Sign in to add a comment