Initial commit for Chrome metainstaller on Mac.

chrome/installer/mac/app/downloader.h

Index: chrome/installer/mac/app/downloader.h
diff --git a/chrome/installer/mac/app/downloader.h b/chrome/installer/mac/app/downloader.h
new file mode 100644
index 0000000000000000000000000000000000000000..639fab4f3c70dc3517f66933cc496260d52f2539
--- /dev/null
+++ b/chrome/installer/mac/app/downloader.h
@@ -0,0 +1,49 @@
+// Copyright (c) 2016 The Chromium Authors. All rights reserved.

[Mark Mentovai, 2016/06/28 15:42:50]

Minor nit: we don’t put the “(c)” in anymore.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_INSTALLER_MAC_APP_DOWNLOADER_H_
+#define CHROME_INSTALLER_MAC_APP_DOWNLOADER_H_
+
+#import <Foundation/Foundation.h>
+
+@interface Downloader : NSObject <NSXMLParserDelegate>
+@property(nonatomic, copy) NSMutableArray *chromeDownloadURLs;

[Mark Mentovai, 2016/06/28 15:42:50]

Nit: Chrome style is to put the * on the type name, not the variable. This is
actually what you’ve done elsewhere, like in
“(void)parseXML:(NSData*)omahaResponseXML” below.

+@property(nonatomic, copy) NSString *chromeImageFilename;
+
+// Prepares the array for use in the following methods.

[Mark Mentovai, 2016/06/28 15:42:50]

A comment isn’t normally needed for -init methods, especially when they don’t
take any arguments. It’s an Objective-C convention that an object isn’t usable
until an -init method has been called.

+- (id)init;
+
+// Will parse the XML passed into it and extract all the url's it finds as well

[Mark Mentovai, 2016/06/28 15:42:50]

Comments should be written more directly. Instead of “will x, then will y”, you
can just say “X and Y.” It’s easier and faster to read if it says “Parses
omahaResponseXML and extracts …, then adds …”

[Mark Mentovai, 2016/06/28 15:42:50]

URLs, not url's. Elsewhere, too.

+// as the filename. Will then add each url into the array of url's and report

[Mark Mentovai, 2016/06/28 15:42:50]

How does it report errors? It has a void return value, so this isn’t obvious.

+// any errors it had.
+- (void)parseXML:(NSData*)omahaResponseXML;
+
+// The URL's we got from the previous method are incomplete and need the

[Mark Mentovai, 2016/06/28 15:42:50]

go/avoidwe

+// filename (which is the same for all the links) appended to the end.
+// This method iterates through the array and appends the filename to each URL.
+- (void)appendFilenameToURL;

[Mark Mentovai, 2016/06/28 15:42:50]

Not sure this makes sense to expose from this class. Usually, when you see that
a method is only used in the .mm file but isn’t called by anyone else, it makes
sense to take it out of the header. One option is to stick it into a private
category in the .mm file to hide it from the rest of the world.

+
+// Expects the NSData object passed in to be an Omaha XML response file

[Mark Mentovai, 2016/06/28 15:42:50]

Start with a description of what this method does, and then deal with parameters
and expectations. Like:

“Calls the XML parser, extracts URLs, and returns the first one.
chromeImageAsData is the XML response from Omaha.”

You can just call it “XML” or an “XML response.” Saying “file” makes it sound
like it’s been written to disk somewhere, but we’ve just got the XML in memory.

+// Will then call the XML parser and extract the four URL's to the current

[Mark Mentovai, 2016/06/28 15:42:50]

Why four? It might not always be four.

+// version of Chrome and return the first entry in that array
+// TODO(ivan): check the URL's actually work ie. response is a 200/202 and
+// return the first working URL.
+- (NSURL*)getChromeImageURL: (NSData*)chromeImageAsData;

[Mark Mentovai, 2016/06/28 15:42:50]

Style nit: no space after the colon. Do it like you did with -parseXML.

[Mark Mentovai, 2016/06/28 15:42:50]

but I think that chromeImageAsData is a misleading name here.

+
+// Takes the URL from getChromeImageURL and packages it into a NSData for easier

[Mark Mentovai, 2016/06/28 15:42:50]

Not sure what this does.

+// downloading.
+- (NSData*)downloadChromeAsData: (NSURL*)chromeURL;
+
+// Returns a path to an individuals home download folder.

[Mark Mentovai, 2016/06/28 15:42:50]

an individuals→the user’s

+- (NSString*)getDownloadsFilePath;
+
+// Uses the packaged data from before as well as the filepath from before to

[Mark Mentovai, 2016/06/28 15:42:50]

“before” = when?

+// download the chrome install image to the downloads folder.
+- (void)writeChromeImageToDownloadsDirectory: (NSData*)chromeImageAsData;
+
+- (void)downloadChromeImageToDownloadsDirectory: (NSData*)responseXMLData;

[Mark Mentovai, 2016/06/28 15:42:50]

What’s this do?

+
+@end
+
+#endif // CHROME_INSTALLER_MAC_APP_DOWNLOADER_H_