[OriginChip] Don't crash for invalid extension URL

[OriginChip] Don't crash for invalid extension URL

If the user enters a chrome-extension:// url with an invalid extension
id, let's not crash. That's rude.

Instead, origin chip will display <id> as extension name, show a "page
not available" message.

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

[groby-ooo-7-16, 2014-05-21 19:33:40]

PTAL. Screenshot is on bug. Tests will be separate CL, to minimize merge size.

[groby-ooo-7-16, 2014-05-21 21:35:19]

Now with actual reviewer assigned...

Peter: PTAL?

[groby-ooo-7-16, 2014-05-22 16:49:49]

Friendly ping...

[Peter Kasting, 2014-05-22 17:30:29]

https://codereview.chromium.org/291173008/diff/1/chrome/browser/ui/toolbar/or...
File chrome/browser/ui/toolbar/origin_chip_info.cc (right):

https://codereview.chromium.org/291173008/diff/1/chrome/browser/ui/toolbar/or...
chrome/browser/ui/toolbar/origin_chip_info.cc:124: extension_icon_image_.reset(
It seems like an even simpler change would have been to just conditionalize this
code, so we would have:

  if (displayed_url_.SchemeIs(extensions::kExtensionScheme)) {
    extension = ...;
    if (extension) {
      icon_ = IDR_EXTENSIONS_FAVICON;
      extension_icon_image_.reset(...);
      ...
      return true;
    }
  }

  if (extension_icon_image_) {
    ...

[groby-ooo-7-16, 2014-05-22 23:12:59]

PTAL

https://codereview.chromium.org/291173008/diff/1/chrome/browser/ui/toolbar/or...
File chrome/browser/ui/toolbar/origin_chip_info.cc (right):

https://codereview.chromium.org/291173008/diff/1/chrome/browser/ui/toolbar/or...
chrome/browser/ui/toolbar/origin_chip_info.cc:124: extension_icon_image_.reset(
On 2014/05/22 17:30:29, Peter Kasting wrote:
> It seems like an even simpler change would have been to just conditionalize
this
> code, so we would have:
> 
>   if (displayed_url_.SchemeIs(extensions::kExtensionScheme)) {
>     extension = ...;
>     if (extension) {
>       icon_ = IDR_EXTENSIONS_FAVICON;
>       extension_icon_image_.reset(...);
>       ...
>       return true;
>     }
>   }
> 
>   if (extension_icon_image_) {
>     ...

Done.

[Peter Kasting, 2014-05-22 23:26:52]

LGTM

[groby-ooo-7-16, 2014-05-22 23:28:16]

The CQ bit was checked by groby@chromium.org

[commit-bot: I haz the power, 2014-05-22 23:29:11]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/groby@chromium.org/291173008/20001

[commit-bot: I haz the power, 2014-05-23 03:04:56]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-23 06:09:50]

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

[commit-bot: I haz the power, 2014-05-23 06:09:51]

Try jobs failed on following builders:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[groby-ooo-7-16, 2014-05-27 17:57:47]

Hm. Can't repro the failures locally at all. Giving the CQ another shot, also
pinged owner of tests.

[groby-ooo-7-16, 2014-05-27 17:57:51]

The CQ bit was checked by groby@chromium.org

[commit-bot: I haz the power, 2014-05-27 17:58:23]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/groby@chromium.org/291173008/20001

[commit-bot: I haz the power, 2014-05-27 20:53:56]

Change committed as 273020