Apply review's findings

youtube · Dec 13, 2024 · f7d6de8 · f7d6de8
1 parent c4665ca
commit f7d6de8
Show file tree

Hide file tree

Showing 6 changed files with 116 additions and 51 deletions.
diff --git a/starboard/nplb/BUILD.gn b/starboard/nplb/BUILD.gn
@@ -309,6 +309,14 @@ target(gtest_target_type, "nplb") {
         [ "//starboard/nplb/compiler_compliance/cpp_compiler_version_check.cc" ]
   }
 
+  if (is_win) {
+    sources +=
+        [ "//starboard/nplb/time_zone_with_expect_value_win.cc" ]
+  } else {
+    sources +=
+        [ "//starboard/nplb/time_zone_with_expect_value.cc" ]
+  }
+
   configs += [ "//starboard/build/config:starboard_implementation" ]
   if (sb_enable_cast_codec_tests) {
     configs += [ ":cast_codec_tests" ]

diff --git a/starboard/nplb/time_zone_get_current_test.cc b/starboard/nplb/time_zone_get_current_test.cc
@@ -12,6 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include "time_zone_with_expect_value.h"
+
 #include "starboard/extension/time_zone.h"
 #include "starboard/nplb/time_constants.h"
 #include "starboard/system.h"
@@ -25,20 +27,6 @@ namespace starboard {
 namespace nplb {
 namespace {
 
-struct TimeZoneWithExpectValue {
-  TimeZoneWithExpectValue(std::string timeZoneName_,
-                          SbTimeZone expectedStandardValue_,
-                          SbTimeZone expectedDaylightValue_)
-      : timeZoneName{timeZoneName_},
-        expectedStandardValue{expectedStandardValue_},
-        expectedDaylightValue{expectedDaylightValue_} {}
-
-  std::string timeZoneName;
-
-  SbTimeZone expectedStandardValue;
-  SbTimeZone expectedDaylightValue;
-};
-
 class SbTimeZoneGetCurrentSetTimeZoneTest
     : public testing::Test,
       public testing::WithParamInterface<TimeZoneWithExpectValue> {
@@ -80,39 +68,6 @@ TEST(SbTimeZoneGetCurrentTest, IsKindOfSane) {
   EXPECT_LE(zone, 24 * 60);
 }
 
-#if defined(_WIN32)
-
-std::array<TimeZoneWithExpectValue, 12> timeZonesWithExpectedTimeValues{
-    TimeZoneWithExpectValue("UTC", 0, 0),
-    TimeZoneWithExpectValue("Atlantic Standard Time", 240, 180),
-    TimeZoneWithExpectValue("Eastern Standard Time", 300, 240),
-    TimeZoneWithExpectValue("Central Standard Time", 360, 300),
-    TimeZoneWithExpectValue("Mountain Standard Time", 420, 360),
-    TimeZoneWithExpectValue("Pacific Standard Time", 480, 420),
-    TimeZoneWithExpectValue("Yukon Standard Time", 420, 420),
-    TimeZoneWithExpectValue("Samoa Standard Time", -780, -780),
-    TimeZoneWithExpectValue("China Standard Time", -480, -480),
-    TimeZoneWithExpectValue("Central European Standard Time", -60, -120),
-    TimeZoneWithExpectValue("Omsk Standard Time", -360, -360),
-    TimeZoneWithExpectValue("Cen. Australia Standard Time", -570, -630)};
-
-#else
-
-std::array<TimeZoneWithExpectValue, 11> timeZonesWithExpectedTimeValues{
-    TimeZoneWithExpectValue("UTC", 0, 0),
-    TimeZoneWithExpectValue("America/Puerto_Rico", 240, 240),
-    TimeZoneWithExpectValue("America/New_York", 300, 300),
-    TimeZoneWithExpectValue("US/Eastern", 300, 300),
-    TimeZoneWithExpectValue("America/Chicago", 360, 360),
-    TimeZoneWithExpectValue("US/Mountain", 420, 420),
-    TimeZoneWithExpectValue("US/Pacific", 480, 480),
-    TimeZoneWithExpectValue("US/Alaska", 540, 540),
-    TimeZoneWithExpectValue("Pacific/Honolulu", 600, 600),
-    TimeZoneWithExpectValue("US/Samoa", 660, 660),
-    TimeZoneWithExpectValue("Pacific/Guam", -600, -600)};
-
-#endif
-
 TEST_P(SbTimeZoneGetCurrentSetTimeZoneTest, IsKindOfSane) {
   EXPECT_TRUE(time_zone_extension->SetTimeZone(GetParam().timeZoneName.c_str()));
   auto zone = SbTimeZoneGetCurrent();

diff --git a/starboard/nplb/time_zone_with_expect_value.cc b/starboard/nplb/time_zone_with_expect_value.cc
@@ -0,0 +1,19 @@
+#include "time_zone_with_expect_value.h"
+
+#if !defined(_WIN32)
+
+const std::array<TimeZoneWithExpectValue, 12> timeZonesWithExpectedTimeValues{
+    TimeZoneWithExpectValue("UTC", 0, 0),
+    TimeZoneWithExpectValue("America/Puerto_Rico", 240, 240),
+    TimeZoneWithExpectValue("America/New_York", 300, 300),
+    TimeZoneWithExpectValue("US/Eastern", 300, 300),
+    TimeZoneWithExpectValue("America/Chicago", 360, 360),
+    TimeZoneWithExpectValue("US/Mountain", 420, 420),
+    TimeZoneWithExpectValue("US/Pacific", 480, 480),
+    TimeZoneWithExpectValue("US/Alaska", 540, 540),
+    TimeZoneWithExpectValue("Pacific/Honolulu", 600, 600),
+    TimeZoneWithExpectValue("US/Samoa", 660, 660),
+    TimeZoneWithExpectValue("Australia/South", -570, -570),
+    TimeZoneWithExpectValue("Pacific/Guam", -600, -600)};
+
+#endif
diff --git a/starboard/nplb/time_zone_with_expect_value.h b/starboard/nplb/time_zone_with_expect_value.h
@@ -0,0 +1,20 @@
+#include "starboard/time_zone.h"
+
+#include <array>
+#include <gmock/gmock.h>
+
+struct TimeZoneWithExpectValue {
+  TimeZoneWithExpectValue(std::string timeZoneName_,
+                          SbTimeZone expectedStandardValue_,
+                          SbTimeZone expectedDaylightValue_)
+      : timeZoneName{timeZoneName_},
+        expectedStandardValue{expectedStandardValue_},
+        expectedDaylightValue{expectedDaylightValue_} {}
+
+  std::string timeZoneName;
+
+  SbTimeZone expectedStandardValue;
+  SbTimeZone expectedDaylightValue;
+};
+
+extern const std::array<TimeZoneWithExpectValue, 12> timeZonesWithExpectedTimeValues;
diff --git a/starboard/nplb/time_zone_with_expect_value_win.cc b/starboard/nplb/time_zone_with_expect_value_win.cc
@@ -0,0 +1,19 @@
+#include "time_zone_with_expect_value.h"
+
+#if defined(_WIN32)
+
+const std::array<TimeZoneWithExpectValue, 12> timeZonesWithExpectedTimeValues{
+    TimeZoneWithExpectValue("UTC", 0, 0),
+    TimeZoneWithExpectValue("Atlantic Standard Time", 240, 180),
+    TimeZoneWithExpectValue("Eastern Standard Time", 300, 240),
+    TimeZoneWithExpectValue("Central Standard Time", 360, 300),
+    TimeZoneWithExpectValue("Mountain Standard Time", 420, 360),
+    TimeZoneWithExpectValue("Pacific Standard Time", 480, 420),
+    TimeZoneWithExpectValue("Yukon Standard Time", 420, 420),
+    TimeZoneWithExpectValue("Samoa Standard Time", -840, -840),
+    TimeZoneWithExpectValue("China Standard Time", -480, -480),
+    TimeZoneWithExpectValue("Central European Standard Time", -60, -120),
+    TimeZoneWithExpectValue("Omsk Standard Time", -360, -360),
+    TimeZoneWithExpectValue("Cen. Australia Standard Time", -570, -630)};
+
+#endif
diff --git a/starboard/shared/win32/time_zone.cc b/starboard/shared/win32/time_zone.cc
@@ -12,23 +12,61 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include "starboard/common/log.h"
 #include "starboard/extension/time_zone.h"
 #include "starboard/time_zone.h"
-#include "starboard/common/log.h"
 
-#include <string>
 #include <Windows.h>
+#include <string>
 
 namespace starboard {
 namespace shared {
 namespace win32 {
 
 namespace {
 
+bool SetPrivilege(HANDLE hToken, LPCTSTR lpszPrivilege, bool bEnablePrivilege) {
+  TOKEN_PRIVILEGES tp;
+  LUID luid;
+
+  if (!LookupPrivilegeValue(NULL, lpszPrivilege, &luid)) {
+    SB_LOG(ERROR) << "LookupPrivilegeValue error: " << GetLastError();
+    return false;
+  }
+
+  tp.PrivilegeCount = 1;
+  tp.Privileges[0].Luid = luid;
+  tp.Privileges[0].Attributes = bEnablePrivilege ? SE_PRIVILEGE_ENABLED : 0;
+
+  if (!AdjustTokenPrivileges(hToken, FALSE, &tp, sizeof(TOKEN_PRIVILEGES),
+                             (PTOKEN_PRIVILEGES)NULL, (PDWORD)NULL)) {
+    SB_LOG(ERROR) << "AdjustTokenPrivileges error: " << GetLastError();
+    return false;
+  }
+
+  if (GetLastError() == ERROR_NOT_ALL_ASSIGNED) {
+    SB_LOG(ERROR) << "The token does not have the specified privilege.";
+    return false;
+  }
+
+  return true;
+}
+
 bool SetTimeZone(const char* time_zone_name) {
   std::string tzName(time_zone_name);
   std::wstring windowsTzName(tzName.begin(), tzName.end());
 
+  HANDLE hToken;
+  if (!OpenProcessToken(GetCurrentProcess(),
+                        TOKEN_ADJUST_PRIVILEGES | TOKEN_QUERY, &hToken)) {
+    SB_LOG(ERROR) << "OpenProcessToken error: " << GetLastError();
+    return false;
+  }
+
+  if (!SetPrivilege(hToken, SE_TIME_ZONE_NAME, true)) {
+    return false;
+  }
+
   DYNAMIC_TIME_ZONE_INFORMATION dtzi{0};
   TIME_ZONE_INFORMATION tzi{0};
   int index = 0;
@@ -51,11 +89,17 @@ bool SetTimeZone(const char* time_zone_name) {
   if (result == 0) {
     DWORD error = GetLastError();
     SB_LOG(ERROR) << "SetDynamicTimeZoneInformation failed for  time zone: "
-              << tzName.c_str() << " return code: " << result
-              << " last error: " << error;
+                  << tzName.c_str() << " return code: " << result
+                  << " last error: " << error;
     return false;
   }
 
+  if (!SetPrivilege(hToken, SE_TIME_ZONE_NAME, false)) {
+    return false;
+  }
+
+  CloseHandle(hToken);
+
   return true;
 }