New issue
Advanced search Search tips

Issue 611917 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug

Blocking:
issue 577170



Sign in to add a comment

Refactor ChooserBubbleController to be ChooserController

Project Member Reported by juncai@chromium.org, May 13 2016

Issue description

ChooserBubbleController class is used by both WebUSB and WebBluetooth. As  bug 577170  described, a chooser dialog needs to be implemented on Chrome Apps so that WebBluetooth API can be used there. And step one is to refactor ChooserBubbleController to be ChooserController and move it probably to //components/chooser_controller.

 
Project Member

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

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

commit bf183dd5c853046173f09abdd3313084d36d13a3
Author: juncai <juncai@chromium.org>
Date: Fri May 27 16:57:24 2016

Refactor ChooserBubbleController

This is part 1 of enabling the chooser UIs for Bluetooth from
Apps to enable developers to make a transition.

There are many common functionalities in ChooserBubbleController
that chooser UI needs on both desktops and from Apps. But since
now it is at:
//chrome/browser/ui/website_settings/chooser_bubble_controller.h
code from //extensions can not use it. This patch pulled some
functionalities from ChooserBubbleController and moved it to
ChooserController, which is added to
//components/chooser_controller

BUG= 611917 ,  577170 

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

[modify] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/chrome/browser/ui/DEPS
[delete] https://crrev.com/8de46b66b8bdc743df472180d79206e6a22cc1f9/chrome/browser/ui/bluetooth/bluetooth_chooser_bubble_controller.cc
[delete] https://crrev.com/8de46b66b8bdc743df472180d79206e6a22cc1f9/chrome/browser/ui/bluetooth/bluetooth_chooser_bubble_controller.h
[add] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc
[add] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/chrome/browser/ui/bluetooth/bluetooth_chooser_controller.h
[modify] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/chrome/browser/ui/bluetooth/bluetooth_chooser_desktop.cc
[modify] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/chrome/browser/ui/bluetooth/bluetooth_chooser_desktop.h
[modify] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/chrome/browser/ui/browser.cc
[modify] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/chrome/browser/ui/cocoa/website_settings/chooser_bubble_ui_cocoa.h
[modify] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/chrome/browser/ui/cocoa/website_settings/chooser_bubble_ui_cocoa.mm
[modify] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.cc
[modify] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.h
[delete] https://crrev.com/8de46b66b8bdc743df472180d79206e6a22cc1f9/chrome/browser/ui/website_settings/chooser_bubble_controller.cc
[delete] https://crrev.com/8de46b66b8bdc743df472180d79206e6a22cc1f9/chrome/browser/ui/website_settings/chooser_bubble_controller.h
[add] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/chrome/browser/ui/website_settings/chooser_bubble_delegate.cc
[add] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/chrome/browser/ui/website_settings/chooser_bubble_delegate.h
[rename] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/chrome/browser/usb/usb_chooser_controller.cc
[rename] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/chrome/browser/usb/usb_chooser_controller.h
[modify] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/chrome/browser/usb/web_usb_chooser_service.cc
[modify] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/chrome/chrome_browser.gypi
[modify] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/chrome/chrome_browser_ui.gypi
[add] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/components/chooser_controller.gypi
[add] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/components/chooser_controller/BUILD.gn
[add] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/components/chooser_controller/DEPS
[add] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/components/chooser_controller/OWNERS
[add] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/components/chooser_controller/README
[add] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/components/chooser_controller/chooser_controller.cc
[add] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/components/chooser_controller/chooser_controller.h
[modify] https://crrev.com/bf183dd5c853046173f09abdd3313084d36d13a3/components/components.gyp

Status: Fixed (was: Started)

Comment 3 by jam@chromium.org, Jun 30 2016

This says that it's moved to components/chooser_controller so that extensions/ can use it. However the parent bug is fixed and it's only called by chrome/.

If this is only called by chrome, can we move it back? The context is  bug 624590 .

Comment 4 by juncai@chromium.org, Jun 30 2016

I see. Do you think //chrome/browser/ui/ is a good place to put it?

Comment 5 by juncai@chromium.org, Jun 30 2016

ah, saw the spreadsheet at  bug 624590 . So move the whole chooser_controller directory to: //chrome/browser/chooser_controller, right?
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 11 2016

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

commit badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05
Author: juncai <juncai@chromium.org>
Date: Mon Jul 11 20:36:54 2016

Move chooser_controller directory from //components to //chrome/browser

Since chooser_controller is only called by //chrome, this
CL moved the directory from //components to //chrome/browser.

The context is: https://bugs.chromium.org/p/chromium/issues/detail?id=624590

BUG= 611917 

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

[modify] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/browser/BUILD.gn
[rename] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/browser/chooser_controller/OWNERS
[rename] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/browser/chooser_controller/README
[rename] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/browser/chooser_controller/chooser_controller.cc
[rename] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/browser/chooser_controller/chooser_controller.h
[rename] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/browser/chooser_controller/mock_chooser_controller.cc
[rename] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/browser/chooser_controller/mock_chooser_controller.h
[modify] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/browser/ui/DEPS
[modify] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/browser/ui/bluetooth/bluetooth_chooser_controller.h
[modify] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/browser/ui/cocoa/chooser_content_view_cocoa.mm
[modify] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/browser/ui/cocoa/extensions/chooser_dialog_cocoa.mm
[modify] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/browser/ui/cocoa/extensions/chooser_dialog_cocoa_controller.mm
[modify] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/browser/ui/cocoa/extensions/chooser_dialog_cocoa_controller_unittest.mm
[modify] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/browser/ui/cocoa/website_settings/chooser_bubble_ui_cocoa.mm
[modify] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/browser/ui/views/browser_dialogs_views.cc
[modify] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/browser/ui/views/chooser_content_view.cc
[modify] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/browser/ui/views/chooser_content_view_unittest.cc
[modify] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/browser/ui/views/extensions/chooser_dialog_view.cc
[modify] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc
[modify] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.cc
[modify] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/browser/ui/website_settings/chooser_bubble_delegate.cc
[modify] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/browser/usb/usb_chooser_controller.h
[modify] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/chrome_browser.gypi
[modify] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/chrome_browser_ui.gypi
[modify] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/chrome_tests_unit.gypi
[modify] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/chrome/test/BUILD.gn
[modify] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/components/OWNERS
[delete] https://crrev.com/33b11974ba7343fbf2ad6673447488a1563cc8e3/components/chooser_controller.gypi
[delete] https://crrev.com/33b11974ba7343fbf2ad6673447488a1563cc8e3/components/chooser_controller/BUILD.gn
[delete] https://crrev.com/33b11974ba7343fbf2ad6673447488a1563cc8e3/components/chooser_controller/DEPS
[modify] https://crrev.com/badc1daaca94e5ac2d4c1ed07f0f70f0e8360a05/components/components.gyp

Sign in to add a comment