Create Desktop to iOS Promotion skeleton.

Create Desktop to iOS Promotion.

Added General skeleton to all windows to ios promotions in:
chrome/browser/ui/desktop_io_promotion/
Which will be responsible for determining the eligibility of users
to get the promotion, executes actions like sending SMS and general logging.

Create Savepasswords desktop to ios promotion UI.
Guarded by false flag to not show, a bubble appears that contains
the promotion text and send to phone call to action and dismiss option
(see the attached screen).

screen: https://drive.google.com/a/google.com/file/d/0B9T1TKmsVG94YVd6cHMwUk9CMVk/view

BUG=676655
Review-Url: https://codereview.chromium.org/2617543002
Cr-Commit-Position: refs/heads/master@{#442124}
Committed: https://chromium.googlesource.com/chromium/src/+/bd2449f5de94d47bf4b33c439021897a8f3a86d0

[mrefaat, 2017-01-04 17:19:35]

mrefaat@chromium.org changed reviewers:
+ sky@chromium.org, vasilii@chromium.org

[mrefaat, 2017-01-04 17:19:36]

Same as the old CL but created with chromium account.

[mrefaat, 2017-01-04 17:20:48]

Description was changed from

==========
Create Desktop to iOS Promotion skeleton.
Add Savepasswords promotion UI under false flag.

screen:
https://drive.google.com/a/google.com/file/d/0B9T1TKmsVG94YVd6cHMwUk9CMVk/view

BUG=676655
==========

to

==========
Create Desktop to iOS Promotion skeleton.
Add Savepasswords promotion UI under false flag.

screen:
https://drive.google.com/a/google.com/file/d/0B9T1TKmsVG94YVd6cHMwUk9CMVk/view

BUG=676655
==========

[mrefaat, 2017-01-04 17:20:49]

mrefaat@chromium.org changed reviewers:
+ justincohen@chromium.org

[sky, 2017-01-04 20:52:37]

https://codereview.chromium.org/2617543002/diff/1/chrome/app/generated_resour...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2617543002/diff/1/chrome/app/generated_resour...
chrome/app/generated_resources.grd:6246: +      <message
name="IDS_PASSWORD_MANAGER_DESKTOP_IOS_PROMO_TITLE" desc="Title for Chrome iOS
promotion appearing in the password bubble after a password is saved.">
Are these in is_macosx sections?

I also recommend making it clear this is shown on the desktop as the description
makes it sounds like it appears on iOS, which isn't the case.

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/desktop_i...
File chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_model.h
(right):

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/desktop_i...
chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_model.h:8:
#include <memory>
AFAICT you don't use this include.

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/desktop_i...
chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_model.h:11:
#include "chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_util.h"
Generally we forward declare when possible (it's in chromium style guide). I
think you have this include because of PromotionEntryPoint. You should be able
to forward declare it.

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/desktop_i...
chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_model.h:15: class
DesktopIOSPromotionModel {
How come you are naming this class as the model? This class doesn't seem like a
model from what I can tell.

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/desktop_i...
chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_model.h:20: 
Don't inline the destructor.

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/desktop_i...
chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_model.h:28: 
remove newline.

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/desktop_i...
chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_model.h:29:
desktop_ios_promotion::PromotionEntryPoint entry_point_;
const

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/views/des...
File chrome/browser/ui/views/desktop_ios_promotion/desktop_ios_promotion_view.cc
(right):

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/views/des...
chrome/browser/ui/views/desktop_ios_promotion/desktop_ios_promotion_view.cc:55:
int DesktopIOSPromotionView::GetDesiredBubbleWidth(
AFAICT you don't need these to be member functions. Effective C++ (and google
style guide) recommend non-member functions when possible. In other words make
these be functions in an anonymous namespace at the top of the file.

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/views/des...
chrome/browser/ui/views/desktop_ios_promotion/desktop_ios_promotion_view.cc:76:
if (sender == send_sms_button_) {
no {}

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/views/des...
File chrome/browser/ui/views/desktop_ios_promotion/desktop_ios_promotion_view.h
(right):

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/views/des...
chrome/browser/ui/views/desktop_ios_promotion/desktop_ios_promotion_view.h:22:
class DesktopIOSPromotionView : public views::View,
Why do you need to subclass View here?

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/views/des...
chrome/browser/ui/views/desktop_ios_promotion/desktop_ios_promotion_view.h:26: 
Don't inline destructor.

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/views/pas...
File chrome/browser/ui/views/passwords/manage_passwords_bubble_view.h (right):

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/views/pas...
chrome/browser/ui/views/passwords/manage_passwords_bubble_view.h:26: static
const int kDesiredBubbleWidth = 370;
constexpr

[mrefaat, 2017-01-05 00:02:53]

Patchset #2 (id:20001) has been deleted

[mrefaat, 2017-01-05 00:03:08]

https://codereview.chromium.org/2617543002/diff/1/chrome/app/generated_resour...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2617543002/diff/1/chrome/app/generated_resour...
chrome/app/generated_resources.grd:6246: +      <message
name="IDS_PASSWORD_MANAGER_DESKTOP_IOS_PROMO_TITLE" desc="Title for Chrome iOS
promotion appearing in the password bubble after a password is saved.">
On 2017/01/04 20:52:36, sky wrote:
> Are these in is_macosx sections?
> 
> I also recommend making it clear this is shown on the desktop as the
description
> makes it sounds like it appears on iOS, which isn't the case.

No they are not in is_macosx -> this promotion should run for windows.
second part done.

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/desktop_i...
File chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_model.h
(right):

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/desktop_i...
chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_model.h:15: class
DesktopIOSPromotionModel {
On 2017/01/04 20:52:37, sky wrote:
> How come you are naming this class as the model? This class doesn't seem like
a
> model from what I can tell.

i thought first i'd name it Controller - but after looking through the code for
password manager i found that it's the model that controls the actions of the
password bubble. vasilii@ what do you think ?

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/desktop_i...
chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_model.h:20: 
On 2017/01/04 20:52:37, sky wrote:
> Don't inline the destructor.

Done.

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/desktop_i...
chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_model.h:28: 
On 2017/01/04 20:52:36, sky wrote:
> remove newline.

Done.

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/desktop_i...
chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_model.h:29:
desktop_ios_promotion::PromotionEntryPoint entry_point_;
On 2017/01/04 20:52:36, sky wrote:
> const

Done.

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/views/pas...
File chrome/browser/ui/views/passwords/manage_passwords_bubble_view.h (right):

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/views/pas...
chrome/browser/ui/views/passwords/manage_passwords_bubble_view.h:26: static
const int kDesiredBubbleWidth = 370;
On 2017/01/04 20:52:37, sky wrote:
> constexpr

Done.

[mrefaat, 2017-01-05 00:04:09]

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/desktop_i...
File chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_model.h
(right):

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/desktop_i...
chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_model.h:8:
#include <memory>
On 2017/01/04 20:52:36, sky wrote:
> AFAICT you don't use this include.

Done.

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/desktop_i...
chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_model.h:11:
#include "chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_util.h"
On 2017/01/04 20:52:36, sky wrote:
> Generally we forward declare when possible (it's in chromium style guide). I
> think you have this include because of PromotionEntryPoint. You should be able
> to forward declare it.

Done.

[sky, 2017-01-05 00:35:21]

LGTM

https://codereview.chromium.org/2617543002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/desktop_ios_promotion/desktop_ios_promotion_view.cc
(right):

https://codereview.chromium.org/2617543002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/desktop_ios_promotion/desktop_ios_promotion_view.cc:28:
}
Generally we add
  // namespace
for the closing } of a namespace, and it's two spaces, e.g.:
}  // namespace

We generally have a newline before and after the {} of the namespace as well,
but we aren't always consistent so that is up to you.

[vasilii, 2017-01-05 17:40:21]

lgtm

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/desktop_i...
File chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_model.h
(right):

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/desktop_i...
chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_model.h:15: class
DesktopIOSPromotionModel {
On 2017/01/05 00:03:08, mrefaat wrote:
> On 2017/01/04 20:52:37, sky wrote:
> > How come you are naming this class as the model? This class doesn't seem
like
> a
> > model from what I can tell.
> 
> i thought first i'd name it Controller - but after looking through the code
for
> password manager i found that it's the model that controls the actions of the
> password bubble. vasilii@ what do you think ?

For the password manager there are two classes: ManagePasswordsBubbleModel
(contains as much platform independent code for the bubble as possible) and
ManagePasswordsUIController. The latter does the real stuff.

[mrefaat, 2017-01-05 23:54:42]

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/desktop_i...
File chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_model.h
(right):

https://codereview.chromium.org/2617543002/diff/1/chrome/browser/ui/desktop_i...
chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_model.h:15: class
DesktopIOSPromotionModel {
On 2017/01/05 17:40:20, vasilii wrote:
> On 2017/01/05 00:03:08, mrefaat wrote:
> > On 2017/01/04 20:52:37, sky wrote:
> > > How come you are naming this class as the model? This class doesn't seem
> like
> > a
> > > model from what I can tell.
> > 
> > i thought first i'd name it Controller - but after looking through the code
> for
> > password manager i found that it's the model that controls the actions of
the
> > password bubble. vasilii@ what do you think ?
> 
> For the password manager there are two classes: ManagePasswordsBubbleModel
> (contains as much platform independent code for the bubble as possible) and
> ManagePasswordsUIController. The latter does the real stuff.

i changed it to be controller instead

https://codereview.chromium.org/2617543002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/desktop_ios_promotion/desktop_ios_promotion_view.cc
(right):

https://codereview.chromium.org/2617543002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/desktop_ios_promotion/desktop_ios_promotion_view.cc:28:
}
On 2017/01/05 00:35:21, sky wrote:
> Generally we add
>   // namespace
> for the closing } of a namespace, and it's two spaces, e.g.:
> }  // namespace
> 
> We generally have a newline before and after the {} of the namespace as well,
> but we aren't always consistent so that is up to you.

Done, thanks for the advice

[mrefaat, 2017-01-05 23:59:11]

Description was changed from

==========
Create Desktop to iOS Promotion skeleton.
Add Savepasswords promotion UI under false flag.

screen:
https://drive.google.com/a/google.com/file/d/0B9T1TKmsVG94YVd6cHMwUk9CMVk/view

BUG=676655
==========

to

==========
Create Desktop to iOS Promotion.

Added General skeleton to all windows to ios promotions in:
chrome/browser/ui/desktop_io_promotion/
Which will be responsible for determining the eligibility of users to get the
promotion, executes actions like sending SMS and general logging.

Create Savepasswords desktop to ios promotion UI.
Guarded by false flag to not show, a bubble appears that contains
the promotion text and send to phone call to action and dismiss option (see the
attached screen).

screen:
https://drive.google.com/a/google.com/file/d/0B9T1TKmsVG94YVd6cHMwUk9CMVk/view

BUG=676655
==========

[mrefaat, 2017-01-05 23:59:35]

Description was changed from

==========
Create Desktop to iOS Promotion.

Added General skeleton to all windows to ios promotions in:
chrome/browser/ui/desktop_io_promotion/
Which will be responsible for determining the eligibility of users to get the
promotion, executes actions like sending SMS and general logging.

Create Savepasswords desktop to ios promotion UI.
Guarded by false flag to not show, a bubble appears that contains
the promotion text and send to phone call to action and dismiss option (see the
attached screen).

screen:
https://drive.google.com/a/google.com/file/d/0B9T1TKmsVG94YVd6cHMwUk9CMVk/view

BUG=676655
==========

to

==========
Create Desktop to iOS Promotion.

Added General skeleton to all windows to ios promotions in:
chrome/browser/ui/desktop_io_promotion/
Which will be responsible for determining the eligibility of users
to get the promotion, executes actions like sending SMS and general logging.

Create Savepasswords desktop to ios promotion UI.
Guarded by false flag to not show, a bubble appears that contains
the promotion text and send to phone call to action and dismiss option (see the
attached screen).

screen:
https://drive.google.com/a/google.com/file/d/0B9T1TKmsVG94YVd6cHMwUk9CMVk/view

BUG=676655
==========

[mrefaat, 2017-01-05 23:59:50]

Description was changed from

==========
Create Desktop to iOS Promotion.

Added General skeleton to all windows to ios promotions in:
chrome/browser/ui/desktop_io_promotion/
Which will be responsible for determining the eligibility of users
to get the promotion, executes actions like sending SMS and general logging.

Create Savepasswords desktop to ios promotion UI.
Guarded by false flag to not show, a bubble appears that contains
the promotion text and send to phone call to action and dismiss option (see the
attached screen).

screen:
https://drive.google.com/a/google.com/file/d/0B9T1TKmsVG94YVd6cHMwUk9CMVk/view

BUG=676655
==========

to

==========
Create Desktop to iOS Promotion.

Added General skeleton to all windows to ios promotions in:
chrome/browser/ui/desktop_io_promotion/
Which will be responsible for determining the eligibility of users
to get the promotion, executes actions like sending SMS and general logging.

Create Savepasswords desktop to ios promotion UI.
Guarded by false flag to not show, a bubble appears that contains
the promotion text and send to phone call to action and dismiss option
(see the attached screen).

screen:
https://drive.google.com/a/google.com/file/d/0B9T1TKmsVG94YVd6cHMwUk9CMVk/view

BUG=676655
==========

[mrefaat, 2017-01-06 15:33:17]

The CQ bit was checked by mrefaat@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-06 15:33:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-06 15:56:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-06 15:56:22]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mrefaat, 2017-01-06 16:38:54]

The CQ bit was checked by mrefaat@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-06 16:39:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mrefaat, 2017-01-06 16:55:46]

The CQ bit was unchecked by mrefaat@chromium.org

[mrefaat, 2017-01-06 17:02:39]

Patchset #4 (id:80001) has been deleted

[mrefaat, 2017-01-06 17:03:07]

The CQ bit was checked by mrefaat@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-06 17:03:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-06 17:26:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-06 17:26:21]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mrefaat, 2017-01-06 18:47:36]

Patchset #4 (id:100001) has been deleted

[mrefaat, 2017-01-06 18:47:45]

The CQ bit was checked by mrefaat@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-06 18:48:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-06 20:24:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-06 20:24:58]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[mrefaat, 2017-01-06 23:46:02]

The CQ bit was checked by mrefaat@chromium.org

[mrefaat, 2017-01-06 23:46:03]

The patchset sent to the CQ was uploaded after l-g-t-m from
vasilii@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2617543002/#ps140001
(title: "add cocoa modifications")

[commit-bot: I haz the power, 2017-01-06 23:46:41]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-07 01:07:01]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1483746362822810,
"parent_rev": "02c81cd8f5b5ee59289345c3c73f2953c5708b9e", "commit_rev":
"bd2449f5de94d47bf4b33c439021897a8f3a86d0"}

[commit-bot: I haz the power, 2017-01-07 01:08:31]

Description was changed from

==========
Create Desktop to iOS Promotion.

Added General skeleton to all windows to ios promotions in:
chrome/browser/ui/desktop_io_promotion/
Which will be responsible for determining the eligibility of users
to get the promotion, executes actions like sending SMS and general logging.

Create Savepasswords desktop to ios promotion UI.
Guarded by false flag to not show, a bubble appears that contains
the promotion text and send to phone call to action and dismiss option
(see the attached screen).

screen:
https://drive.google.com/a/google.com/file/d/0B9T1TKmsVG94YVd6cHMwUk9CMVk/view

BUG=676655
==========

to

==========
Create Desktop to iOS Promotion.

Added General skeleton to all windows to ios promotions in:
chrome/browser/ui/desktop_io_promotion/
Which will be responsible for determining the eligibility of users
to get the promotion, executes actions like sending SMS and general logging.

Create Savepasswords desktop to ios promotion UI.
Guarded by false flag to not show, a bubble appears that contains
the promotion text and send to phone call to action and dismiss option
(see the attached screen).

screen:
https://drive.google.com/a/google.com/file/d/0B9T1TKmsVG94YVd6cHMwUk9CMVk/view

BUG=676655

Review-Url: https://codereview.chromium.org/2617543002
Cr-Commit-Position: refs/heads/master@{#442124}
Committed:
https://chromium.googlesource.com/chromium/src/+/bd2449f5de94d47bf4b33c439021...
==========

[commit-bot: I haz the power, 2017-01-07 01:08:35]

Committed patchset #5 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/bd2449f5de94d47bf4b33c439021...

[mrefaat, 2017-03-08 22:15:05]

Patchset #4 (id:120001) has been deleted