Add IsBootPath to MountLibrary. Additional boot path check in imageburner so ...

Add IsBootPath to MountLibrary. Additional boot path check in imageburner so boot drive is not presented as a target.
BUG=chromium-os:6788
TEST=chrome://imageburner should not display /dev/sda


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=72733

[xiyuan, 2011-01-25 17:28:29]

http://codereview.chromium.org/6348006/diff/3002/chrome/browser/chromeos/dom_...
File chrome/browser/chromeos/dom_ui/imageburner_ui.cc (right):

http://codereview.chromium.org/6348006/diff/3002/chrome/browser/chromeos/dom_...
chrome/browser/chromeos/dom_ui/imageburner_ui.cc:42: :
DataSource(chrome::kChromeUIImageBurnerHost, MessageLoop::current()) {
nit: indent 4 more spaces

http://codereview.chromium.org/6348006/diff/3002/chrome/browser/chromeos/dom_...
chrome/browser/chromeos/dom_ui/imageburner_ui.cc:117: : handler_(handler) {
nit: indent 4 more spaces

http://codereview.chromium.org/6348006/diff/3002/chrome/browser/chromeos/dom_...
chrome/browser/chromeos/dom_ui/imageburner_ui.cc:263: if
(download_item_observer_added_) // Already added.
nit: two spaces before comment at line end

http://codereview.chromium.org/6348006/diff/3002/chrome/browser/chromeos/dom_...
chrome/browser/chromeos/dom_ui/imageburner_ui.cc:479: image_url_fetched_(true) {
any reason why this needs to be changed?

http://codereview.chromium.org/6348006/diff/3002/chrome/browser/chromeos/dom_...
File chrome/browser/chromeos/dom_ui/imageburner_ui.h (right):

http://codereview.chromium.org/6348006/diff/3002/chrome/browser/chromeos/dom_...
chrome/browser/chromeos/dom_ui/imageburner_ui.h:40: static const std::string
kTempImageFolderName = "chromeos_image";
Since you touch this file, could we get rid of the those std::string and move
them into cc file? It looks like const char [] should be good enough for those
consts and does not need to live in the header file.

[achuithb, 2011-01-25 22:21:45]

http://codereview.chromium.org/6348006/diff/3002/chrome/browser/chromeos/dom_...
File chrome/browser/chromeos/dom_ui/imageburner_ui.cc (right):

http://codereview.chromium.org/6348006/diff/3002/chrome/browser/chromeos/dom_...
chrome/browser/chromeos/dom_ui/imageburner_ui.cc:42: :
DataSource(chrome::kChromeUIImageBurnerHost, MessageLoop::current()) {
On 2011/01/25 17:28:30, xiyuan wrote:
> nit: indent 4 more spaces 

Done.

http://codereview.chromium.org/6348006/diff/3002/chrome/browser/chromeos/dom_...
chrome/browser/chromeos/dom_ui/imageburner_ui.cc:117: : handler_(handler) {
On 2011/01/25 17:28:30, xiyuan wrote:
> nit: indent 4 more spaces

Done.

http://codereview.chromium.org/6348006/diff/3002/chrome/browser/chromeos/dom_...
chrome/browser/chromeos/dom_ui/imageburner_ui.cc:263: if
(download_item_observer_added_) // Already added.
On 2011/01/25 17:28:30, xiyuan wrote:
> nit: two spaces before comment at line end

Done.

http://codereview.chromium.org/6348006/diff/3002/chrome/browser/chromeos/dom_...
chrome/browser/chromeos/dom_ui/imageburner_ui.cc:479: image_url_fetched_(true) {
imageburner was written to use an internal corp url for download. We start out
with image_url_ empty and image_url_fetched_ false. Then we download the config
file to figure out what the image_url_ is, and then set image_url_fetched_ to
true.

There is currently a canonical url where the image can be downloaded, so
downloading the config file is not necessary right now. I will probably need to
download a different config file in the near future, so I haven't removed all
the code that downloads it.

http://codereview.chromium.org/6348006/diff/3002/chrome/browser/chromeos/dom_...
File chrome/browser/chromeos/dom_ui/imageburner_ui.h (right):

http://codereview.chromium.org/6348006/diff/3002/chrome/browser/chromeos/dom_...
chrome/browser/chromeos/dom_ui/imageburner_ui.h:40: static const std::string
kTempImageFolderName = "chromeos_image";
Good point - moved them to .cc. It's convenient for some of these to be
std::string because they are used in string context - otherwise I'd need to
create a tmp string object each time. The rest I've converted to const char*
const.

[xiyuan, 2011-01-25 22:37:22]

http://codereview.chromium.org/6348006/diff/20001/chrome/browser/chromeos/dom...
File chrome/browser/chromeos/dom_ui/imageburner_ui.cc (right):

http://codereview.chromium.org/6348006/diff/20001/chrome/browser/chromeos/dom...
chrome/browser/chromeos/dom_ui/imageburner_ui.cc:37:
"http://chrome-master.mtv.corp.google.com/chromeos/dev-channel/";
We should avoid complex object in global scope when possible. When you need
std::string, you can construct the temp obj by e.g. std::string(kImageBaseURL).
I still think const char* is better here.

http://codereview.chromium.org/6348006/diff/20001/chrome/browser/chromeos/dom...
chrome/browser/chromeos/dom_ui/imageburner_ui.cc:42: static const char* const
kTempImageFolderName = "chromeos_image";
Please use const char[] to be consistent.
See chrome/common/pref_names.h/cc for example.

[achuithb, 2011-01-25 23:12:36]

Done

On 2011/01/25 22:37:22, xiyuan wrote:
>
http://codereview.chromium.org/6348006/diff/20001/chrome/browser/chromeos/dom...
> File chrome/browser/chromeos/dom_ui/imageburner_ui.cc (right):
> 
>
http://codereview.chromium.org/6348006/diff/20001/chrome/browser/chromeos/dom...
> chrome/browser/chromeos/dom_ui/imageburner_ui.cc:37:
> "http://chrome-master.mtv.corp.google.com/chromeos/dev-channel/";
> We should avoid complex object in global scope when possible. When you need
> std::string, you can construct the temp obj by e.g.
std::string(kImageBaseURL).
> I still think const char* is better here.
> 
>
http://codereview.chromium.org/6348006/diff/20001/chrome/browser/chromeos/dom...
> chrome/browser/chromeos/dom_ui/imageburner_ui.cc:42: static const char* const
> kTempImageFolderName = "chromeos_image";
> Please use const char[] to be consistent.
> See chrome/common/pref_names.h/cc for example.

[xiyuan, 2011-01-25 23:16:53]

LGTM

Thank you for making all the changes. :)

[achuithb, 2011-01-25 23:19:18]

Thank you for the suggestions and the careful review! Helps familiarize me with
google coding guidelines.