Copy remoting notification bar to chrome and use it for screen capture API.

Copy remoting notification bar to chrome and use it for screen capture API.

Chrome must show notification bar when screen capture API is used to capture 
the screen. This CL reuses the notification bar that's currently used by 
remoting host (see src/remoting/host/disconnect_window*). The new files are 
mostly copy of the existing code in remoting (except different names). 

The plan is to keep it temporarily and replace it with something better before
the API is enabled for everybody.

BUG=190156
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=190041

[Nico, 2013-03-22 17:51:12]

Questions:

* How much do you think this UI will end up differing from the remoting UI? If
"not all that much", does it make sense to have a shared class for both UIs (or
a shared base class), instead of copying all this code? Less work to maintain,
smaller binary, etc.

* The windows and linux files show up as A+ (moved, then edited). The mac files
don't. Do you know why?

* I think it's fairly unusual to use rc files for UI on windows (probably
partially because that doesn't work on other views platforms such as CrOS)

[Sergey Ulanov, 2013-03-22 18:20:59]

On 2013/03/22 17:51:12, Nico wrote:
> Questions:
> 
> * How much do you think this UI will end up differing from the remoting UI? If
> "not all that much", does it make sense to have a shared class for both UIs
(or
> a shared base class), instead of copying all this code? Less work to maintain,
> smaller binary, etc.

I'm not sure how different it will be. Right now the difference is only in the
strings. Most likely we will want to change the UI used in Chrome and we may
need better integration with the rest of the chrome UI. For chromoting the
current UI works fine. For now I think it's acceptable to keep them separate -
we are not making any changes in chromoting copy anyway. Separate copy allows to
iterate on the screen capture UI faster without breaking chromoting. Binary size
is not a concern here because chromoting host is in a separate binary installed
separately from chrome.


> 
> * The windows and linux files show up as A+ (moved, then edited). The mac
files
> don't. Do you know why?

I guess that's because Git was able to detect that they are copies, but the diff
for Mac version is bigger, so git treats it as a new file. I'll try to do
something about it (maybe just upload two patch-sets to make it easier to
review).


> 
> * I think it's fairly unusual to use rc files for UI on windows (probably
> partially because that doesn't work on other views platforms such as CrOS)

Yes, I agree. We will need to fix it later. Added TODO about it.

[Nico, 2013-03-22 18:34:01]

On 2013/03/22 18:20:59, sergeyu wrote:
> On 2013/03/22 17:51:12, Nico wrote:
> > Questions:
> > 
> > * How much do you think this UI will end up differing from the remoting UI?
If
> > "not all that much", does it make sense to have a shared class for both UIs
> (or
> > a shared base class), instead of copying all this code? Less work to
maintain,
> > smaller binary, etc.
> 
> I'm not sure how different it will be. Right now the difference is only in the
> strings. Most likely we will want to change the UI used in Chrome and we may
> need better integration with the rest of the chrome UI. For chromoting the
> current UI works fine. For now I think it's acceptable to keep them separate -
> we are not making any changes in chromoting copy anyway. Separate copy allows
to
> iterate on the screen capture UI faster without breaking chromoting. Binary
size
> is not a concern here because chromoting host is in a separate binary
installed
> separately from chrome.

Ok.

> 
> 
> > 
> > * The windows and linux files show up as A+ (moved, then edited). The mac
> files
> > don't. Do you know why?
> 
> I guess that's because Git was able to detect that they are copies, but the
diff
> for Mac version is bigger, so git treats it as a new file. I'll try to do
> something about it (maybe just upload two patch-sets to make it easier to
> review).

I think there's some flag to tell git to try harder (--find-copies-harder). `git
cl upload`'s --similarity might be helpful too.

> 
> 
> > 
> > * I think it's fairly unusual to use rc files for UI on windows (probably
> > partially because that doesn't work on other views platforms such as CrOS)
> 
> Yes, I agree. We will need to fix it later. Added TODO about it.

Why not fix now? It's just a few buttons.

[Sergey Ulanov, 2013-03-22 18:43:17]

On 2013/03/22 18:34:01, Nico wrote:
> On 2013/03/22 18:20:59, sergeyu wrote:
> > On 2013/03/22 17:51:12, Nico wrote:
> > > 
> > > * The windows and linux files show up as A+ (moved, then edited). The mac
> > files
> > > don't. Do you know why?
> > 
> > I guess that's because Git was able to detect that they are copies, but the
> diff
> > for Mac version is bigger, so git treats it as a new file. I'll try to do
> > something about it (maybe just upload two patch-sets to make it easier to
> > review).
> 
> I think there's some flag to tell git to try harder (--find-copies-harder).
`git
> cl upload`'s --similarity might be helpful too.

Yes --similarity=40 did the trick. Thanks


> > > * I think it's fairly unusual to use rc files for UI on windows (probably
> > > partially because that doesn't work on other views platforms such as CrOS)
> > 
> > Yes, I agree. We will need to fix it later. Added TODO about it.
> 
> Why not fix now? It's just a few buttons.

I really would like to land this before M27 branches, but it will take some time
for me to fix this. Can I land it with the resources and then fix it next week?

[Nico, 2013-03-22 18:46:37]

A screenshot would be nice. Is this doing its own window gripper?

The bitmap->asset comment applies to gtk / views too, but please show me how
this looks first.

https://codereview.chromium.org/12453019/diff/32001/chrome/browser/ui/cocoa/s...
File chrome/browser/ui/cocoa/screen_capture_notification_ui_cocoa.mm (right):

https://codereview.chromium.org/12453019/diff/32001/chrome/browser/ui/cocoa/s...
chrome/browser/ui/cocoa/screen_capture_notification_ui_cocoa.mm:126: // Then
resize the window appropriately
.

https://codereview.chromium.org/12453019/diff/32001/chrome/browser/ui/cocoa/s...
chrome/browser/ui/cocoa/screen_capture_notification_ui_cocoa.mm:215: [context
setShouldAntialias:NO];
Can you upload a screenshot for this please?

Antialiasing is generally desired; the issue here is probably that your
coordinates are on integer boundaries. In cocoa, integer coordinates are on
pixel corners; if you want a crisp line you need to put your points on .5
coordinates.

For retina displays, it's better to draw rects instead of lines though (1 unit
high).

And generally, we try to use bitmap assets instead of code-drawing in chrome,
because these are easier to work with for the UI folks. (Also makes it easier to
keep assets consistent across platforms.)

https://codereview.chromium.org/12453019/diff/32001/chrome/browser/ui/cocoa/s...
chrome/browser/ui/cocoa/screen_capture_notification_ui_cocoa.mm:217: // Handle
bidirectional locales properly.
Chrome/Mac doesn't handle RTL as far as I know. (mostly fyi)

[Sergey Ulanov, 2013-03-22 18:49:37]

On 2013/03/22 18:46:37, Nico wrote:
> A screenshot would be nice. Is this doing its own window gripper?
> 
> The bitmap->asset comment applies to gtk / views too, but please show me how
> this looks first.

Here is a screen shot of how it looks in Chromoting (the bar with green border):
  http://blog.laptopmag.com/wpress/wp-content/uploads/2013/03/SS5_Share-2.png

[Nico, 2013-03-22 18:55:12]

On 2013/03/22 18:49:37, sergeyu wrote:
> On 2013/03/22 18:46:37, Nico wrote:
> > A screenshot would be nice. Is this doing its own window gripper?
> > 
> > The bitmap->asset comment applies to gtk / views too, but please show me how
> > this looks first.
> 
> Here is a screen shot of how it looks in Chromoting (the bar with green
border):
>   http://blog.laptopmag.com/wpress/wp-content/uploads/2013/03/SS5_Share-2.png

Thanks! Has this UI been approved by UI leads for use in chrome? It seems
different from what we use for similar purposes (info bars for e.g. web cam
capturing, and info bubbles for e.g. mouse lock). (The bug doesn't say.)

[Sergey Ulanov, 2013-03-22 19:14:22]

On 2013/03/22 18:55:12, Nico wrote:
> On 2013/03/22 18:49:37, sergeyu wrote:
> > On 2013/03/22 18:46:37, Nico wrote:
> > > A screenshot would be nice. Is this doing its own window gripper?
> > > 
> > > The bitmap->asset comment applies to gtk / views too, but please show me
how
> > > this looks first.
> > 
> > Here is a screen shot of how it looks in Chromoting (the bar with green
> border):
> >  
http://blog.laptopmag.com/wpress/wp-content/uploads/2013/03/SS5_Share-2.png
> 
> Thanks! Has this UI been approved by UI leads for use in chrome? It seems
> different from what we use for similar purposes (info bars for e.g. web cam
> capturing, and info bubbles for e.g. mouse lock). (The bug doesn't say.)

No. In M27 screen capture API is available to hangouts only, and hangouts will
use it only for dogfood. I plan to get UX folks involved in M28 and get this UI
properly designed. So essentially this CL is just a stop-gap until we have
something better.
Also, jannyz@ landed ChromeOS specific implementation that will integrate nicely
with ChromeOS tray.

[Sergey Ulanov, 2013-03-22 19:39:08]

https://codereview.chromium.org/12453019/diff/32001/chrome/browser/ui/cocoa/s...
File chrome/browser/ui/cocoa/screen_capture_notification_ui_cocoa.mm (right):

https://codereview.chromium.org/12453019/diff/32001/chrome/browser/ui/cocoa/s...
chrome/browser/ui/cocoa/screen_capture_notification_ui_cocoa.mm:126: // Then
resize the window appropriately
On 2013/03/22 18:46:37, Nico wrote:
> .

Done.

https://codereview.chromium.org/12453019/diff/32001/chrome/browser/ui/cocoa/s...
chrome/browser/ui/cocoa/screen_capture_notification_ui_cocoa.mm:215: [context
setShouldAntialias:NO];
On 2013/03/22 18:46:37, Nico wrote:
> Can you upload a screenshot for this please?
> 
> Antialiasing is generally desired; the issue here is probably that your
> coordinates are on integer boundaries. In cocoa, integer coordinates are on
> pixel corners; if you want a crisp line you need to put your points on .5
> coordinates.
> 
> For retina displays, it's better to draw rects instead of lines though (1 unit
> high).
> 

Here is a screenshot from mac: http://i.imgur.com/Qqw7DE1.png
It looks fine to me  and the border seems to look find. Also we do set line
width, so I would guess it should render properly on retina.


> And generally, we try to use bitmap assets instead of code-drawing in chrome,
> because these are easier to work with for the UI folks. (Also makes it easier
to
> keep assets consistent across platforms.)

Yes, that's good point. As I mentioned before this UI will need to be redesigned
anyway, at which point we will probably switch to bitmaps anyway.

[Nico, 2013-03-23 00:39:50]

Please mention in the CL description that the expectation is that this UI will
be completely redone before this feature moves out from behind a flag. Based on
this, lgtm as it doesn't make much sense to polish this if it's going to be
deleted again.

[Sergey Ulanov, 2013-03-23 01:17:43]

On 2013/03/23 00:39:50, Nico wrote:
> Please mention in the CL description that the expectation is that this UI will
> be completely redone before this feature moves out from behind a flag. Based
on
> this, lgtm as it doesn't make much sense to polish this if it's going to be
> deleted again.

Thank you. Updated description.

[commit-bot: I haz the power, 2013-03-23 12:47:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/12453019/37001

[commit-bot: I haz the power, 2013-03-23 14:50:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/12453019/37001

[commit-bot: I haz the power, 2013-03-23 17:37:03]

Change committed as 190041