|
|
Created:
4 years ago by chanpatorikku Modified:
4 years ago CC:
chromium-reviews, binji+watch_chromium.org, Sam Clegg Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[NaCl SDK] Implement FakePepperInterfaceGoogleDriveFs
Implement FakePepperInterfaceGoogleDriveFs, and implement
FakeDriveURLResponseInfoInterface and FakeDriveURLLoaderInterface, which
FakePepperInterfaceGoogleDriveFs uses. FakePepperInterfaceGoogleDriveFs
is going to be used by the unit tests of the future code.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_nacl_sdk;master.tryserver.chromium.mac:mac_nacl_sdk;master.tryserver.chromium.win:win_nacl_sdk
Committed: https://crrev.com/7d7031e3d92025246c5f296a36727c77f5bdcbd0
Cr-Commit-Position: refs/heads/master@{#438081}
Patch Set 1 #
Total comments: 28
Patch Set 2 #Patch Set 3 #
Total comments: 1
Patch Set 4 #
Messages
Total messages: 22 (9 generated)
Description was changed from ========== diff files ========== to ========== [NaCl SDK] Implement FakePepperInterfaceGoogleDriveFs Implement FakePepperInterfaceGoogleDriveFs, and implement FakeDriveURLResponseInfoInterface and FakeDriveURLLoaderInterface, which FakePepperInterfaceGoogleDriveFs uses. FakePepperInterfaceGoogleDriveFs is going to be used by the unit tests of the future code. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_nacl_sdk;master.tryserver.chromium.mac:mac_nacl_sdk;master.tryserver.chromium.win:win_nacl_sdk ==========
chanpatorikku@gmail.com changed reviewers: + binji@chromium.org, sbc@chromium.org
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
GetBodyAsFileRef was not implemented in the previous CL and is implemented as FakeDriveURLResponseInfoInterface::GetBodyAsFileRef in the current review. Just in case you may wonder why I did not ask for a review yesterday while every day I ask for one, something was not smooth yesterday. Please ignore. ptal
I commented certain lines about the codes and example.dsc. If the patch set contains too many codes, could you please reply if having a patch set of adding initial support to FakePepperInterfaceGoogleDriveFs and having other patch sets of implementing the methods of FakeDriveURLLoaderInterface and FakeDriveURLResponseInfoInterface are a better idea? When you are not busy, ptal https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... File native_client_sdk/src/tests/nacl_io_test/example.dsc (right): https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/example.dsc:23: 'fake_ppapi/fake_pepper_interface_googledrivefs.h', My apology. That's weird that I was thinking fake_pepper_interface_googledrivefs.{cc,h} was fake_g... while I was including the files. https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... File native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc (right): https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc:88: PP_Resource request, The .cc function signature names the 2ndparameter as request while the .h function signature names the 2ndparameter as request_info. The parameters may be better renamed. .cc function signature: int32_t FakeDriveURLLoaderInterface::Open(PP_Resource loader, PP_Resource request, PP_CompletionCallback callback); .h function signature: virtual int32_t Open(PP_Resource loader, PP_Resource request_info, PP_CompletionCallback callback); https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... File native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_url_loader.cc (right): https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_url_loader.cc:16: #include "ppapi/c/pp_bool.h" Almost always the ppapi/c/*.* isincluded as "<..>", not "..". "<..>" is probably the include style and ought to be switched from ".." in the next patch set. https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... File native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_url_loader.h (right): https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_url_loader.h:145: FakeVarInterface* var_interface_; // Weak reference. FakeVarInterface can be private, not protected. Nonetheless, look safe to access var_interface_ from FakeDriveURLResponseInfoInterface and further derived classes.
https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... File native_client_sdk/src/tests/nacl_io_test/example.dsc (right): https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/example.dsc:23: 'fake_ppapi/fake_pepper_interface_googledrivefs.h', On 2016/12/02 13:23:33, chanpatorikku wrote: > My apology. That's weird that I was thinking > fake_pepper_interface_googledrivefs.{cc,h} was fake_g... while I was including > the files. Yes, please keep it alphabetical. https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... File native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc (right): https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc:42: std::string response_body; Why not store the response_body in the response? https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc:51: PP_Resource loader; Why doesn't the response own references to these resources? https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc:88: PP_Resource request, On 2016/12/02 13:23:33, chanpatorikku wrote: > The .cc function signature names the 2ndparameter as request while the .h > function signature names the 2ndparameter as request_info. The parameters may > be better renamed. > > .cc function signature: > > int32_t FakeDriveURLLoaderInterface::Open(PP_Resource loader, > PP_Resource request, > PP_CompletionCallback callback); > > .h function signature: > > virtual int32_t Open(PP_Resource loader, > PP_Resource request_info, > PP_CompletionCallback callback); > Feel free to fix, but it's common that the declaration parameter name doesn't match the definition parameter name. https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc:135: if (drive_loader_resource->response == 0) { nit: please be consistent with braces around single statements. Here and elsewhere in the CL. https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc:244: char path_buffer[1024]; nit: 1024 is way overkill here. Even if %u is 64-bit, the max value is 18446744073709551615, which is just 20 chars long. https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... File native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_url_loader.cc (right): https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_url_loader.cc:16: #include "ppapi/c/pp_bool.h" On 2016/12/02 13:23:33, chanpatorikku wrote: > Almost always the ppapi/c/*.* isincluded as "<..>", not "..". > > "<..>" is probably the include style and ought to be switched from ".." in the > next patch set. Yes, for consistency this should be moved after "gtest/gtest.h" and before "fake_ppapi/fake_util.h" and use <..> https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_url_loader.cc:478: // TODO: add tests for this. not necessary, the comments above are because the value isn't used. The value is used here. https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... File native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_url_loader.h (right): https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_url_loader.h:145: FakeVarInterface* var_interface_; // Weak reference. On 2016/12/02 13:23:33, chanpatorikku wrote: > FakeVarInterface can be private, not protected. Nonetheless, look safe to > access var_interface_ from FakeDriveURLResponseInfoInterface and further derived > classes. This is fine. https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... File native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_util.h (right): https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_util.h:15: const int32_t STATUSCODE_NOT_IMPLEMENTED = 501; May as well just add this to http_status_codes.h
ptal https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... File native_client_sdk/src/tests/nacl_io_test/example.dsc (right): https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/example.dsc:23: 'fake_ppapi/fake_pepper_interface_googledrivefs.h', On 2016/12/02 22:30:38, binji wrote: > On 2016/12/02 13:23:33, chanpatorikku wrote: > > My apology. That's weird that I was thinking > > fake_pepper_interface_googledrivefs.{cc,h} was fake_g... while I was including > > the files. > > Yes, please keep it alphabetical. Done. https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... File native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc (right): https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc:42: std::string response_body; On 2016/12/02 22:30:38, binji wrote: > Why not store the response_body in the response? If response_body is stored in the response, FinishStreamingToFile() does not have to be called. Open() can store response_body to the response, and the regular calls from Open(), FinishStreamingToFile(), GetResponseInfo(), to GetBodyAsFileRef() are not being simulated. FinishStreamingToFile() is used to wait for the response body to be completely downloaded, so storing response_body in the loader first and copying response_body to the response can simulate the download of the response body. https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc:51: PP_Resource loader; On 2016/12/02 22:30:38, binji wrote: > Why doesn't the response own references to these resources? Not a reason why. In the current patch set, the response own references to these resources, and a reference returned from GetBodyAsFileRef() is owned by the caller. If the response, which was not owning references, gives you a not very good experience on the review, sorry. Some of the test codes look to be written in a higher coding speed perhaps because the test code is for internal use. I get how almost all (if not all) related nacl_io codes work for googlefilesystemfs but not the test codes. I just read thoroughly on how reference counting in PP_Resource works. I can incrementally update on figuring how all related test codes work from now if the experience was not very good for you. https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc:88: PP_Resource request, On 2016/12/02 22:30:38, binji wrote: > On 2016/12/02 13:23:33, chanpatorikku wrote: > > The .cc function signature names the 2ndparameter as request while the .h > > function signature names the 2ndparameter as request_info. The parameters may > > be better renamed. > > > > .cc function signature: > > > > int32_t FakeDriveURLLoaderInterface::Open(PP_Resource loader, > > PP_Resource request, > > PP_CompletionCallback callback); > > > > .h function signature: > > > > virtual int32_t Open(PP_Resource loader, > > PP_Resource request_info, > > PP_CompletionCallback callback); > > > > Feel free to fix, but it's common that the declaration parameter name doesn't > match the definition parameter name. Not gonna fix because it's common and the code looks to be maintainable and understandable. https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc:135: if (drive_loader_resource->response == 0) { On 2016/12/02 22:30:38, binji wrote: > nit: please be consistent with braces around single statements. Here and > elsewhere in the CL. Done. About being consistent with braces around single statements on the changed codes of the previous CL's, please let me make the change in another CL. https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc:244: char path_buffer[1024]; On 2016/12/02 22:30:38, binji wrote: > nit: 1024 is way overkill here. Even if %u is 64-bit, the max value is > 18446744073709551615, which is just 20 chars long. Changed to path_buffer[32]. I was thinking the stack pointer is going to just move 1024 and is the same as any buffer size between 21 to 1024. https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... File native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_url_loader.cc (right): https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_url_loader.cc:16: #include "ppapi/c/pp_bool.h" On 2016/12/02 22:30:39, binji wrote: > On 2016/12/02 13:23:33, chanpatorikku wrote: > > Almost always the ppapi/c/*.* isincluded as "<..>", not "..". > > > > "<..>" is probably the include style and ought to be switched from ".." in the > > next patch set. > > Yes, for consistency this should be moved after "gtest/gtest.h" and before > "fake_ppapi/fake_util.h" and use <..> Done. https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_url_loader.cc:478: // TODO: add tests for this. On 2016/12/02 22:30:38, binji wrote: > not necessary, the comments above are because the value isn't used. The value is > used here. Done. https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... File native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_url_loader.h (right): https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_url_loader.h:145: FakeVarInterface* var_interface_; // Weak reference. On 2016/12/02 22:30:39, binji wrote: > On 2016/12/02 13:23:33, chanpatorikku wrote: > > FakeVarInterface can be private, not protected. Nonetheless, look safe to > > access var_interface_ from FakeDriveURLResponseInfoInterface and further > derived > > classes. > > This is fine. Acknowledged. https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... File native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_util.h (right): https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_util.h:15: const int32_t STATUSCODE_NOT_IMPLEMENTED = 501; The future code in libraries/nacl_io is not going to use STATUSCODE_NOT_IMPLEMENTED, so leaving STATUSCODE_NOT_IMPLEMENTED in fake_util.h. The codes in libraries/nacl_io seem to be better to have fewer test codes.
When you're not busy, ptal
When you're not busy, ptal
I did not mean to write double messages. Please ignore.
https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... File native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc (right): https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc:42: std::string response_body; On 2016/12/05 15:05:44, chanpatorikku wrote: > On 2016/12/02 22:30:38, binji wrote: > > Why not store the response_body in the response? > > If response_body is stored in the response, FinishStreamingToFile() does not > have to be called. > > Open() can store response_body to the response, and the regular calls from > Open(), FinishStreamingToFile(), GetResponseInfo(), to GetBodyAsFileRef() are > not being simulated. > > FinishStreamingToFile() is used to wait for the response body to be completely > downloaded, so storing response_body in the loader first and copying > response_body to the response can simulate the download of the response body. It doesn't seem as though response_body is actually used. GetBodyAsFileRef uses the response's body directly. The reason I mention this, is that I'd prefer if the response didn't have references to the loader and request, it doesn't seem like it is necessary, and it over-complicates the code. https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc:244: char path_buffer[1024]; On 2016/12/05 15:05:44, chanpatorikku wrote: > On 2016/12/02 22:30:38, binji wrote: > > nit: 1024 is way overkill here. Even if %u is 64-bit, the max value is > > 18446744073709551615, which is just 20 chars long. > > Changed to path_buffer[32]. I was thinking the stack pointer is going to just > move 1024 and is the same as any buffer size between 21 to 1024. Yes, but the stack is not limitless, so it is best to keep it small if you can.
Only a change from "PP_Resource request" to "bool stream_to_file_set". ptal https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... File native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc (right): https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc:42: std::string response_body; On 2016/12/08 20:35:13, binji wrote: > On 2016/12/05 15:05:44, chanpatorikku wrote: > > On 2016/12/02 22:30:38, binji wrote: > > > Why not store the response_body in the response? > > > > If response_body is stored in the response, FinishStreamingToFile() does not > > have to be called. > > > > Open() can store response_body to the response, and the regular calls from > > Open(), FinishStreamingToFile(), GetResponseInfo(), to GetBodyAsFileRef() are > > not being simulated. > > > > FinishStreamingToFile() is used to wait for the response body to be completely > > downloaded, so storing response_body in the loader first and copying > > response_body to the response can simulate the download of the response body. > > It doesn't seem as though response_body is actually used. GetBodyAsFileRef uses > the response's body directly. > > The reason I mention this, is that I'd prefer if the response didn't have > references to the loader and request, it doesn't seem like it is necessary, and > it over-complicates the code. response_body isn't actually used in the current patch, but is going to be used in the future code. The future code, which changes Respond() to be serving different requests so response_body is set from FinishStreamingToFile(), GetBodyAsFileRef(), Open(), and Respond() (in alphabetical order not in function call time order), includes more code of implementing FakeGoogleDriveServer and the GoogleDriveFs nacl_io library code. The patch set is going to contain a few times more lines of code if it contains code of using response_body. response has to have a reference to loader to implement GetBodyAsFileRef(), although it does not have to have a reference to request The description of GetBodyAsFileRef() reveals that: This file is only valid if PP_URLREQUESTPROPERTY_STREAMTOFILE was set on the URLRequestInfo used to produce this response. This file remains valid until the URLLoader associated with this URLResponseInfo is closed or destroyed. (https://developer.chrome.com/native-client/pepper_stable/c/struct_p_p_b___u_r...) response has to have a reference to loader to see if 'server' is set as NULL in loader, if the URLLoader has been closed or destroyed already, and if the file is still valid response does not have to have a reference to request and is able to have another variable 'stream_to_file_set' in response, which is a copy of 'stream_to_file' in request. I am going to remove the reference of request and add 'stream_to_file_set' in patch set 3. Shouldn't be a complex change. https://codereview.chromium.org/2538163003/diff/40001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc:244: char path_buffer[1024]; On 2016/12/08 20:35:12, binji wrote: > On 2016/12/05 15:05:44, chanpatorikku wrote: > > On 2016/12/02 22:30:38, binji wrote: > > > nit: 1024 is way overkill here. Even if %u is 64-bit, the max value is > > > 18446744073709551615, which is just 20 chars long. > > > > Changed to path_buffer[32]. I was thinking the stack pointer is going to just > > move 1024 and is the same as any buffer size between 21 to 1024. > > Yes, but the stack is not limitless, so it is best to keep it small if you can. The stack is about a few megabytes. It is small compared to the heap.
lgtm % nit https://codereview.chromium.org/2538163003/diff/80001/native_client_sdk/src/t... File native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc (right): https://codereview.chromium.org/2538163003/diff/80001/native_client_sdk/src/t... native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc:63: bool stream_to_file_set; maybe just stream_to_file. stream_to_file_set makes it sound like it is a std::set or something.
The CQ bit was checked by chanpatorikku@gmail.com
The patchset sent to the CQ was uploaded after l-g-t-m from binji@chromium.org Link to the patchset: https://codereview.chromium.org/2538163003/#ps100001 (title: "")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 100001, "attempt_start_ts": 1481601621002980, "parent_rev": "fbcf5f4c01198dbb4de7c1ebe9dd8a264fbce21b", "commit_rev": "3f7bbddfc09754c092c56a32e259f5ceb4bf4e38"}
Message was sent while issue was closed.
Description was changed from ========== [NaCl SDK] Implement FakePepperInterfaceGoogleDriveFs Implement FakePepperInterfaceGoogleDriveFs, and implement FakeDriveURLResponseInfoInterface and FakeDriveURLLoaderInterface, which FakePepperInterfaceGoogleDriveFs uses. FakePepperInterfaceGoogleDriveFs is going to be used by the unit tests of the future code. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_nacl_sdk;master.tryserver.chromium.mac:mac_nacl_sdk;master.tryserver.chromium.win:win_nacl_sdk ========== to ========== [NaCl SDK] Implement FakePepperInterfaceGoogleDriveFs Implement FakePepperInterfaceGoogleDriveFs, and implement FakeDriveURLResponseInfoInterface and FakeDriveURLLoaderInterface, which FakePepperInterfaceGoogleDriveFs uses. FakePepperInterfaceGoogleDriveFs is going to be used by the unit tests of the future code. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_nacl_sdk;master.tryserver.chromium.mac:mac_nacl_sdk;master.tryserver.chromium.win:win_nacl_sdk Review-Url: https://codereview.chromium.org/2538163003 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:100001)
Message was sent while issue was closed.
Description was changed from ========== [NaCl SDK] Implement FakePepperInterfaceGoogleDriveFs Implement FakePepperInterfaceGoogleDriveFs, and implement FakeDriveURLResponseInfoInterface and FakeDriveURLLoaderInterface, which FakePepperInterfaceGoogleDriveFs uses. FakePepperInterfaceGoogleDriveFs is going to be used by the unit tests of the future code. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_nacl_sdk;master.tryserver.chromium.mac:mac_nacl_sdk;master.tryserver.chromium.win:win_nacl_sdk Review-Url: https://codereview.chromium.org/2538163003 ========== to ========== [NaCl SDK] Implement FakePepperInterfaceGoogleDriveFs Implement FakePepperInterfaceGoogleDriveFs, and implement FakeDriveURLResponseInfoInterface and FakeDriveURLLoaderInterface, which FakePepperInterfaceGoogleDriveFs uses. FakePepperInterfaceGoogleDriveFs is going to be used by the unit tests of the future code. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_nacl_sdk;master.tryserver.chromium.mac:mac_nacl_sdk;master.tryserver.chromium.win:win_nacl_sdk Committed: https://crrev.com/7d7031e3d92025246c5f296a36727c77f5bdcbd0 Cr-Commit-Position: refs/heads/master@{#438081} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/7d7031e3d92025246c5f296a36727c77f5bdcbd0 Cr-Commit-Position: refs/heads/master@{#438081} |