[Download] Fix and complete Download API #352

rtxm · 2014-07-11T14:26:34Z

Refactored Filesytem and Download API to share code
related to virtual roots.
Pass TCT tests.
Fix example.

BUG=XWALK-1070

crosswalk-trybot · 2014-07-11T14:26:39Z

Testing patch series with aa18873 as its head.

Bot	Status
tizen-extensions-crosswalk IVI	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/tizen-extensions-crosswalk IVI/builds/233)
tizen-extensions-crosswalk Common	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/tizen-extensions-crosswalk Common/builds/243)

rtxm · 2014-07-11T14:30:29Z

To run the Download API on Tizen you need a fully functional download-provider service (behind the core CAPI). To achieve that, you may need to apply this patch https://review.tizen.org/gerrit/#/c/24123/ and/or refer to Tizen bugs https://bugs.tizen.org/jira/browse/TC-1409 and https://bugs.tizen.org/jira/browse/TC-1415.

crosswalk-trybot · 2014-07-15T14:30:14Z

Testing patch series with d210cfe as its head.

Bot	Status
tizen-extensions-crosswalk Common	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/tizen-extensions-crosswalk Common/builds/250)
tizen-extensions-crosswalk IVI	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/tizen-extensions-crosswalk IVI/builds/240)

rtxm · 2014-07-16T14:11:13Z

Feedback welcome.

deqing · 2014-07-17T02:34:16Z

common/virtualfs.cc

+#include <dirent.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <cassert>


order please

rtxm · 2014-07-17T09:02:29Z

Thank you for your feedback Deqing, I'm going to fix that now.

crosswalk-trybot · 2014-07-17T10:05:32Z

Testing patch series with 7bb1d94 as its head.

Bot	Status
tizen-extensions-crosswalk Common	[FAILED 💔](https://build.crosswalk-project.org/try/builders/tizen-extensions-crosswalk Common/builds/256)
tizen-extensions-crosswalk IVI	[FAILED 💔](https://build.crosswalk-project.org/try/builders/tizen-extensions-crosswalk IVI/builds/246)

crosswalk-trybot · 2014-07-17T11:08:56Z

Testing patch series with 9474cd1 as its head.

Bot	Status
tizen-extensions-crosswalk Common	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/tizen-extensions-crosswalk Common/builds/257)
tizen-extensions-crosswalk IVI	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/tizen-extensions-crosswalk IVI/builds/247)

halton · 2014-07-23T03:14:57Z

common/virtualfs.cc

+  return retval;
+}
+
+bool VirtualFS::GetStorageByLabel(std::string label, Storage& storage) {


std::string label -> const std::string& label

deqing · 2014-07-23T03:15:08Z

LGTM after fix the indent comment. Thanks for the rework.

halton · 2014-07-23T03:23:06Z

common/virtualfs.h

+      const std::string& fullpath = "");
+
+  std::string type() const;
+  std::string state() const;


Above are not simple getter, so please rename them as GetType and GetState

crosswalk-trybot · 2014-08-04T09:46:21Z

Testing patch series with 542b43a as its head.

Bot	Status
tizen-extensions-crosswalk IVI	[FAILED 💔](https://build.crosswalk-project.org/try/builders/tizen-extensions-crosswalk IVI/builds/261)
tizen-extensions-crosswalk Common	[FAILED 💔](https://build.crosswalk-project.org/try/builders/tizen-extensions-crosswalk Common/builds/271)

crosswalk-trybot · 2014-08-04T09:48:03Z

Testing patch series with a49351e as its head.

Bot	Status
tizen-extensions-crosswalk IVI	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/tizen-extensions-crosswalk IVI/builds/262)
tizen-extensions-crosswalk Common	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/tizen-extensions-crosswalk Common/builds/272)

halton · 2014-08-06T07:28:33Z

hi @rtxm please ping us when you're ready for a review. And please rebase your code to resolve the conflict.

clecou · 2014-08-06T07:33:18Z

hi @halton , romuald (rtxm) doesn't work here anymore, that's why I sent a patch according to the latest review. Can you review it before rebasing the patch ?

halton · 2014-08-06T07:58:14Z

@clecou I'm sorry that I generally do not review code that even can not merge. Basically, conflict the merging means there are some changes needed for the rebasing, I need review again after rebasing.

crosswalk-trybot · 2014-08-06T09:16:05Z

Testing patch series with eurogiciel-oss/tizen-extensions-crosswalk@dcd2e7f as its head.

Bot	Status
tizen-extensions-crosswalk IVI	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/tizen-extensions-crosswalk IVI/builds/270)
tizen-extensions-crosswalk Common	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/tizen-extensions-crosswalk Common/builds/280)

clecou · 2014-08-12T12:04:07Z

@halton, ping ?

halton · 2014-08-13T02:47:33Z

common/virtual_fs.h

+  std::string GetType() const;
+  std::string GetState() const;
+  int GetId() const { return id_; }
+  const std::string& GetFullPath() const { return fullpath_; }


nit comment, as chromium does the simple getter normally name as member, which is GetId() -> id(), GetFullPath() -> full_path()

crosswalk-trybot · 2014-08-14T12:43:23Z

Testing patch series with eurogiciel-oss/tizen-extensions-crosswalk@dd921e7 as its head.

Bot	Status
tizen-extensions-crosswalk IVI	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/tizen-extensions-crosswalk IVI/builds/276)
tizen-extensions-crosswalk Common	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/tizen-extensions-crosswalk Common/builds/286)

clecou · 2014-08-14T12:51:04Z

@halton, @deqing, thanks for the review.
Please find a new patch taking into consideration all of your remarks except ones about static members and const methods (related are in previous patch review).

halton · 2014-08-15T07:46:01Z

common/virtual_fs.cc

+
+}  // namespace
+
+


one empty line

Refactored Filesytem and Download API to share code related to virtual roots. Pass TCT tests. Fix example. BUG=XWALK-1070

crosswalk-trybot · 2014-08-18T09:02:51Z

Testing patch series with eurogiciel-oss/tizen-extensions-crosswalk@bcee207 as its head.

Bot	Status
tizen-extensions-crosswalk IVI	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/tizen-extensions-crosswalk IVI/builds/278)
tizen-extensions-crosswalk Common	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/tizen-extensions-crosswalk Common/builds/288)

clecou · 2014-08-18T09:08:47Z

@halton, with the vfs_const namespace.

halton · 2014-08-18T14:13:26Z

lgtm now, will merge. Thanks for your patient reworking.

[Download] Fix and complete Download API

deqing reviewed Jul 17, 2014
View reviewed changes

common/virtualfs.cc

#include <dirent.h>

#include <sys/stat.h>

#include <sys/types.h>

#include <cassert>

Copy link

Contributor

deqing Jul 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

order please

halton reviewed Jul 23, 2014
View reviewed changes

halton reviewed Aug 13, 2014
View reviewed changes

halton reviewed Aug 15, 2014
View reviewed changes

common/virtual_fs.cc

} // namespace

Copy link

halton Aug 15, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one empty line

[Download] Fix and complete Download API

bcee207

Refactored Filesytem and Download API to share code related to virtual roots. Pass TCT tests. Fix example. BUG=XWALK-1070

halton pushed a commit that referenced this pull request Aug 18, 2014

Merge pull request #352 from eurogiciel-oss/XWALK-1070

b2b6999

[Download] Fix and complete Download API

halton merged commit b2b6999 into crosswalk-project:master Aug 18, 2014

clecou deleted the XWALK-1070 branch September 2, 2014 17:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Download] Fix and complete Download API #352

[Download] Fix and complete Download API #352

rtxm commented Jul 11, 2014

crosswalk-trybot commented Jul 11, 2014

rtxm commented Jul 11, 2014

crosswalk-trybot commented Jul 15, 2014

rtxm commented Jul 16, 2014

deqing Jul 17, 2014

rtxm commented Jul 17, 2014

crosswalk-trybot commented Jul 17, 2014

crosswalk-trybot commented Jul 17, 2014

halton Jul 23, 2014

deqing commented Jul 23, 2014

halton Jul 23, 2014

crosswalk-trybot commented Aug 4, 2014

crosswalk-trybot commented Aug 4, 2014

halton commented Aug 6, 2014

clecou commented Aug 6, 2014

halton commented Aug 6, 2014

crosswalk-trybot commented Aug 6, 2014

clecou commented Aug 12, 2014

halton Aug 13, 2014

crosswalk-trybot commented Aug 14, 2014

clecou commented Aug 14, 2014

halton Aug 15, 2014

crosswalk-trybot commented Aug 18, 2014

clecou commented Aug 18, 2014

halton commented Aug 18, 2014

[Download] Fix and complete Download API #352

[Download] Fix and complete Download API #352

Conversation

rtxm commented Jul 11, 2014

crosswalk-trybot commented Jul 11, 2014

rtxm commented Jul 11, 2014

crosswalk-trybot commented Jul 15, 2014

rtxm commented Jul 16, 2014

deqing Jul 17, 2014

Choose a reason for hiding this comment

rtxm commented Jul 17, 2014

crosswalk-trybot commented Jul 17, 2014

crosswalk-trybot commented Jul 17, 2014

halton Jul 23, 2014

Choose a reason for hiding this comment

deqing commented Jul 23, 2014

halton Jul 23, 2014

Choose a reason for hiding this comment

crosswalk-trybot commented Aug 4, 2014

crosswalk-trybot commented Aug 4, 2014

halton commented Aug 6, 2014

clecou commented Aug 6, 2014

halton commented Aug 6, 2014

crosswalk-trybot commented Aug 6, 2014

clecou commented Aug 12, 2014

halton Aug 13, 2014

Choose a reason for hiding this comment

crosswalk-trybot commented Aug 14, 2014

clecou commented Aug 14, 2014

halton Aug 15, 2014

Choose a reason for hiding this comment

crosswalk-trybot commented Aug 18, 2014

clecou commented Aug 18, 2014

halton commented Aug 18, 2014