Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2478863003:
Fix the Web Bluetooth chooser when it is used on Chrome apps on non-Mac (Closed)
DescriptionFix the Web Bluetooth chooser when it is used on Chrome apps on non-Mac
This CL adds code to override the views::DialogDelegate::CreateClientView()
and views::DialogDelegate::CreateNonClientFrameView() to fix the artifacts
on the Web Bluetooth chooser when it is used on Chrome apps on non-Mac.
I uploaded some screenshots on the issue page.
BUG=662444
Committed: https://crrev.com/3e540deecddbbdc2f857bba72f0fc19bdd4b3f04
Cr-Commit-Position: refs/heads/master@{#431120}
Patch Set 1 : fix the Web Bluetooth chooser when it is used on Chrome apps on non-Mac #
Total comments: 4
Patch Set 2 : address comments #Patch Set 3 : removed unnecessary include files #
Total comments: 6
Patch Set 4 : address comments #Patch Set 5 : added code to deal with ChooserDialogView::CreateFootnoteView() not called #Patch Set 6 : added code to handle the footnote link pointer ownership #
Total comments: 2
Patch Set 7 : modified code to make ChooserContentsView take a footnote as parameter #Patch Set 8 : updated ChooserDialogViewTest #
Total comments: 10
Patch Set 9 : address comments #
Total comments: 2
Patch Set 10 : initialize pointer to be nullptr #Messages
Total messages: 61 (40 generated)
The CQ bit was checked by juncai@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
juncai@chromium.org changed reviewers: + sky@chromium.org
Please take a look.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2478863003/diff/1/chrome/browser/ui/views/ext... File chrome/browser/ui/views/extensions/chooser_dialog_view.cc (right): https://codereview.chromium.org/2478863003/diff/1/chrome/browser/ui/views/ext... chrome/browser/ui/views/extensions/chooser_dialog_view.cc:74: client->set_button_row_insets(gfx::Insets()); It would be nice not to have explicitly set this. What happens if you don't? Is there another way that it can be automatically detected? https://codereview.chromium.org/2478863003/diff/1/chrome/browser/ui/views/ext... chrome/browser/ui/views/extensions/chooser_dialog_view.cc:80: views::BubbleFrameView* frame = new views::BubbleFrameView( Isn't this the same as DialogDelegate::CreateDialogFrameView but with content_margins? If I'm write, can you make DialogDelegate::CreateDialogFrameView() take an insets and call it? Also, this style is not always used, do you not care about that? By that I mean this code in DialogDelegate is only used if ShouldUseCustomFrame() returns true, which isn't always the case.
The CQ bit was checked by juncai@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by juncai@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2478863003/diff/1/chrome/browser/ui/views/ext... File chrome/browser/ui/views/extensions/chooser_dialog_view.cc (right): https://codereview.chromium.org/2478863003/diff/1/chrome/browser/ui/views/ext... chrome/browser/ui/views/extensions/chooser_dialog_view.cc:74: client->set_button_row_insets(gfx::Insets()); On 2016/11/04 21:35:35, sky wrote: > It would be nice not to have explicitly set this. What happens if you don't? Is > there another way that it can be automatically detected? The DialogDelegate::CreateClientView() doesn't call client->set_button_row_insets(gfx::Insets()): https://cs.chromium.org/chromium/src/ui/views/window/dialog_delegate.cc?dr=C&... And it causes the right side of the "Cancel" button not vertically aligned with the right side of the table view. The chooser on the Chrome browser works fine and I found the BubbleDialogDelegateView::CreateClientView() calls client->set_button_row_insets(gfx::Insets()): https://cs.chromium.org/chromium/src/ui/views/bubble/bubble_dialog_delegate.c... That's why I added the call to client->set_button_row_insets(gfx::Insets()) here. https://codereview.chromium.org/2478863003/diff/1/chrome/browser/ui/views/ext... chrome/browser/ui/views/extensions/chooser_dialog_view.cc:80: views::BubbleFrameView* frame = new views::BubbleFrameView( On 2016/11/04 21:35:35, sky wrote: > Isn't this the same as DialogDelegate::CreateDialogFrameView but with > content_margins? If I'm write, can you make > DialogDelegate::CreateDialogFrameView() take an insets and call it? > > Also, this style is not always used, do you not care about that? By that I mean > this code in DialogDelegate is only used if ShouldUseCustomFrame() returns true, > which isn't always the case. Yes, it is the same as DialogDelegate::CreateDialogFrameView except the content_margins. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2478863003/diff/40001/chrome/browser/ui/views... File chrome/browser/ui/views/extensions/chooser_dialog_view.cc (right): https://codereview.chromium.org/2478863003/diff/40001/chrome/browser/ui/views... chrome/browser/ui/views/extensions/chooser_dialog_view.cc:78: if (ShouldUseCustomFrame()) { How does this dialog look if !ShouldUseCustomFrame? Can you attach a screenshot? https://codereview.chromium.org/2478863003/diff/40001/ui/views/window/dialog_... File ui/views/window/dialog_delegate.h (right): https://codereview.chromium.org/2478863003/diff/40001/ui/views/window/dialog_... ui/views/window/dialog_delegate.h:107: const gfx::Insets& content_margins); Document what content_margins are.
The CQ bit was checked by juncai@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2478863003/diff/40001/chrome/browser/ui/views... File chrome/browser/ui/views/extensions/chooser_dialog_view.cc (right): https://codereview.chromium.org/2478863003/diff/40001/chrome/browser/ui/views... chrome/browser/ui/views/extensions/chooser_dialog_view.cc:78: if (ShouldUseCustomFrame()) { On 2016/11/07 16:13:06, sky wrote: > How does this dialog look if !ShouldUseCustomFrame? Can you attach a screenshot? I tried it, and the chooser crashes the browser. Since if !ShouldUseCustomFrame, it calls: WidgetDelegate::CreateNonClientFrameView() and it returns nullptr: https://cs.chromium.org/chromium/src/ui/views/widget/widget_delegate.cc?q=Wid... I guess I need to remove this if statement and just return views::DialogDelegate::CreateDialogFrameView(). https://codereview.chromium.org/2478863003/diff/40001/ui/views/window/dialog_... File ui/views/window/dialog_delegate.h (right): https://codereview.chromium.org/2478863003/diff/40001/ui/views/window/dialog_... ui/views/window/dialog_delegate.h:107: const gfx::Insets& content_margins); On 2016/11/07 16:13:06, sky wrote: > Document what content_margins are. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2478863003/diff/40001/chrome/browser/ui/views... File chrome/browser/ui/views/extensions/chooser_dialog_view.cc (right): https://codereview.chromium.org/2478863003/diff/40001/chrome/browser/ui/views... chrome/browser/ui/views/extensions/chooser_dialog_view.cc:78: if (ShouldUseCustomFrame()) { On 2016/11/07 21:05:45, juncai wrote: > On 2016/11/07 16:13:06, sky wrote: > > How does this dialog look if !ShouldUseCustomFrame? Can you attach a > screenshot? > > I tried it, and the chooser crashes the browser. Since if !ShouldUseCustomFrame, > it calls: WidgetDelegate::CreateNonClientFrameView() > and it returns nullptr: > https://cs.chromium.org/chromium/src/ui/views/widget/widget_delegate.cc?q=Wid... > > I guess I need to remove this if statement and just return > views::DialogDelegate::CreateDialogFrameView(). Can you attach a screenshot for the !ShouldUseCustomFrame case?
https://codereview.chromium.org/2478863003/diff/40001/chrome/browser/ui/views... File chrome/browser/ui/views/extensions/chooser_dialog_view.cc (right): https://codereview.chromium.org/2478863003/diff/40001/chrome/browser/ui/views... chrome/browser/ui/views/extensions/chooser_dialog_view.cc:78: if (ShouldUseCustomFrame()) { On 2016/11/07 22:01:04, sky wrote: > On 2016/11/07 21:05:45, juncai wrote: > > On 2016/11/07 16:13:06, sky wrote: > > > How does this dialog look if !ShouldUseCustomFrame? Can you attach a > > screenshot? > > > > I tried it, and the chooser crashes the browser. Since if > !ShouldUseCustomFrame, > > it calls: WidgetDelegate::CreateNonClientFrameView() > > and it returns nullptr: > > > https://cs.chromium.org/chromium/src/ui/views/widget/widget_delegate.cc?q=Wid... > > > > I guess I need to remove this if statement and just return > > views::DialogDelegate::CreateDialogFrameView(). > > Can you attach a screenshot for the !ShouldUseCustomFrame case? Changed return WidgetDelegate::CreateNonClientFrameView(widget); to return DialogDelegate::CreateNonClientFrameView(widget); https://cs.chromium.org/chromium/src/ui/views/window/dialog_delegate.cc?sq=pa... I uploaded a screenshot on the issue page for the !ShouldUseCustomFrame case.
The CQ bit was checked by juncai@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Moved the initialization of |footnote_link_| from ChooserContentView::CreateFootnoteView() to ChooserContentView's constructor.
The CQ bit was checked by juncai@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2478863003/diff/100001/chrome/browser/ui/view... File chrome/browser/ui/views/chooser_content_view.cc (right): https://codereview.chromium.org/2478863003/diff/100001/chrome/browser/ui/view... chrome/browser/ui/views/chooser_content_view.cc:286: views::StyledLabel* ChooserContentView::CreateFootnoteView() { I find the usage of footnote_link_ and footnote_link_ptr_ in this class mildly confusing. I think the confusion is because of the combination scoped_ptr and raw pointer. I favor making ChooserContentsView *take* footnote_link_ in its constructor. That way there you get rid of the create and it's clear someone else owns the footnote. WDYT?
On 2016/11/08 05:04:19, sky wrote: > https://codereview.chromium.org/2478863003/diff/100001/chrome/browser/ui/view... > File chrome/browser/ui/views/chooser_content_view.cc (right): > > https://codereview.chromium.org/2478863003/diff/100001/chrome/browser/ui/view... > chrome/browser/ui/views/chooser_content_view.cc:286: views::StyledLabel* > ChooserContentView::CreateFootnoteView() { > I find the usage of footnote_link_ and footnote_link_ptr_ in this class mildly > confusing. I think the confusion is because of the combination scoped_ptr and > raw pointer. I favor making ChooserContentsView *take* footnote_link_ in its > constructor. That way there you get rid of the create and it's clear someone > else owns the footnote. WDYT? Also, in looking at this code a bit more ShouldUseCustomFrame() should always be true for your code. So, I think a DCHECK in CreateNonClientFrameView to that effect is fine.
The CQ bit was checked by juncai@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2478863003/diff/100001/chrome/browser/ui/view... File chrome/browser/ui/views/chooser_content_view.cc (right): https://codereview.chromium.org/2478863003/diff/100001/chrome/browser/ui/view... chrome/browser/ui/views/chooser_content_view.cc:286: views::StyledLabel* ChooserContentView::CreateFootnoteView() { On 2016/11/08 05:04:19, sky wrote: > I find the usage of footnote_link_ and footnote_link_ptr_ in this class mildly > confusing. I think the confusion is because of the combination scoped_ptr and > raw pointer. I favor making ChooserContentsView *take* footnote_link_ in its > constructor. That way there you get rid of the create and it's clear someone > else owns the footnote. WDYT? That sounds better. I modified the ChooserContentsView constructor to take a footnote link. Done.
On 2016/11/08 05:05:15, sky wrote: > On 2016/11/08 05:04:19, sky wrote: > > > https://codereview.chromium.org/2478863003/diff/100001/chrome/browser/ui/view... > > File chrome/browser/ui/views/chooser_content_view.cc (right): > > > > > https://codereview.chromium.org/2478863003/diff/100001/chrome/browser/ui/view... > > chrome/browser/ui/views/chooser_content_view.cc:286: views::StyledLabel* > > ChooserContentView::CreateFootnoteView() { > > I find the usage of footnote_link_ and footnote_link_ptr_ in this class mildly > > confusing. I think the confusion is because of the combination scoped_ptr and > > raw pointer. I favor making ChooserContentsView *take* footnote_link_ in its > > constructor. That way there you get rid of the create and it's clear someone > > else owns the footnote. WDYT? > > Also, in looking at this code a bit more ShouldUseCustomFrame() should always be > true for your code. So, I think a DCHECK in CreateNonClientFrameView to that > effect is fine. If adding a DCHECK() there, it will cause the test failure on Linux since: https://cs.chromium.org/chromium/src/chrome/browser/ui/views/extensions/choos... is called with |parent| set to nullptr. And it changes the |supports_custom_frame_| to be false: https://cs.chromium.org/chromium/src/ui/views/window/dialog_delegate.cc?dr=Ss... And the test fails with DCHECK(ShouldUseCustomFrame()).
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On Tue, Nov 8, 2016 at 11:44 AM, <juncai@chromium.org> wrote: > On 2016/11/08 05:05:15, sky wrote: >> On 2016/11/08 05:04:19, sky wrote: >> > >> > https://codereview.chromium.org/2478863003/diff/100001/chrome/browser/ui/view... >> > File chrome/browser/ui/views/chooser_content_view.cc (right): >> > >> > >> > https://codereview.chromium.org/2478863003/diff/100001/chrome/browser/ui/view... >> > chrome/browser/ui/views/chooser_content_view.cc:286: views::StyledLabel* >> > ChooserContentView::CreateFootnoteView() { >> > I find the usage of footnote_link_ and footnote_link_ptr_ in this class > mildly >> > confusing. I think the confusion is because of the combination >> > scoped_ptr > and >> > raw pointer. I favor making ChooserContentsView *take* footnote_link_ in >> > its >> > constructor. That way there you get rid of the create and it's clear >> > someone >> > else owns the footnote. WDYT? >> >> Also, in looking at this code a bit more ShouldUseCustomFrame() should >> always > be >> true for your code. So, I think a DCHECK in CreateNonClientFrameView to >> that >> effect is fine. > > If adding a DCHECK() there, it will cause the test failure on Linux since: > https://cs.chromium.org/chromium/src/chrome/browser/ui/views/extensions/choos... > is called with |parent| set to nullptr. And it changes the > |supports_custom_frame_| to be false: > https://cs.chromium.org/chromium/src/ui/views/window/dialog_delegate.cc?dr=Ss... > And the test fails with DCHECK(ShouldUseCustomFrame()). > > https://codereview.chromium.org/2478863003/ Correct me I'm wrong, but doesn't that mean test should be updated? I'm assuming the test is doing something production code never does. Am I wrong? -Scott -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
The CQ bit was checked by juncai@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
> Correct me I'm wrong, but doesn't that mean test should be updated? > I'm assuming the test is doing something production code never does. > Am I wrong? > > -Scott > > -- > You received this message because you are subscribed to the Google Groups > "Chromium-reviews" group. > To unsubscribe from this group and stop receiving emails from it, send an email > to mailto:chromium-reviews+unsubscribe@chromium.org. > Right. Thanks for the comment. I have updated the ChooserDialogView test code. Done.
https://codereview.chromium.org/2478863003/diff/140001/chrome/browser/ui/view... File chrome/browser/ui/views/extensions/chooser_dialog_view.cc (right): https://codereview.chromium.org/2478863003/diff/140001/chrome/browser/ui/view... chrome/browser/ui/views/extensions/chooser_dialog_view.cc:38: footnote_link_.reset(new views::StyledLabel(base::string16(), nullptr)); Use MakeUnique (see threads on chromium-dev for reason). https://codereview.chromium.org/2478863003/diff/140001/chrome/browser/ui/view... chrome/browser/ui/views/extensions/chooser_dialog_view.cc:80: DCHECK(ShouldUseCustomFrame()); Please document why there is the DCHECK. I'm thinking something like: ChooserDialogView always has a parent, so ShouldUseCustomFrame() should always be true. https://codereview.chromium.org/2478863003/diff/140001/chrome/browser/ui/view... File chrome/browser/ui/views/extensions/chooser_dialog_view_unittest.cc (right): https://codereview.chromium.org/2478863003/diff/140001/chrome/browser/ui/view... chrome/browser/ui/views/extensions/chooser_dialog_view_unittest.cc:71: views::Widget parent_widget; parent_widget_. Also, I think it better to wrap the widget in a unique_ptr. That way it isn't created/destroyed before/after ViewsTestBase. https://codereview.chromium.org/2478863003/diff/140001/chrome/browser/ui/view... File chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.cc (right): https://codereview.chromium.org/2478863003/diff/140001/chrome/browser/ui/view... chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.cc:106: chooser_content_view_ = new ChooserContentView( MakeUnique on all these. https://codereview.chromium.org/2478863003/diff/140001/ui/views/controls/styl... File ui/views/controls/styled_label.h (right): https://codereview.chromium.org/2478863003/diff/140001/ui/views/controls/styl... ui/views/controls/styled_label.h:71: void set_listener(StyledLabelListener* listener) { listener_ = listener; } I would prefer not to expose this. I think it's moderately error prone. Sorry for sending you down this path. Can you go back to having ChooserContentView create the link? I recommend moving construction to the constructor though. And you can have ChooserContentView own the label (in a unique_ptr, or by value if you prefer), but call set_owned_by_client.
The CQ bit was checked by juncai@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2478863003/diff/140001/chrome/browser/ui/view... File chrome/browser/ui/views/extensions/chooser_dialog_view.cc (right): https://codereview.chromium.org/2478863003/diff/140001/chrome/browser/ui/view... chrome/browser/ui/views/extensions/chooser_dialog_view.cc:38: footnote_link_.reset(new views::StyledLabel(base::string16(), nullptr)); On 2016/11/09 17:55:47, sky wrote: > Use MakeUnique (see threads on chromium-dev for reason). Done. https://codereview.chromium.org/2478863003/diff/140001/chrome/browser/ui/view... chrome/browser/ui/views/extensions/chooser_dialog_view.cc:80: DCHECK(ShouldUseCustomFrame()); On 2016/11/09 17:55:47, sky wrote: > Please document why there is the DCHECK. I'm thinking something like: > > ChooserDialogView always has a parent, so ShouldUseCustomFrame() should always > be true. Done. https://codereview.chromium.org/2478863003/diff/140001/chrome/browser/ui/view... File chrome/browser/ui/views/extensions/chooser_dialog_view_unittest.cc (right): https://codereview.chromium.org/2478863003/diff/140001/chrome/browser/ui/view... chrome/browser/ui/views/extensions/chooser_dialog_view_unittest.cc:71: views::Widget parent_widget; On 2016/11/09 17:55:47, sky wrote: > parent_widget_. Also, I think it better to wrap the widget in a unique_ptr. That > way it isn't created/destroyed before/after ViewsTestBase. Done. https://codereview.chromium.org/2478863003/diff/140001/chrome/browser/ui/view... File chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.cc (right): https://codereview.chromium.org/2478863003/diff/140001/chrome/browser/ui/view... chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.cc:106: chooser_content_view_ = new ChooserContentView( On 2016/11/09 17:55:47, sky wrote: > MakeUnique on all these. |chooser_content_view_| is a raw pointer of ChooserContentView. https://codereview.chromium.org/2478863003/diff/140001/ui/views/controls/styl... File ui/views/controls/styled_label.h (right): https://codereview.chromium.org/2478863003/diff/140001/ui/views/controls/styl... ui/views/controls/styled_label.h:71: void set_listener(StyledLabelListener* listener) { listener_ = listener; } On 2016/11/09 17:55:48, sky wrote: > I would prefer not to expose this. I think it's moderately error prone. Sorry > for sending you down this path. Can you go back to having ChooserContentView > create the link? I recommend moving construction to the constructor though. And > you can have ChooserContentView own the label (in a unique_ptr, or by value if > you prefer), but call set_owned_by_client. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM - thanks for you patience. https://codereview.chromium.org/2478863003/diff/160001/chrome/browser/ui/view... File chrome/browser/ui/views/chooser_content_view_unittest.cc (right): https://codereview.chromium.org/2478863003/diff/160001/chrome/browser/ui/view... chrome/browser/ui/views/chooser_content_view_unittest.cc:73: views::StyledLabel* footnote_link_; optional: nuke this and expose a footnote_link() function that returns chooser_content_view_->footnote_link(). If you don't do the option, set footnote_link_ to null here (or the constructor), in fact all these pointers should be set in the constructor here.
The CQ bit was checked by juncai@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2478863003/diff/160001/chrome/browser/ui/view... File chrome/browser/ui/views/chooser_content_view_unittest.cc (right): https://codereview.chromium.org/2478863003/diff/160001/chrome/browser/ui/view... chrome/browser/ui/views/chooser_content_view_unittest.cc:73: views::StyledLabel* footnote_link_; On 2016/11/09 22:26:43, sky wrote: > optional: nuke this and expose a footnote_link() function that returns > chooser_content_view_->footnote_link(). > > If you don't do the option, set footnote_link_ to null here (or the > constructor), in fact all these pointers should be set in the constructor here. Thanks for all the comments! Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by juncai@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org Link to the patchset: https://codereview.chromium.org/2478863003/#ps180001 (title: "initialize pointer to be nullptr")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #10 (id:180001)
Message was sent while issue was closed.
Description was changed from ========== Fix the Web Bluetooth chooser when it is used on Chrome apps on non-Mac This CL adds code to override the views::DialogDelegate::CreateClientView() and views::DialogDelegate::CreateNonClientFrameView() to fix the artifacts on the Web Bluetooth chooser when it is used on Chrome apps on non-Mac. I uploaded some screenshots on the issue page. BUG=662444 ========== to ========== Fix the Web Bluetooth chooser when it is used on Chrome apps on non-Mac This CL adds code to override the views::DialogDelegate::CreateClientView() and views::DialogDelegate::CreateNonClientFrameView() to fix the artifacts on the Web Bluetooth chooser when it is used on Chrome apps on non-Mac. I uploaded some screenshots on the issue page. BUG=662444 Committed: https://crrev.com/3e540deecddbbdc2f857bba72f0fc19bdd4b3f04 Cr-Commit-Position: refs/heads/master@{#431120} ==========
Message was sent while issue was closed.
Patchset 10 (id:??) landed as https://crrev.com/3e540deecddbbdc2f857bba72f0fc19bdd4b3f04 Cr-Commit-Position: refs/heads/master@{#431120} |
