Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(27)

Issue 1679002: Possible fix / workaround for issue where infobar fails to show... (Closed)

Created:
8 years, 11 months ago by joth
Modified:
7 years, 10 months ago
CC:
chromium-reviews, John Grabowski, pam+watch_chromium.org, ben+cc_chromium.org
Visibility:
Public.

Description

Possible fix / workaround for issue where infobar fails to show BUG=41712 TEST=as per bug comment 8 http://crbug.com/41712#c8 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=45295

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -1 line) Patch
M chrome/browser/cocoa/infobar_container_controller.mm View 1 1 chunk +2 lines, -1 line 1 comment Download

Messages

Total messages: 3 (0 generated)
joth
Hi rohitrao I think you're the expert on this code, would you have a look ...
8 years, 11 months ago (2010-04-20 03:23:39 UTC) #1
rohitrao (ping after 24h)
Heh, has this been broken forever? Wonder why no one noticed it before. LGTM http://codereview.chromium.org/1679002/diff/2001/3001 ...
8 years, 11 months ago (2010-04-21 14:00:52 UTC) #2
joth
8 years, 11 months ago (2010-04-22 11:27:15 UTC) #3
Thanks! Committed.

Yeah, I guess this was always there. I suppose not so many flows completely rely
on infobars compared so geolocation, so it was just not noticed.

See your point about the logic differing, but this seems the simplest way to do
it here and as you say, it should work fine.

Cheers

On 2010/04/21 14:00:52, rohitrao wrote:
> Heh, has this been broken forever?  Wonder why no one noticed it before.
> 
> LGTM
> 
> http://codereview.chromium.org/1679002/diff/2001/3001
> File chrome/browser/cocoa/infobar_container_controller.mm (right):
> 
> http://codereview.chromium.org/1679002/diff/2001/3001#newcode126
> chrome/browser/cocoa/infobar_container_controller.mm:126: if
> (currentTabContents_ == contents)
> This doesn't quite match the logic in browser_view.cc, but it should work
fine.

Powered by Google App Engine
This is Rietveld 408576698