Settings: Fix some BrowserProxy compile issues.

Settings: Fix some BrowserProxy compile issues.

BUG=NONE
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/2097443addfa17431c95d89578e37d9912e30297
Cr-Commit-Position: refs/heads/master@{#403381}

[tommycli, 2016-06-30 22:16:55]

Description was changed from

==========
Settings: Fix some BrowserProxy compile issues.

BUG=NONE
==========

to

==========
Settings: Fix some BrowserProxy compile issues.

BUG=NONE
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[tommycli, 2016-06-30 22:17:14]

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

[commit-bot: I haz the power, 2016-06-30 22:17:46]

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

[tommycli, 2016-06-30 22:19:44]

tommycli@chromium.org changed reviewers:
+ dschuyler@chromium.org

[tommycli, 2016-06-30 22:19:45]

dschuyler: PTAL. This brings the People and Site Settings sections up to the
'correct way' of compiling browser proxies now.

It also contains some fixes, since the typechecking wasn't working before.
Thanks.

https://codereview.chromium.org/2110663007/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/site_settings/site_settings_prefs_browser_proxy.js
(right):

https://codereview.chromium.org/2110663007/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/site_settings_prefs_browser_proxy.js:50:
* @param {string} contentType The name of the category to change.
I have confirmed (by looking in the C++ handler) that these should all actually
be strings.

[dschuyler, 2016-06-30 22:26:33]

https://codereview.chromium.org/2110663007/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/people_page/sync_page.js (right):

https://codereview.chromium.org/2110663007/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/people_page/sync_page.js:81: * @type
{?settings.SyncPrefs}
Is it workable to set the type here to
  {!settings.SyncPrefs}
instead of doing the
    if (!this.syncPrefs)
below?

https://codereview.chromium.org/2110663007/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/site_settings/site_settings_prefs_browser_proxy.js
(right):

https://codereview.chromium.org/2110663007/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/site_settings_prefs_browser_proxy.js:50:
* @param {string} contentType The name of the category to change.
On 2016/06/30 22:19:45, tommycli wrote:
> I have confirmed (by looking in the C++ handler) that these should all
actually
> be strings.

Acknowledged.

[tommycli, 2016-06-30 22:32:52]

https://codereview.chromium.org/2110663007/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/people_page/sync_page.js (right):

https://codereview.chromium.org/2110663007/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/people_page/sync_page.js:81: * @type
{?settings.SyncPrefs}
On 2016/06/30 22:26:33, dschuyler wrote:
> Is it workable to set the type here to
>   {!settings.SyncPrefs}
> instead of doing the
>     if (!this.syncPrefs)
> below?

No, because it's genuinely undefined until it's initialized by the c++ handler.

However, i did change this to an assert, since this can never reasonably happen.
That might be cleaner.

[Dan Beam, 2016-06-30 23:38:01]

https://codereview.chromium.org/2110663007/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/people_page/sync_page.js (right):

https://codereview.chromium.org/2110663007/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/people_page/sync_page.js:81: * @type
{?settings.SyncPrefs}
On 2016/06/30 22:32:51, tommycli wrote:
> On 2016/06/30 22:26:33, dschuyler wrote:
> > Is it workable to set the type here to
> >   {!settings.SyncPrefs}
> > instead of doing the
> >     if (!this.syncPrefs)
> > below?
> 
> No, because it's genuinely undefined until it's initialized by the c++
handler.
> 
> However, i did change this to an assert, since this can never reasonably
happen.
> That might be cleaner.

fyi: ?Type means Type|null

This seems like Type|undefined as it doesn't explicitly set the initial value:
null

[tommycli, 2016-06-30 23:51:18]

good tip, thanks!

https://codereview.chromium.org/2110663007/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/people_page/sync_page.js (right):

https://codereview.chromium.org/2110663007/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/people_page/sync_page.js:81: * @type
{?settings.SyncPrefs}
On 2016/06/30 23:38:00, Dan Beam wrote:
> On 2016/06/30 22:32:51, tommycli wrote:
> > On 2016/06/30 22:26:33, dschuyler wrote:
> > > Is it workable to set the type here to
> > >   {!settings.SyncPrefs}
> > > instead of doing the
> > >     if (!this.syncPrefs)
> > > below?
> > 
> > No, because it's genuinely undefined until it's initialized by the c++
> handler.
> > 
> > However, i did change this to an assert, since this can never reasonably
> happen.
> > That might be cleaner.
> 
> fyi: ?Type means Type|null
> 
> This seems like Type|undefined as it doesn't explicitly set the initial value:
> null

Done.

[dschuyler, 2016-07-01 00:32:11]

lgtm

[tommycli, 2016-07-01 00:48:31]

The CQ bit was checked by tommycli@chromium.org

[commit-bot: I haz the power, 2016-07-01 00:48:50]

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, 2016-07-01 01:33:41]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-07-01 01:36:24]

Description was changed from

==========
Settings: Fix some BrowserProxy compile issues.

BUG=NONE
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Settings: Fix some BrowserProxy compile issues.

BUG=NONE
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/2097443addfa17431c95d89578e37d9912e30297
Cr-Commit-Position: refs/heads/master@{#403381}
==========

[commit-bot: I haz the power, 2016-07-01 01:36:25]

Patchset 3 (id:??) landed as
https://crrev.com/2097443addfa17431c95d89578e37d9912e30297
Cr-Commit-Position: refs/heads/master@{#403381}