Audit codebase to find all Views-based dialogs |
|||
Issue descriptionWe have a list of dialogs for Harmony at https://docs.google.com/spreadsheets/d/1xDrxVl--AysmTowWQ7x5XeNVjpomZzQwGX-lhVGdwBA/edit#gid=1131158076 , but it's thought to be incomplete. For the benefit of Harmony, MacViews, and the general effort to make dialogs invokable via the browser test framework, we need to audit the codebase and find everything missing from this list. tapted has graciously volunteered to take this on :) P1 since knowing the scope of work sooner rather than later would be good. (But, hopefully, we don't find anything very important during this audit that wasn't already on that list.) Note: It may make sense to add more data to that spreadsheet, e.g. the source code location of the dialog in question, so it's easier to identify "which dialog is this row actually talking about".
,
Feb 3 2017
There's also some "known" missing stuff that I've been giving descriptions to, starting at https://codereview.chromium.org/2675653002/diff/20001/tools/ui/cells.cache#newcode103 (these appear as "Matched", but will get flagged when I do that sheets-output). These include gems like the attached two dialogs.
,
Feb 3 2017
#3: !
,
Feb 9 2017
I've added a column to https://docs.google.com/spreadsheets/d/1xDrxVl--AysmTowWQ7x5XeNVjpomZzQwGX-lhVGdwBA/edit#gid=1131158076 labelled "CS" and there's a separate worksheet called "Code Audit" that it gets its contents from. The 'CS' column should be a link to codesearch (when it can be). The python scripts at https://codereview.chromium.org/2675653002 grep the code and match it up from a dump of columns C and D, then output csv that can be pasted back into 'Code Audit' verbatim. There are 31 "interesting" dialog classes that don't seem to be accounted for. I've given descriptions to 11 of them so far (if you feel like contributing, you can comment on the 'cells.cache' file at https://codereview.chromium.org/2675653002 There are 12 dialogs described in the sheet that I haven't matched up to a class name. These might reuse a class with different constructor arguments, or be something not inheriting from DialogDelegate, or be something that the audit script missed (e.g. a bug :). The audit script ignores test files and and DialogDelegate that has more things inheriting from it (but those are listed).
,
Feb 10 2017
OK I think I've matched up everything in the spreadsheet, so I'm taking this off my runqueue for now. Not sure if there's more to do (e.g. Instructions to invoke and screenshots of all of these would be "nice" but I think that should be farmed out to the specific work items). The list of somewhat interesting dialogs that don't seem to be accounted for are: DownloadInProgressDialogView Shown when exiting Chrome and there are downloads in progress UninstallView The dialog that confirms Chrome uninstallation ImportLockDialogView ExternalProcessImporterHost::ShowWarningDialog() invokes this when importing from Firefox and Firefox is open CryptoModulePasswordDialogView Dialog that prompts for security device password for cert enrollment DownloadFeedbackDialogView Shown once: asks user to participate in the Safe Browsing download feedback program. HomePageUndoBubble This appears when you drag & drop a link onto the home button on the toolbar FirstRunBubble Bubble hanging off the omnibox on Chrome first run FirstRunDialog First run UI for reporting opt-in, import data, etc. NetworkProfileBubbleView Win: Warning shown to the user when they are running Chrome on Windows with a profile located on a network share. InvertBubbleView Win: "Show a bubble telling the user that they are using Windows high-contrast mode with a light-on-dark scheme" ConflictingModuleView Win: UI for the bubble showing that there is a 3rd party module loaded that conflicts with Chrome CriticalNotificationBubbleView Win: Shown when UpgradeDetector emits NOTIFICATION_CRITICAL_UPGRADE_INSTALLED ImeWarningBubbleView Provides warning information to the user upon activation of an IME extension PaymentRequestDialogView Displays the view associated with the current state of the WebPayments flow. ProfileChooserView Displayed when the user clicks the "avatar" (i.e. profile name?) ValidationMessageBubbleView Triggered by the renderer when an HTML form has failed validation constraints PasswordGenerationPopupViewViews Autofill prompt asking whether the user wants to use an auto-generated password (WidgetDelegate) ScreenCaptureNotificationUIViews Floating window that is shown while sharing your screen (WidgetDelegate) TaskManagerView The task manager! SimpleMessageBoxViews Various SimpleMessageBoxViews::ShowMessageBoxWithButtonText Linux: process singleton SimpleMessageBoxViews::ShowQuestionMessageBox Win: process singleton, also bookmark_utils_desktop.cc SimpleMessageBoxViews::ShowWarningMessageBox profile error, bad startup flags, print error, feedback, extension_error_reporter, main_parts, ... SimpleMessageBoxViews::ShowWarningMessageBoxWithCheckbox profile error CreateUrlApplicationShortcutView related to Add to desktop?? CreateChromeApplicationShortcutView related to Add to desktop?? MediaGalleriesDialogViews Extensions: The media galleries configuration view. RequestFileSystemDialogView Extensions: dialog shown to a user for granting access to a file system. NativeDialogContainer Extensions: App Info dialog (chrome://extensions -> Details) ProximityAuthErrorBubbleView Extensions: easy_unlock_private_api ColorChooserView Implements <input type="color"> This list can probably be trimmed a bit further. There's another ~80 dialogs that I don't think are interesting -- e.g. they just wrap a WebContents, or are ChromeOS-only. And another ~100 dialogs that are plumbing/testing that are listed for completeness.
,
Feb 10 2017
CrOS-only dialogs are explicitly Interesting (they are in the main scope of work for Harmony).
,
Feb 10 2017
Actually, dialogs that wrap a WebContents are interesting too, but not for us as much as for the designers to be aware of, so they can evaluate the current WebUI and decide whether in the Harmony world it should be changed at all. If some of these become native UI they would then become our problem.
,
Jul 7 2017
I'm adding BrowserDialogTests for some of these right now. I will file bugs for them too as I go along and note them here. Filed bug 739961 for InvertBubbleView.
,
Aug 8 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 by tapted@chromium.org
, Feb 2 2017Status: Started (was: Assigned)
16.7 KB
16.7 KB View Download