Issue 6776019: [cros] Create SIM unlock dialog wrapper.

Issue 6776019: [cros] Create SIM unlock dialog wrapper. (Closed)

Created:
9 years, 8 months ago by Nikita (slow)

Modified:
9 years, 7 months ago

Reviewers:
whywhat

CC:
chromium-reviews, arv (Not doing code reviews), davemoore+watch_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

Description

[cros] Create SIM unlock dialog wrapper. BUG=chromium-os:12007 TEST=Manual, displayed dialog, went through all states. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=79984

Patch Set 3 : fix #

Created: 9 years, 8 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+149 lines, -7 lines)			Patch
M	chrome/browser/chromeos/cros/network_library.cc	View		2 chunks	+2 lines, -2 lines	0 comments	Download
A	chrome/browser/chromeos/sim_unlock_dialog_delegate.h	View	1	1 chunk	+41 lines, -0 lines	0 comments	Download
A	chrome/browser/chromeos/sim_unlock_dialog_delegate.cc	View	1 2	1 chunk	+96 lines, -0 lines	0 comments	Download
M	chrome/browser/chromeos/status/network_menu.cc	View		2 chunks	+4 lines, -1 line	0 comments	Download
M	chrome/browser/resources/chromeos/sim_unlock.css	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/resources/chromeos/sim_unlock.js	View	1	2 chunks	+2 lines, -2 lines	0 comments	Download
M	chrome/browser/ui/webui/chromeos/sim_unlock_ui.cc	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/chrome_browser.gypi	View	1 2	1 chunk	+2 lines, -0 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

whywhat

LGTM http://codereview.chromium.org/6776019/diff/2001/chrome/browser/chromeos/sim_unlock_dialog_delegate.cc File chrome/browser/chromeos/sim_unlock_dialog_delegate.cc (right): http://codereview.chromium.org/6776019/diff/2001/chrome/browser/chromeos/sim_unlock_dialog_delegate.cc#newcode14 chrome/browser/chromeos/sim_unlock_dialog_delegate.cc:14: // Default width/height of the dialog. Empty line ...

9 years, 8 months ago (2011-03-30 19:12:15 UTC) #2

Nikita (slow)

http://codereview.chromium.org/6776019/diff/2001/chrome/browser/chromeos/sim_unlock_dialog_delegate.cc File chrome/browser/chromeos/sim_unlock_dialog_delegate.cc (right): http://codereview.chromium.org/6776019/diff/2001/chrome/browser/chromeos/sim_unlock_dialog_delegate.cc#newcode68 chrome/browser/chromeos/sim_unlock_dialog_delegate.cc:68: std::vector<WebUIMessageHandler*>* handlers) const {} On 2011/03/30 19:12:15, whywhat wrote: ...

9 years, 8 months ago (2011-03-30 19:47:55 UTC) #3

whywhat

9 years, 8 months ago (2011-03-30 20:05:29 UTC) #4

Nikita (slow)

9 years, 8 months ago (2011-03-31 08:44:17 UTC) #5

http://codereview.chromium.org/6776019/diff/2001/chrome/browser/chromeos/sim_...
File chrome/browser/chromeos/sim_unlock_dialog_delegate.cc (right):

http://codereview.chromium.org/6776019/diff/2001/chrome/browser/chromeos/sim_...
chrome/browser/chromeos/sim_unlock_dialog_delegate.cc:14: // Default
width/height of the dialog.
On 2011/03/30 19:12:15, whywhat wrote:
> Empty line before?

Done.

http://codereview.chromium.org/6776019/diff/2001/chrome/browser/chromeos/sim_...
chrome/browser/chromeos/sim_unlock_dialog_delegate.cc:24: virtual
~HtmlDialogWithoutContextMenuView() {}
On 2011/03/30 19:12:15, whywhat wrote:
> I think you can live with auto generated one.

Done.

http://codereview.chromium.org/6776019/diff/2001/chrome/browser/chromeos/sim_...
chrome/browser/chromeos/sim_unlock_dialog_delegate.cc:68:
std::vector<WebUIMessageHandler*>* handlers) const {}
On 2011/03/30 20:05:29, whywhat wrote:
> On 2011/03/30 19:47:55, Nikita Kostylev wrote:
> > On 2011/03/30 19:12:15, whywhat wrote:
> > > Could you move the closing bracket to the next line or define the method
in
> > > header file?
> > 
> > That would mean that we're inlining virtual function.
> > Is this OK in case when it has empty body?
> > http://dev.chromium.org/developers/coding-style#TOC-Inline-functions
> 
> Well, I think it's better to leave it in .cc file then.

Done.

http://codereview.chromium.org/6776019/diff/2001/chrome/browser/chromeos/sim_...
chrome/browser/chromeos/sim_unlock_dialog_delegate.cc:68:
std::vector<WebUIMessageHandler*>* handlers) const {}
On 2011/03/30 19:12:15, whywhat wrote:
> Could you move the closing bracket to the next line or define the method in
> header file?

Done.

http://codereview.chromium.org/6776019/diff/2001/chrome/browser/chromeos/sim_...
chrome/browser/chromeos/sim_unlock_dialog_delegate.cc:81: return;
On 2011/03/30 20:05:29, whywhat wrote:
> You don't need return here.

Done.

Expand Messages | Collapse Messages