Add new API function: feedbackPrivate.readLogSource

chrome/browser/extensions/api/feedback_private/feedback_private_api_chromeos_unittest.cc

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/extensions/api/feedback_private/feedback_private_api.h"
+
+#include "base/json/json_writer.h"
+#include "base/macros.h"
+#include "base/test/simple_test_tick_clock.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "base/values.h"
+#include "chrome/browser/extensions/api/feedback_private/log_source_resource.h"
+#include "chrome/browser/extensions/api/feedback_private/single_log_source_factory.h"
+#include "chrome/browser/extensions/extension_api_unittest.h"
+#include "extensions/browser/api_test_utils.h"
+#include "extensions/browser/api_unittest.h"
+
+namespace extensions {
+
+namespace {
+
+using api::feedback_private::ReadLogSourceResult;
+using api::feedback_private::ReadLogSourceParams;
+using system_logs::SingleLogSource;
+using system_logs::SystemLogsResponse;
+using SupportedSource = system_logs::SingleLogSource::SupportedSource;
+
+std::unique_ptr<KeyedService> ApiResourceManagerTestFactory(
+    content::BrowserContext* context) {
+  return base::MakeUnique<ApiResourceManager<LogSourceResource>>(context);
+}
+
+// A dummy SingleLogSource that does not require real system logs to be
+// available during testing.
+class TestSingleLogSource : public SingleLogSource {
+ public:
+  explicit TestSingleLogSource(SupportedSource type)
+      : SingleLogSource(type), call_count_(0) {}
+
+  ~TestSingleLogSource() override {}

[tbarzic, 2017/06/06 20:27:25]

= default

[Simon Que, 2017/06/06 22:29:14]

Done.

+
+  // Fetch() will return a single different string each time, in the following
+  // sequence: "a", "bb", "ccc", until a string of 26 z's. Will never return an
+  // empty result.
+  void Fetch(const system_logs::SysLogsSourceCallback& callback) override {
+    int count_modulus = call_count_ % kNumCharsToIterate;
+    std::string result(count_modulus + 1, kInitialChar + count_modulus);
+    CHECK_GT(result.size(), 0U);

[tbarzic, 2017/06/06 20:27:25]

ASSERT_GT

[Simon Que, 2017/06/06 22:29:14]

Done.

+    ++call_count_;
+
+    SystemLogsResponse* result_map = new SystemLogsResponse;
+    result_map->emplace("", result);
+
+    // Do not directly pass the result to the callback, because that's not how
+    // log sources actually work. Instead, simulate the asynchronous operation
+    // of a SingleLogSource by invoking the callback separately.
+    base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(

[tbarzic, 2017/06/06 20:27:25]

You can use PostTask instead PostDelayedTask(..., FromMilliseconds(0));

[Simon Que, 2017/06/06 22:29:14]

Done.

+        FROM_HERE, base::Bind(callback, base::Owned(result_map)),
+        base::TimeDelta::FromMilliseconds(0));
+  }
+
+  // Instantiates a new instance of this class. Does not retain ownership. Used
+  // to create a Callback that can be used to override the default behavior of
+  // SingleLogSourceFactory.
+  static SingleLogSource* Create(SupportedSource type) {
+    return new TestSingleLogSource(type);
+  }
+
+ private:
+  // Iterate over the whole lowercase alphabet, starting from 'a'.
+  const int kNumCharsToIterate = 26;
+  const char kInitialChar = 'a';
+
+  // Keep track of how many times Fetch() has been called, in order to determine
+  // its behavior each time.
+  int call_count_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestSingleLogSource);
+};
+
+}  // namespace
+
+class FeedbackApiUnittest : public ExtensionApiUnittest {
+ public:
+  FeedbackApiUnittest()
+      : create_callback_(base::Bind(&TestSingleLogSource::Create)) {}
+  ~FeedbackApiUnittest() override {}
+
+  void SetUp() override {
+    ExtensionApiUnittest::SetUp();
+
+    // The ApiResourceManager used for LogSourceResource is destroyed every time
+    // a unit test finishes, during TearDown(). There is no way to re-create it
+    // normally. The below code forces it to be re-created during SetUp(), so
+    // that there is always a valid ApiResourceManager<LogSourceResource> when
+    // subsequent unit tests are running.
+    ApiResourceManager<LogSourceResource>::GetFactoryInstance()
+        ->SetTestingFactoryAndUse(browser()->profile(),

[tbarzic, 2017/06/07 03:39:02]

use profile() instead of browser()->profile()

[Simon Que, 2017/06/07 18:25:58]

Done.

+                                  ApiResourceManagerTestFactory);
+
+    SingleLogSourceFactory::SetForTesting(&create_callback_);
+  }
+
+  void TearDown() override {
+    SingleLogSourceFactory::SetForTesting(nullptr);
+    LogSourceAccessManager::SetRateLimitingTimeoutForTesting(nullptr);
+
+    FeedbackPrivateAPI::GetFactoryInstance()
+        ->Get(browser()->profile())
+        ->log_source_access_manager()
+        ->set_tick_clock(nullptr);
+
+    ExtensionApiUnittest::TearDown();
+  }
+
+  // Runs the feedbackPrivate.readLogSource() function. See API function
+  // definition for argument descriptions.
+  //
+  // Note that the second argument of the result is a list of strings, but the
+  // test class TestSingleLogSource always returns a list containing a single
+  // string. To simplify things, the single string result will be returned in
+  // |*result_string|, while the reader ID is returned in |*result_reader_id|.
+  testing::AssertionResult RunReadLogSourceFunction(
+      const ReadLogSourceParams& params,
+      int* result_reader_id,
+      std::string* result_string) {
+    scoped_refptr<FeedbackPrivateReadLogSourceFunction> function =
+        new FeedbackPrivateReadLogSourceFunction;
+
+    std::unique_ptr<base::ListValue> result_list =
+        RunFunctionAndReturnValue(function.get(), ParamsToJSON(params));
+    if (!result_list)
+      return testing::AssertionFailure() << "No result";
+
+    ReadLogSourceResult result;
+    testing::AssertionResult parse_result =
+        ParseReadLogSourceResult(*result_list, &result);
+    if (!parse_result)
+      return parse_result;
+
+    if (result.log_lines.size() != 1) {
+      return testing::AssertionFailure()
+             << "Expected |log_lines| to contain 1 string, actual number: "
+             << result.log_lines.size();
+    }
+
+    *result_reader_id = result.reader_id;
+    *result_string = result.log_lines[0];
+
+    return testing::AssertionSuccess();
+  }
+
+ private:
+  // Converts |params| to a string containing a JSON dictionary within an
+  // argument list.
+  std::string ParamsToJSON(const ReadLogSourceParams& params) {
+    base::ListValue params_value;
+    params_value.Append(params.ToValue());
+    std::string params_json_string;
+    EXPECT_TRUE(base::JSONWriter::Write(params_value, &params_json_string));
+
+    return params_json_string;
+  }
+
+  // Runs |function| with arguments |args| in JSON string format and returns the
+  // result as a base::ListValue.
+  std::unique_ptr<base::ListValue> RunFunctionAndReturnValue(

[tbarzic, 2017/06/06 20:27:25]

I think you can use RunFunctionAndReturnValue instead of this now.

[Simon Que, 2017/06/06 22:29:14]

That function still ends up with an EXPECT that the result is valid.

https://cs.chromium.org/chromium/src/extensions/browser/api_test_utils.cc?l=138

[Simon Que, 2017/06/07 01:43:24]

When ApiUnitTest and api_test_utils get refactored and possibly combined, there
should be a flag to disable the EXPECTs. If that feature were added

For now, I can add a TODO(tbarzic?) to do as you suggested, once that has been
done.

[Simon Que, 2017/06/07 01:57:51]

*If that feature were added, we can remove the local implementation of
RunFunctionAndReturnValue().

[tbarzic, 2017/06/07 03:39:02]

That feature kinda already exists - instead of using
RunFunctionAnrReturnDictionary, you'd have to use RunFunctionAndReturnError 
when the function is expected to fail (which would also enable you to test which
error was encounterred, not only whether there was an error).

[Simon Que, 2017/06/07 18:25:58]

Done.

+      UIThreadExtensionFunction* function,
+      const std::string& args) {
+    function->set_extension(extension());
+    if (!api_test_utils::RunFunction(function, args, browser()->profile()))
+      return nullptr;
+
+    const base::ListValue* result_list = function->GetResultList();
+    if (!result_list)
+      return nullptr;
+
+    return result_list->CreateDeepCopy();
+  }
+
+  // Attempts to interpret the result as a ReadLogSourceResult. Returns failure
+  // if |value| does not conform to the format of a list containing a single
+  // element that is a ReadLogSourceResult object.
+  testing::AssertionResult ParseReadLogSourceResult(
+      const base::ListValue& list_value,
+      ReadLogSourceResult* result) {
+    if (list_value.empty())
+      return testing::AssertionFailure() << "Result is empty.";
+
+    const base::Value* result_value = nullptr;
+    if (!list_value.Get(0, &result_value)) {
+      return testing::AssertionFailure()
+             << "Unable to get first element of result list.";
+    }
+
+    if (!ReadLogSourceResult::Populate(*result_value, result)) {
+      std::string json_string;
+      if (!base::JSONWriter::Write(*result_value, &json_string)) {
+        return testing::AssertionFailure()
+               << "Unable to parse result object as JSON.";
+      }
+
+      return testing::AssertionFailure()
+             << "Result not in valid ReadLogSourceResult format: "

[tbarzic, 2017/06/06 20:27:25]

I think you can use base::Value directly for logging.

if (!ReafLogSourceResult::Populate()) {
  return AssertionFailure() << "Invalid ReadLogSourceResult " << *result_value;
}

[Simon Que, 2017/06/06 22:29:14]

Done.

+             << json_string;
+    }
+
+    return testing::AssertionSuccess();
+  }
+
+  // Passed to SingleLogSourceFactory so that the API can create an instance of
+  // TestSingleLogSource for testing.
+  const SingleLogSourceFactory::CreateCallback create_callback_;
+
+  // content::TestBrowserThreadBundle browser_thread_bundle_;
+
+  DISALLOW_COPY_AND_ASSIGN(FeedbackApiUnittest);
+};
+
+TEST_F(FeedbackApiUnittest, ReadLogSourceInvalidId) {
+  const base::TimeDelta timeout(base::TimeDelta::FromMilliseconds(0));
+  LogSourceAccessManager::SetRateLimitingTimeoutForTesting(&timeout);
+
+  ReadLogSourceParams params;
+  params.source = api::feedback_private::LOG_SOURCE_MESSAGES;
+  params.incremental = true;
+  params.reader_id.reset(new int(9999));
+
+  int result_reader_id;
+  std::string result_string;
+  EXPECT_FALSE(
+      RunReadLogSourceFunction(params, &result_reader_id, &result_string));
+}
+
+TEST_F(FeedbackApiUnittest, ReadLogSourceNonIncremental) {
+  const base::TimeDelta timeout(base::TimeDelta::FromMilliseconds(0));
+  LogSourceAccessManager::SetRateLimitingTimeoutForTesting(&timeout);
+
+  ReadLogSourceParams params;
+  params.source = api::feedback_private::LOG_SOURCE_MESSAGES;
+  params.incremental = false;
+
+  // Test multiple non-incremental reads.
+  int result_reader_id = -1;
+  std::string result_string;
+  EXPECT_TRUE(
+      RunReadLogSourceFunction(params, &result_reader_id, &result_string));
+  EXPECT_EQ(0, result_reader_id);
+  EXPECT_EQ("a", result_string);
+
+  result_reader_id = -1;
+  result_string.clear();
+  EXPECT_TRUE(
+      RunReadLogSourceFunction(params, &result_reader_id, &result_string));
+  EXPECT_EQ(0, result_reader_id);
+  EXPECT_EQ("a", result_string);
+
+  result_reader_id = -1;
+  result_string.clear();
+  EXPECT_TRUE(
+      RunReadLogSourceFunction(params, &result_reader_id, &result_string));
+  EXPECT_EQ(0, result_reader_id);
+  EXPECT_EQ("a", result_string);
+}
+
+TEST_F(FeedbackApiUnittest, ReadLogSourceIncremental) {
+  const base::TimeDelta timeout(base::TimeDelta::FromMilliseconds(0));
+  LogSourceAccessManager::SetRateLimitingTimeoutForTesting(&timeout);
+
+  ReadLogSourceParams params;
+  params.source = api::feedback_private::LOG_SOURCE_MESSAGES;
+  params.incremental = true;
+
+  int result_reader_id = 0;
+  std::string result_string;
+  EXPECT_TRUE(
+      RunReadLogSourceFunction(params, &result_reader_id, &result_string));
+  EXPECT_EQ(1, result_reader_id);
+  EXPECT_EQ("a", result_string);
+  params.reader_id.reset(new int(result_reader_id));
+
+  EXPECT_TRUE(
+      RunReadLogSourceFunction(params, &result_reader_id, &result_string));
+  EXPECT_EQ(1, result_reader_id);
+  EXPECT_EQ("bb", result_string);
+
+  EXPECT_TRUE(
+      RunReadLogSourceFunction(params, &result_reader_id, &result_string));
+  EXPECT_EQ(1, result_reader_id);
+  EXPECT_EQ("ccc", result_string);
+
+  // End the incremental read.
+  params.incremental = false;
+  EXPECT_TRUE(
+      RunReadLogSourceFunction(params, &result_reader_id, &result_string));
+  EXPECT_EQ(0, result_reader_id);
+  EXPECT_EQ("dddd", result_string);
+
+  // The log source will no longer be valid if we try to read it.
+  params.incremental = true;
+  EXPECT_FALSE(
+      RunReadLogSourceFunction(params, &result_reader_id, &result_string));
+}
+
+TEST_F(FeedbackApiUnittest, ReadLogSourceMultipleSources) {
+  const base::TimeDelta timeout(base::TimeDelta::FromMilliseconds(0));
+  LogSourceAccessManager::SetRateLimitingTimeoutForTesting(&timeout);
+
+  int result_reader_id = 0;
+  std::string result_string;
+
+  // Attempt to open LOG_SOURCE_MESSAGES twice.
+  ReadLogSourceParams params1a;
+  params1a.source = api::feedback_private::LOG_SOURCE_MESSAGES;
+  params1a.incremental = true;
+  EXPECT_TRUE(
+      RunReadLogSourceFunction(params1a, &result_reader_id, &result_string));
+  EXPECT_EQ(1, result_reader_id);
+  params1a.reader_id.reset(new int(result_reader_id));
+
+  // Cannot perform a second read from the same log source.
+  ReadLogSourceParams params1b;
+  params1b.source = api::feedback_private::LOG_SOURCE_MESSAGES;
+  params1b.incremental = true;
+  EXPECT_FALSE(
+      RunReadLogSourceFunction(params1b, &result_reader_id, &result_string));
+
+  // Attempt to open LOG_SOURCE_UI_LATEST twice.
+  ReadLogSourceParams params2a;
+  params2a.source = api::feedback_private::LOG_SOURCE_UILATEST;
+  params2a.incremental = true;
+  result_reader_id = -1;
+  EXPECT_TRUE(
+      RunReadLogSourceFunction(params2a, &result_reader_id, &result_string));
+  params2a.reader_id.reset(new int(result_reader_id));
+  EXPECT_EQ(2, result_reader_id);
+
+  // Cannot perform a second read from the same log source.
+  ReadLogSourceParams params2b;
+  params2b.source = api::feedback_private::LOG_SOURCE_UILATEST;
+  params2b.incremental = true;
+  EXPECT_FALSE(
+      RunReadLogSourceFunction(params2b, &result_reader_id, &result_string));
+
+  // Close the two open log source readers, and make sure new ones can be
+  // opened.
+  params1a.incremental = false;
+  result_reader_id = -1;
+  EXPECT_TRUE(
+      RunReadLogSourceFunction(params1a, &result_reader_id, &result_string));
+  EXPECT_EQ(0, result_reader_id);
+
+  params2a.incremental = false;
+  result_reader_id = -1;
+  EXPECT_TRUE(
+      RunReadLogSourceFunction(params2a, &result_reader_id, &result_string));
+  EXPECT_EQ(0, result_reader_id);
+
+  EXPECT_TRUE(
+      RunReadLogSourceFunction(params1b, &result_reader_id, &result_string));
+  EXPECT_EQ(3, result_reader_id);
+  EXPECT_TRUE(
+      RunReadLogSourceFunction(params2b, &result_reader_id, &result_string));
+  EXPECT_EQ(4, result_reader_id);
+}
+
+TEST_F(FeedbackApiUnittest, ReadLogSourceWithAccessTimeouts) {
+  const base::TimeDelta timeout(base::TimeDelta::FromMilliseconds(100));
+  LogSourceAccessManager::SetRateLimitingTimeoutForTesting(&timeout);
+
+  base::SimpleTestTickClock* test_clock = new base::SimpleTestTickClock;
+  FeedbackPrivateAPI::GetFactoryInstance()
+      ->Get(browser()->profile())
+      ->log_source_access_manager()
+      ->set_tick_clock(std::unique_ptr<base::TickClock>(test_clock));
+
+  ReadLogSourceParams params;
+  params.source = api::feedback_private::LOG_SOURCE_MESSAGES;
+  params.incremental = true;
+  int result_reader_id = 0;
+  std::string result_string;
+
+  auto FromMilliseconds = &base::TimeDelta::FromMilliseconds;
+  // |test_clock| must start out at something other than 0, which is interpreted
+  // as an invalid value.
+  test_clock->Advance(FromMilliseconds(100));
+
+  EXPECT_TRUE(
+      RunReadLogSourceFunction(params, &result_reader_id, &result_string));
+  EXPECT_EQ(1, result_reader_id);
+  params.reader_id.reset(new int(result_reader_id));
+
+  // Immediately perform another read. This is not allowed.
+  EXPECT_FALSE(
+      RunReadLogSourceFunction(params, &result_reader_id, &result_string));
+
+  // Advance to t=120, but it will not be allowed.
+  test_clock->Advance(FromMilliseconds(20));
+  EXPECT_FALSE(
+      RunReadLogSourceFunction(params, &result_reader_id, &result_string));
+
+  // Advance to t=150, but still not allowed.
+  test_clock->Advance(FromMilliseconds(30));
+  EXPECT_FALSE(
+      RunReadLogSourceFunction(params, &result_reader_id, &result_string));
+
+  // Advance to t=199, but still not allowed.
+  test_clock->Advance(FromMilliseconds(49));
+  EXPECT_FALSE(
+      RunReadLogSourceFunction(params, &result_reader_id, &result_string));
+
+  // Advance to t=210, annd the access is finally allowed.
+  test_clock->Advance(FromMilliseconds(11));
+  EXPECT_TRUE(
+      RunReadLogSourceFunction(params, &result_reader_id, &result_string));
+
+  // Advance to t=309, but it will not be allowed.
+  test_clock->Advance(FromMilliseconds(99));
+  EXPECT_FALSE(
+      RunReadLogSourceFunction(params, &result_reader_id, &result_string));
+
+  // Another read is finally allowed at t=310.
+  test_clock->Advance(FromMilliseconds(1));
+  EXPECT_TRUE(
+      RunReadLogSourceFunction(params, &result_reader_id, &result_string));
+}
+
+}  // namespace extensions