Fix ZoomController crash on kiosk launching.

Fix ZoomController crash on kiosk launching.

BUG=418214
Committed: https://crrev.com/2a49d598998386086b0f542fae378fe91bd1904a
Cr-Commit-Position: refs/heads/master@{#297094}

[xiyuan, 2014-09-26 20:02:43]

xiyuan@chromium.org changed reviewers:
+ wjmaclean@chromium.org

[xiyuan, 2014-09-26 20:02:43]

Let me know what you think. Thanks.

[wjmaclean, 2014-09-26 21:05:44]

https://codereview.chromium.org/609983002/diff/1/chrome/browser/ui/zoom/zoom_...
File chrome/browser/ui/zoom/zoom_controller.cc (right):

https://codereview.chromium.org/609983002/diff/1/chrome/browser/ui/zoom/zoom_...
chrome/browser/ui/zoom/zoom_controller.cc:85: entry && entry->GetPageType() ==
content::PAGE_TYPE_NORMAL;
If there's no committed entry, what are we zooming?

It seems like the proper thing to do here would be to exit without applying
zoom. Or is there something about kiosk mode that allows it to display valid
content without a committed entry? (In which case, what value of "host" is being
passed to the HostZoomMap?)

[xiyuan, 2014-09-26 21:39:37]

https://codereview.chromium.org/609983002/diff/1/chrome/browser/ui/zoom/zoom_...
File chrome/browser/ui/zoom/zoom_controller.cc (right):

https://codereview.chromium.org/609983002/diff/1/chrome/browser/ui/zoom/zoom_...
chrome/browser/ui/zoom/zoom_controller.cc:85: entry && entry->GetPageType() ==
content::PAGE_TYPE_NORMAL;
On 2014/09/26 21:05:44, wjmaclean wrote:
> If there's no committed entry, what are we zooming?
> 
> It seems like the proper thing to do here would be to exit without applying
> zoom. Or is there something about kiosk mode that allows it to display valid
> content without a committed entry? (In which case, what value of "host" is
being
> passed to the HostZoomMap?)

If there is no committed entry, |is_normal_page| would false and we bail out in
line 91, right?

[wjmaclean, 2014-09-26 23:48:40]

lgtm!

https://codereview.chromium.org/609983002/diff/1/chrome/browser/ui/zoom/zoom_...
File chrome/browser/ui/zoom/zoom_controller.cc (right):

https://codereview.chromium.org/609983002/diff/1/chrome/browser/ui/zoom/zoom_...
chrome/browser/ui/zoom/zoom_controller.cc:85: entry && entry->GetPageType() ==
content::PAGE_TYPE_NORMAL;
On 2014/09/26 21:39:37, xiyuan wrote:
> On 2014/09/26 21:05:44, wjmaclean wrote:
> > If there's no committed entry, what are we zooming?
> > 
> > It seems like the proper thing to do here would be to exit without applying
> > zoom. Or is there something about kiosk mode that allows it to display valid
> > content without a committed entry? (In which case, what value of "host" is
> being
> > passed to the HostZoomMap?)
> 
> If there is no committed entry, |is_normal_page| would false and we bail out
in
> line 91, right?

Sorry ... I realized shortly after I left the office that I ha read that line
wrong.

[xiyuan, 2014-09-27 02:04:40]

Cool. Thank you for the quick review.

[xiyuan, 2014-09-27 02:04:55]

The CQ bit was checked by xiyuan@chromium.org

[commit-bot: I haz the power, 2014-09-27 02:06:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/609983002/1

[commit-bot: I haz the power, 2014-09-27 02:18:29]

Committed patchset #1 (id:1) as cd42caba6a01af91754d31156de8a5f992a609c3

[commit-bot: I haz the power, 2014-09-27 02:19:12]

Patchset 1 (id:??) landed as
https://crrev.com/2a49d598998386086b0f542fae378fe91bd1904a
Cr-Commit-Position: refs/heads/master@{#297094}