Review Feedback #1

peiche-ms · peiche-ms · commit 9c139a7cba0b · 2020-09-29T14:00:16.000-07:00
diff --git a/specs/CookieManagement.md b/specs/CookieManagement.md
@@ -4,7 +4,7 @@ Cookie management in WebView has been one of the top feature requests. With that
 
 # Description
 
-One can create a `CookieManager` off a WebView to `GetCookies` via a `CookieList` collection, `SetCookie`, `DeleteCookies`, and `ClearAllCookies` in a WebView environment.
+One can get the `CookieManager` associated with a WebView to `GetCookies` via a `CookieList` collection, `AddOrUpdateCookie`, `DeleteCookies`, and `DeleteAllCookies` in a WebView environment.
 
 # Examples
 
@@ -16,9 +16,9 @@ The following code snippet demonstrates how the cookie management APIs can be us
 ScenarioCookieManagement::ScenarioCookieManagement(AppWindow* appWindow)
     : m_appWindow(appWindow), m_webView(appWindow->GetWebView())
 {
-    //! [CreateCookieManager]
-    CHECK_FAILURE(m_webView->CreateCookieManager(&m_cookieManager));
-    //! [CreateCookieManager]
+    //! [CookieManager]
+    CHECK_FAILURE(m_webViewstaging->get_CookieManager(&m_cookieManager));
+    //! [CookieManager]
 
     CHECK_FAILURE(m_webView->add_WebMessageReceived(
         Microsoft::WRL::Callback<ICoreWebView2WebMessageReceivedEventHandler>(
@@ -43,25 +43,25 @@ ScenarioCookieManagement::ScenarioCookieManagement(AppWindow* appWindow)
                         L"{\"CookiesGot\":\"" + GetCookiesHelper(message.substr(11)) + L"\"}";
                     CHECK_FAILURE(sender->PostWebMessageAsJson(reply.c_str()));
                 }
-                else if (message.compare(0, 10, L"SetCookie ") == 0)
+                else if (message.compare(0, 10, L"AddOrUpdateCookie ") == 0)
                 {
                     message = message.substr(10);
                     std::wstring name = message.substr(0, message.find(' '));
                     std::wstring value = message.substr(message.find(' ') + 1);
 
-                    //! [SetCookie]
+                    //! [AddOrUpdateCookie]
                     wil::com_ptr<ICoreWebView2Cookie> cookie;
                     CHECK_FAILURE(m_cookieManager->CreateCookie(
                         name.c_str(), value.c_str(), L".bing.com", L"/", &cookie));
-                    CHECK_FAILURE(m_cookieManager->SetCookie(cookie.get()));
-                    reply = L"{\"CookieSet\":\"Cookie set successfully.\"}";
+                    CHECK_FAILURE(m_cookieManager->AddOrUpdateCookie(cookie.get()));
+                    reply = L"{\"CookieAddedOrUpdated\":\"Cookie added or updated successfully.\"}";
                     CHECK_FAILURE(sender->PostWebMessageAsJson(reply.c_str()));
-                    //! [SetCookie]
+                    //! [AddOrUpdateCookie]
                 }
                 else if (message.compare(0, 16, L"ClearAllCookies ") == 0)
                 {
-                    CHECK_FAILURE(m_cookieManager->ClearAllCookies());
-                    reply = L"{\"CookiesCleared\":\"Cookies cleared.\"}";
+                    CHECK_FAILURE(m_cookieManager->DeleteAllCookies());
+                    reply = L"{\"CookiesDeleted\":\"Cookies all deleted.\"}";
                     CHECK_FAILURE(sender->PostWebMessageAsJson(reply.c_str()));
                 }
                 return S_OK;
@@ -84,7 +84,7 @@ static std::wstring CookieToJsonString(ICoreWebView2Cookie* cookie)
     double expires;
     CHECK_FAILURE(cookie->get_Expires(&expires));
     BOOL httpOnly;
-    CHECK_FAILURE(cookie->get_HttpOnly(&httpOnly));
+    CHECK_FAILURE(cookie->get_IsHttpOnly(&httpOnly));
     COREWEBVIEW2_COOKIE_SAME_SITE_KIND same_site;
     std::wstring same_site_as_string;
     CHECK_FAILURE(cookie->get_SameSite(&same_site));
@@ -101,7 +101,7 @@ static std::wstring CookieToJsonString(ICoreWebView2Cookie* cookie)
         break;
     }
     BOOL secure;
-    CHECK_FAILURE(cookie->get_Secure(&secure));
+    CHECK_FAILURE(cookie->get_IsSecure(&secure));
 
     std::wstring result = L"{";
     result += L"\"Name\": " + EncodeQuote(name.get()) + L", " + L"\"Value\": " +
@@ -174,39 +174,36 @@ void ScenarioCookieManagement::GetCookiesHelper(std::wstring uri)
 ## .NET and WinRT
 
 ```c#
-CoreWebView2CookieManager _cookieManager;
-
-private void WebView_CoreWebView2Ready(object sender, EventArgs e)
+void CookieManagementCmdsCanExecute(object sender, CanExecuteRoutedEventArgs e)
 {
-    _cookieManager = webView.CoreWebView2.CreateCookieManager();
+    e.CanExecute = webView != null && webView.CoreWebView2 != null && webView.CoreWebView2.CookieManager != null;
 }
 
-void SetCookieCmdExecuted(object target, ExecutedRoutedEventArgs e)
+void AddOrUpdateCookieCmdExecuted(object target, ExecutedRoutedEventArgs e)
 {
-    CoreWebView2Cookie cookie = _cookieManager.CreateCookie("CookieName", "CookieValue", ".bing.com", "/");
+    CoreWebView2Cookie cookie = webView.CoreWebView2.CookieManager.CreateCookie("CookieName", "CookieValue", ".bing.com", "/");
     cookie.SameSite = CoreWebView2CookieSameSiteKind.None;
-    _cookieManager.SetCookie(cookie);
+    webView.CoreWebView2.CookieManager.AddOrUpdateCookie(cookie);
 }
 
 async void GetCookiesCmdExecuted(object target, ExecutedRoutedEventArgs e)
 {
-    CoreWebView2CookieList cookieList = await _cookieManager.GetCookiesAsync("https://www.bing.com");
+    CoreWebView2CookieList cookieList = await webView.CoreWebView2.CookieManager.GetCookiesAsync("https://www.bing.com");
     for (uint i = 0; i < cookieList.Size; ++i)
     {
         CoreWebView2Cookie cookie = cookieList.GetValueAtIndex(i);
         Console.WriteLine(CookieAsString(cookie));
     }
-
 }
 
-void ClearAllCookiesCmdExecuted(object target, ExecutedRoutedEventArgs e)
+void DeleteAllCookiesCmdExecuted(object target, ExecutedRoutedEventArgs e)
 {
-    _cookieManager.ClearAllCookies();
+    webView.CoreWebView2.CookieManager.DeleteAllCookies();
 }
 
 void DeleteCookiesCmdExecuted(object target, ExecutedRoutedEventArgs e)
 {
-    _cookieManager.DeleteCookies("CookieName", "https://www.bing.com", "", "");
+    webView.CoreWebView2.CookieManager.DeleteCookies("CookieName", "https://www.bing.com", "", "");
 }
 ```
 
@@ -240,8 +237,8 @@ typedef enum COREWEBVIEW2_COOKIE_SAME_SITE_KIND {
 
 [uuid(20113081-93BD-4F2A-86B9-ADF92DEAAF10), object, pointer_default(unique)]
 interface ICoreWebView2 : IUnknown {
-  /// Create a new cookie manager object. See ICoreWebView2CookieManager.
-  HRESULT CreateCookieManager([out, retval] ICoreWebView2CookieManager** cookieManager);
+  /// Gets the cookie manager object associated with this ICoreWebView2.
+  [propget] HRESULT CookieManager([out, retval] ICoreWebView2CookieManager** cookieManager);
 }
 
 /// Provides a set of properties that are used to manage an
@@ -274,9 +271,9 @@ interface ICoreWebView2Cookie : IUnknown {
   /// Whether this cookie is http-only.
   /// True if a page script or other active content cannot access this
   /// cookie. The default is false.
-  [propget] HRESULT HttpOnly([out, retval] BOOL* httpOnly);
+  [propget] HRESULT IsHttpOnly([out, retval] BOOL* httpOnly);
   /// Set the HttpOnly property.
-  [propput] HRESULT HttpOnly([in] BOOL httpOnly);
+  [propput] HRESULT IsHttpOnly([in] BOOL httpOnly);
 
   /// SameSite status of the cookie which represents the enforcement mode of the cookie.
   /// The default is COREWEBVIEW2_COOKIE_SAME_SITE_KIND_LAX.
@@ -289,18 +286,20 @@ interface ICoreWebView2Cookie : IUnknown {
   /// The default is false.
   /// Note that cookie that requests COREWEBVIEW2_COOKIE_SAME_SITE_KIND_NONE but
   /// is not marked Secure will be rejected.
-  [propget] HRESULT Secure([out, retval] BOOL* secure);
+  [propget] HRESULT IsSecure([out, retval] BOOL* secure);
   /// Set the Secure property.
-  [propput] HRESULT Secure([in] BOOL secure);
+  [propput] HRESULT IsSecure([in] BOOL secure);
 }
 
-/// Add or delete an ICoreWebView2Cookie or ICoreWebView2Cookies,
-/// or view the cookies. The changes would apply to the context of the user profile.
-/// That is, other WebViews under the same user profile could be affected.
+/// Creates, adds or updates, gets, or or view the cookies. The changes would
+/// apply to the context of the user profile. That is, other WebViews under the
+/// same user profile could be affected.
 [uuid(588C8A15-A28A-4FFD-926B-5E6EE7449E7C), object, pointer_default(unique)]
 interface ICoreWebView2CookieManager : IUnknown {
   /// Create a cookie object with a specified name, value, domain, and path.
   /// One can set other optional properties after cookie creation.
+  /// This only creates a cookie object and it is not added to the cookie
+  /// manager until you call AddOrUpdateCookie.
   /// See ICoreWebView2Cookie for more details.
   HRESULT CreateCookie(
     [in] LPCWSTR name,
@@ -311,15 +310,15 @@ interface ICoreWebView2CookieManager : IUnknown {
 
   /// Gets a list of cookies matching the specific URI.
   /// You can modify the cookie objects, call
-  /// ICoreWebView2CookieManager::SetCookie, and the changes
+  /// ICoreWebView2CookieManager::AddOrUpdateCookie, and the changes
   /// will be applied to the webview.
   HRESULT GetCookies(
     [in] LPCWSTR uri,
     [in] ICoreWebView2GetCookiesCompletedHandler* handler);
 
-  /// Sets a cookie with the given cookie data; may overwrite equivalent cookies
-  /// if they exist.
-  HRESULT SetCookie([in] ICoreWebView2Cookie* cookie);
+  /// Adds or updates a cookie with the given cookie data; may overwrite
+  /// equivalent cookies if they exist.
+  HRESULT AddOrUpdateCookie([in] ICoreWebView2Cookie* cookie);
 
   /// Deletes browser cookies with matching name and uri or domain/path pair.
   /// Cookie name is required.
@@ -329,9 +328,9 @@ interface ICoreWebView2CookieManager : IUnknown {
   /// If path is specified, deletes only cookies with the exact path.
   HRESULT DeleteCookies([in] LPCWSTR name, [in] LPCWSTR uri, [in] LPCWSTR domain, [in] LPCWSTR path);
 
-  /// Clears all cookies under the same profile.
+  /// Deletes all cookies under the same profile.
   /// This could affect other WebViews under the same user profile.
-  HRESULT ClearAllCookies();
+  HRESULT DeleteAllCookies();
 }
 
 /// A list of cookie objects. See ICoreWebView2Cookie.
@@ -360,6 +359,8 @@ interface ICoreWebView2GetCookiesCompletedHandler : IUnknown {
 ```c#
 namespace Microsoft.Web.WebView2.Core
 {
+    // ...
+
     /// Kind of cookie SameSite status used in the CoreWebView2Cookie class.
     /// These fields match those as specified in https://developer.mozilla.org/docs/Web/HTTP/Cookies#.
     /// Learn more about SameSite cookies here: https://tools.ietf.org/html/draft-west-first-party-cookies-07
@@ -375,28 +376,32 @@ namespace Microsoft.Web.WebView2.Core
 
     runtimeclass CoreWebView2
     {
-        /// Create a new cookie manager object.
-        CoreWebView2CookieManager CreateCookieManager();
+        /// Gets the cookie manager object associated with this CoreWebView2.
+        CoreWebView2CookieManager CookieManager { get; };
+
+        // ...
     }
 
-    /// Add or delete an CoreWebView2Cookie or CoreWebView2Cookies,
-    /// or view the cookies. The changes would apply to the context of the user profile.
-    /// That is, other WebViews under the same user profile could be affected.
+    /// Creates, adds or updates, gets, or or view the cookies. The changes would
+    /// apply to the context of the user profile. That is, other WebViews under the
+    /// same user profile could be affected.
     runtimeclass CoreWebView2CookieManager
     {
         /// Create a cookie object with a specified name, value, domain, and path.
         /// One can set other optional properties after cookie creation.
+        /// This only creates a cookie object and it is not added to the cookie
+        /// manager until you call AddOrUpdateCookie.
         CoreWebView2Cookie CreateCookie(String name, String value, String Domain, String Path);
 
         /// Gets a list of cookies matching the specific URI.
         /// You can modify the cookie objects, call
-        /// CoreWebView2CookieManager.SetCookie, and the changes
+        /// CoreWebView2CookieManager.AddOrUpdateCookie, and the changes
         /// will be applied to the webview.
         Windows.Foundation.IAsyncOperation<CoreWebView2CookieList> GetCookiesAsync(String uri);
 
-        /// Sets a cookie with the given cookie data; may overwrite equivalent cookies
-        /// if they exist.
-        void SetCookie(CoreWebView2Cookie cookie);
+        /// Adds or updates a cookie with the given cookie data; may overwrite
+        /// equivalent cookies if they exist.
+        void AddOrUpdateCookie(CoreWebView2Cookie cookie);
 
         /// Deletes browser cookies with matching name and uri or domain/path pair.
         /// Cookie name is required.
@@ -406,9 +411,9 @@ namespace Microsoft.Web.WebView2.Core
         /// If path is specified, deletes only cookies with the exact path.
         void DeleteCookies(String name, String uri, String Domain, String Path);
 
-        /// Clears all cookies under the same profile.
+        /// Deletes all cookies under the same profile.
         /// This could affect other WebViews under the same user profile.
-        void ClearAllCookies();
+        void DeleteAllCookies();
     }
 
     /// A list of cookie objects.
@@ -445,7 +450,7 @@ namespace Microsoft.Web.WebView2.Core
         /// Whether this cookie is http-only.
         /// True if a page script or other active content cannot access this
         /// cookie. The default is false.
-        Int32 HttpOnly { get; set; };
+        Boolean IsHttpOnly { get; set; };
 
         /// SameSite status of the cookie which represents the enforcement mode of the cookie.
         /// The default is CoreWebView2CookieSameSiteKind.Lax.
@@ -456,7 +461,9 @@ namespace Microsoft.Web.WebView2.Core
         /// The default is false.
         /// Note that cookie that requests CoreWebView2CookieSameSiteKind.None but
         /// is not marked Secure will be rejected.
-        Int32 Secure { get; set; };
+        Boolean IsSecure { get; set; };
     }
+
+    // ...
 }
 ```
