Closed Bug 1166645 Opened 7 years ago Closed 7 years ago

Implement MAP profile manager connection related function

Categories

(Firefox OS Graveyard :: Bluetooth, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.2r+, firefox43 fixed, b2g-v2.2r fixed)

RESOLVED FIXED
FxOS-S7 (18Sep)
blocking-b2g 2.2r+
Tracking Status
firefox43 --- fixed
b2g-v2.2r --- fixed

People

(Reporter: shawnjohnjr, Assigned: shawnjohnjr)

References

Details

(Whiteboard: needs-uplift)

Attachments

(2 files, 21 obsolete files)

40.45 KB, patch
Details | Diff | Splinter Review
41.78 KB, patch
Details | Diff | Splinter Review
MAP Message Server Equipment (MSE) role.
1. Listen MAP MSE socket
2. Handle MCE connection/disconnection requests
Assignee: nobody → shuang
Attached patch bug1166645-mc.patch(WIP) (obsolete) — Splinter Review
1. Handle foler listing query
2. Add virtual folder
Comment on attachment 8644904 [details] [diff] [review]
Bug 1166645 - Implement MAP profile manager connection related function

Review of attachment 8644904 [details] [diff] [review]:
-----------------------------------------------------------------

Please see my comment below.

::: dom/bluetooth/bluedroid/BluetoothMapFolder.cpp
@@ +14,5 @@
> +BluetoothMapFolder::~BluetoothMapFolder()
> +{ }
> +
> +BluetoothMapFolder::BluetoothMapFolder(const nsAString& aFolderName,
> +  BluetoothMapFolder* aParent) : mName(aFolderName), mParent(aParent)

nit: Align as following

BluetoothMapFolder::BluetoothMapFolder(const nsAString& aFolderName,
                                       BluetoothMapFolder* aParent)
  : mName(aFolderName)
  , mParent(aParent)

@@ +51,5 @@
> +  // Based on Element Specification, 9.1.1, IrObex 1.2
> +  nsAutoCString folderListingObejct("<?xml version=\"1.0\"?>\n");
> +  folderListingObejct.
> +    Append("<!DOCTYPE folder-listing SYSTEM \" obex-folder-listing.dtd\">\n");
> +  folderListingObejct.Append("<folder-listing version=\"1.0\">\n");

Replace with defined strings.

  #define FOLDER_LIST_PREFIX "<?xml ..."
  #define FOLDER_LIST_SUFFIX "</folder-listing>"

@@ +53,5 @@
> +  folderListingObejct.
> +    Append("<!DOCTYPE folder-listing SYSTEM \" obex-folder-listing.dtd\">\n");
> +  folderListingObejct.Append("<folder-listing version=\"1.0\">\n");
> +
> +  for (auto iter = mSubFolders.Iter(); !iter.Done(); iter.Next()) {

Declare |count| right before for loop to be close where it's first used.

@@ +58,5 @@
> +    if (count > aMaxListCount) {
> +      break;
> +    }
> +
> +    if (count < aStartOffset) {

nit: switch the order of these if-conditions since the second one decides where to start and first one where to end.

@@ +64,5 @@
> +    }
> +
> +    const nsAString& key = iter.GetKey();
> +    folderListingObejct.Append("<folder name=\"");
> +    folderListingObejct.Append( NS_ConvertUTF16toUTF8(key).get());

nit: remove extra space after (.

::: dom/bluetooth/bluedroid/BluetoothMapFolder.h
@@ +4,5 @@
> + * License, v. 2.0. If a copy of the MPL was not distributed with this file,
> + * You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +#ifndef mozilla_dom_bluetooth_bluetoothmapfolder_h__
> +#define mozilla_dom_bluetooth_bluetoothmapfolder_h__

Revise to |mozilla_dom_bluetooth_bluedroid_BluetoothMapFolder_h|. See bug 1106007 for more info.

@@ +9,5 @@
> +
> +#include "BluetoothCommon.h"
> +#include "nsAutoPtr.h"
> +#include "nsTArray.h"
> +#include "nsRefPtrHashtable.h"

nit: alphabetical order.

@@ +13,5 @@
> +#include "nsRefPtrHashtable.h"
> +
> +BEGIN_BLUETOOTH_NAMESPACE
> +
> +/* This class maps MAP virtual folder strctures */

nit: str'u'ctures

@@ +37,5 @@
> +  nsRefPtrHashtable<nsStringHashKey, BluetoothMapFolder> mSubFolders;
> +};
> +
> +END_BLUETOOTH_NAMESPACE
> +#endif

Add // mozilla_dom_bluetooth_bluedroid_BluetoothMapFolder_h

::: dom/bluetooth/bluedroid/BluetoothMapSmsManager.cpp
@@ +65,5 @@
> +
> +NS_IMETHODIMP
> +BluetoothMapSmsManager::Observe(nsISupports* aSubject,
> +                              const char* aTopic,
> +                              const char16_t* aData)

nit: align these lines.

@@ +180,5 @@
> +    mServerSocket = nullptr;
> +  }
> +
> +  mServerSocket =
> +    new BluetoothSocket(this);

nit: put in one line.

@@ +203,5 @@
> +  sdpString.AppendLiteral("SMS/MMS Message Access");
> +  nsresult rv = mServerSocket->Listen(sdpString, kMapMas,
> +                                      BluetoothSocketType::RFCOMM, -1, false,
> +                                      true);
> +

nit: remove this newline.

@@ +215,5 @@
> +
> +// Virtual function of class SocketConsumer
> +void
> +BluetoothMapSmsManager::ReceiveSocketData(BluetoothSocket* aSocket,
> +                                        nsAutoPtr<UnixSocketBuffer>& aMessage)

nit: alignment

@@ +219,5 @@
> +                                        nsAutoPtr<UnixSocketBuffer>& aMessage)
> +{
> +  MOZ_ASSERT(NS_IsMainThread());
> +
> +  if (aSocket == mMnsSocket) {

Wrap this section into function |MnsDataHandler|.

@@ +231,5 @@
> +    const uint8_t* data = aMessage->GetData();
> +    uint8_t opCode = data[0];
> +
> +    // Check response code and send out system message as finished if the
> +    // response code is somehow incorrect.

Remove this outdated comment since we don't send out system message here.

@@ +233,5 @@
> +
> +    // Check response code and send out system message as finished if the
> +    // response code is somehow incorrect.
> +    uint8_t expectedOpCode = ObexResponseCode::Success;
> +    if (opCode != expectedOpCode) {

Simplify to

  if (opCode != ObexResponseCode::Success) {

@@ +234,5 @@
> +    // Check response code and send out system message as finished if the
> +    // response code is somehow incorrect.
> +    uint8_t expectedOpCode = ObexResponseCode::Success;
> +    if (opCode != expectedOpCode) {
> +      BT_LOGR("Not expectedOpCode: %d", opCode);

"Unexpected opCode: %x"

@@ +240,5 @@
> +          mLastCommand == ObexRequestCode::Put ||
> +          mLastCommand == ObexRequestCode::Abort ||
> +          mLastCommand == ObexRequestCode::PutFinal) {
> +        SendMnsClientDisconnectRequest();
> +      }

What to do if |mLastCommand| is not one of these four?

@@ +243,5 @@
> +        SendMnsClientDisconnectRequest();
> +      }
> +    }
> +  } else {
> +    /**

Wrap this section into function |MnsDataHandler|.

@@ +360,5 @@
> +          BT_LOGR("MAS folder-listing, current path: %s",
> +            NS_ConvertUTF16toUTF8(mCurrentPath).get());
> +          HandleSmsMmsFolderListing(pktHeaders);
> +        } else if (type.EqualsLiteral("x-bt/MAP-msg-listing")) {
> +        } else if (type.EqualsLiteral("x-bt/message"))  {

Please leave comment if you'll implement in later patches.

@@ +404,5 @@
> +}
> +
> +uint8_t
> +BluetoothMapSmsManager::SetPath(uint8_t flags,
> +                                       const ObexHeaderSet& aHeader)

nit: alignment

@@ +406,5 @@
> +uint8_t
> +BluetoothMapSmsManager::SetPath(uint8_t flags,
> +                                       const ObexHeaderSet& aHeader)
> +{
> +  // Section 5.2 "SetPat Function", MapSms 1.2

nit: SetPat'h'

@@ +531,5 @@
> +
> +  req[3] = 0x10; // version=1.0
> +  req[4] = 0x00; // flag=0x00
> +  req[5] = BluetoothMapSmsManager::MAX_PACKET_LENGTH >> 8;
> +  req[6] = (uint8_t) BluetoothMapSmsManager::MAX_PACKET_LENGTH;

nit: remove space after ).

@@ +590,5 @@
> +
> +void
> +BluetoothMapSmsManager::CreateMnsObexConnection()
> +{
> +  if (!mMnsSocket) {

Revise with guardian clause:

  if (mMnsSocket) {
    return;
  }

  mMnsSocket = new BluetoothSocket(this);
  mMnsSocket->Connect(mDeviceAddress, kMapMns,
                      BluetoothSocketType::RFCOMM, -1, false, false);

@@ +592,5 @@
> +BluetoothMapSmsManager::CreateMnsObexConnection()
> +{
> +  if (!mMnsSocket) {
> +    mMnsSocket = new BluetoothSocket(this);
> +    // Already encrypted in previous session?

What's the ? comment for?

@@ +601,5 @@
> +
> +void
> +BluetoothMapSmsManager::DestroyMnsObexConnection()
> +{
> +  if (mMnsSocket) {

Revise with guardian clause:

  if (!mMnsSocket) {
    return;
  }

  mMnsSocket->Close();

@@ +604,5 @@
> +{
> +  if (mMnsSocket) {
> +    mMnsSocket->Close();
> +  } else {
> +    BT_LOGR("MSN client socket is not opened");

nit: MNS

@@ +652,5 @@
> +
> +void
> +BluetoothMapSmsManager::HandleSmsMmsFolderListing(const ObexHeaderSet& aHeader)
> +{
> +  MOZ_ASSERT(NS_IsMainThread());

nit: add newline after assertion.

@@ +662,5 @@
> +  int index = 3;
> +
> +  foldersize = mCurrentFolder->GetSubFolderSize();
> +  BT_LOGR("CurrentPath: %s, sub folder size: %d",
> +    NS_ConvertUTF16toUTF8(mCurrentPath).get(), foldersize);

Move this section to below.

@@ +664,5 @@
> +  foldersize = mCurrentFolder->GetSubFolderSize();
> +  BT_LOGR("CurrentPath: %s, sub folder size: %d",
> +    NS_ConvertUTF16toUTF8(mCurrentPath).get(), foldersize);
> +
> +  if (aHeader.GetAppParameter(Map::AppParametersTagId::MaxListCount,

Declare |maxListCount| right before if to close where it's first used.

@@ +671,5 @@
> +    // convert big endian to little endian
> +    maxListCount = (maxListCount >> 8) | (maxListCount << 8);
> +  }
> +
> +  if (aHeader.GetAppParameter(Map::AppParametersTagId::StartOffset,

Ditto.

@@ +678,5 @@
> +    // convert big endian to little endian
> +    startOffset = (startOffset >> 8) | (startOffset << 8);
> +  }
> +
> +  // folder listing size

Revise as following:

  // Folder listing size
  int foldersize = mCurrentFolder->GetSubFolderSize();
  BT_LOGR("CurrentPath: %s, sub folder size: %d",
          NS_ConvertUTF16toUTF8(mCurrentPath).get(), foldersize);

  // Convert little endian to big endian
  uint8_t folderListingSizeValue[2];
  folderListingSizeValue[0] = (foldersize & 0xFF00) >> 8;
  folderListingSizeValue[1] = (foldersize & 0x00FF);

  // Section 3.3.4 "GetResponse", IrOBEX 1.2
  // [opcode:1][length:2][Headers:var] where
  // [FolderListingSize:4] = [tagId:1][length:1][value: 2]
  AppendAppParameter(appParameter, sizeof(appParameter),
                     (uint8_t) Map::AppParametersTagId::FolderListingSize,
                     folderListingSizeValue, sizeof(folderListingSizeValue));

@@ +680,5 @@
> +  }
> +
> +  // folder listing size
> +  // Section 3.3.4 "GetResponse", IrOBEX 1.2
> +  // [opcode:1][length:2][Headers:var] where

Where is [FolderListingSize:4]?

@@ +689,5 @@
> +  // Convert little endian to big endian
> +  folderListingSizeValue[0] = (foldersize & 0xFF00) >> 8;
> +  folderListingSizeValue[1] = (foldersize & 0x00FF);
> +  AppendAppParameter(appParameter, sizeof(appParameter),
> +                     (uint8_t) Map::AppParametersTagId::FolderListingSize,

nit: remove space after )

@@ +695,5 @@
> +
> +  index += AppendHeaderAppParameters(&resp[index], 255, appParameter,
> +                                     sizeof(appParameter));
> +
> +  if (maxListCount == 0) {

if (!maxListCount) {

@@ +744,5 @@
> +{
> +  MOZ_ASSERT(NS_IsMainThread());
> +
> +  uint8_t buf[64];
> +  if (aHeader.GetAppParameter(Map::AppParametersTagId::NotificationStatus,

Revise with guardian clause as following. Also remove |mNtfRegistered| since it's unused elsewhere.

  if (!aHeader.Get( ... )) {
    return;
  }

  if (buf[0]) {
    CreateMnsObexConnection();
  } else {
    DestroyMnsObexConnection();
  }

@@ +749,5 @@
> +                              buf, 64)) {
> +    mNtfRegistered = buf[0];
> +    /*
> +     * Initialization sequence for a MAP session that uses both the Messsage
> +     * Access service and the Message Notification service. The MSN connection

MNS

@@ +751,5 @@
> +    /*
> +     * Initialization sequence for a MAP session that uses both the Messsage
> +     * Access service and the Message Notification service. The MSN connection
> +     * shall be established by the first SetNotificationRegistration set to ON
> +     * during MAP session. Only one MSN connection per device pair.

MNS

@@ +790,5 @@
> +
> +void
> +BluetoothMapSmsManager::SendObexData(uint8_t* aData, uint8_t aOpcode, int aSize)
> +{
> +  mLastCommand = aOpcode;

Store |mLastCommand| only for client as [1] since server replies also call |SendObexData|.
[1] https://dxr.mozilla.org/mozilla-central/source/dom/bluetooth/bluedroid/BluetoothOppManager.cpp#1402

Also will you add more client commands? This patch only implements |ObexRequestCode::Connect| and |ObexRequestCode::Disonnect|.

@@ +835,5 @@
> +
> +  if (aSocket == mMnsSocket) {
> +    mMnsConnected = false;
> +    mMnsSocket = nullptr;
> +    BT_LOGR("MSN socket disconnected");

nit: MNS

@@ +865,5 @@
> +NS_IMPL_ISUPPORTS(BluetoothMapSmsManager, nsIObserver)
> +
> +void
> +BluetoothMapSmsManager::Connect(const nsAString& aDeviceAddress,
> +                              BluetoothProfileController* aController)

nit: alignment

@@ +873,5 @@
> +
> +void
> +BluetoothMapSmsManager::OnGetServiceChannel(const nsAString& aDeviceAddress,
> +                                          const nsAString& aServiceUuid,
> +                                          int aChannel)

nit: alignment

::: dom/bluetooth/bluedroid/BluetoothMapSmsManager.h
@@ +4,5 @@
> + * License, v. 2.0. If a copy of the MPL was not distributed with this file,
> + * You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +#ifndef mozilla_dom_bluetooth_bluetoothmapsmsmanager_h__
> +#define mozilla_dom_bluetooth_bluetoothmapsmsmanager_h__

Ditto. Revise to |mozilla_dom_bluetooth_bluedroid_BluetoothMapSmsManager_h|.

@@ +46,5 @@
> +};
> +
> +class BluetoothSocket;
> +class ObexHeaderSet;
> +class BluetoothNamedValue;

nit: alphabetical order.

@@ +50,5 @@
> +class BluetoothNamedValue;
> +
> +/*
> + * BluetoothMapSmsManager acts as Message Server Equipment (MSE) and runs both
> + * MAS server and MNS client to exchange Sms/MMS message.

nit: SMS/MMS for consistency

@@ +51,5 @@
> +
> +/*
> + * BluetoothMapSmsManager acts as Message Server Equipment (MSE) and runs both
> + * MAS server and MNS client to exchange Sms/MMS message.
> + *

Remove this extra line.

@@ +55,5 @@
> + *
> + */
> +
> +class BluetoothMapSmsManager : public BluetoothSocketObserver
> +                          , public BluetoothProfileManagerBase

nit: alignment

@@ +71,5 @@
> +  static const int SDP_MESSAGE_TYPE_EMAIL = 0x01;
> +  static const int SDP_MESSAGE_TYPE_SMS_GSM = 0x02;
> +  static const int SDP_MESSAGE_TYPE_SMS_CDMA = 0x04;
> +  static const int SDP_MESSAGE_TYPE_MMS = 0x08;
> +  // By defualt SMS/MMS is default support type

Do you mean "By default SMS/MMS type is supported"?

@@ +103,5 @@
> +  void AfterMapSmsDisconnected();
> +  void CreateMnsObexConnection();
> +  void DestroyMnsObexConnection();
> +  void SendMnsClientConnectRequest();
> +  void SendMnsClientDisconnectRequest();

Rename to |SendMnsConnectRequest| since connect request implies client role.

@@ +132,5 @@
> +
> +  // If a connection has been established, mSocket will be the socket
> +  // communicating with the remote socket. We maintain the invariant that if
> +  // mSocket is non-null, mServerSocket must be null (and vice versa).
> +  nsRefPtr<BluetoothSocket> mSocket;

Rename to |mMasSocket|.

@@ +137,5 @@
> +
> +  // Server socket. Once an inbound connection is established, it will hand
> +  // over the ownership to mSocket, and get a new server socket while Listen()
> +  // is called.
> +  nsRefPtr<BluetoothSocket> mServerSocket;

Rename to |mMasServerSocket|.

@@ +139,5 @@
> +  // over the ownership to mSocket, and get a new server socket while Listen()
> +  // is called.
> +  nsRefPtr<BluetoothSocket> mServerSocket;
> +
> +  // Message Notification service client socket

nit: to lower case - 'n'otification

@@ +145,5 @@
> +};
> +
> +END_BLUETOOTH_NAMESPACE
> +
> +#endif

Add // mozilla_dom_bluetooth_bluedroid_BluetoothMapSmsManager_h

::: dom/bluetooth/bluedroid/BluetoothServiceBluedroid.cpp
@@ +1918,5 @@
>        BT_LOGR("Fail to start BluetoothPbapManager listening");
>      }
> +
> +    BluetoothMapSmsManager* map = BluetoothMapSmsManager::Get();
> +    BT_LOGR("Start BluetoothMapSmsManager listening");

Move this log into |BluetoothMapSmsManager::Listen|.

@@ +2007,5 @@
>        BT_LOGR("Fail to start BluetoothPbapManager listening");
>      }
> +
> +    BluetoothMapSmsManager* map = BluetoothMapSmsManager::Get();
> +    BT_LOGR("Start BluetoothMapSmsManager listening");

Ditto.
Attachment #8644904 - Flags: review?(btian)
Comment on attachment 8644904 [details] [diff] [review]
Bug 1166645 - Implement MAP profile manager connection related function

Review of attachment 8644904 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bluetooth/bluedroid/BluetoothMapFolder.cpp
@@ +51,5 @@
> +  // Based on Element Specification, 9.1.1, IrObex 1.2
> +  nsAutoCString folderListingObejct("<?xml version=\"1.0\"?>\n");
> +  folderListingObejct.
> +    Append("<!DOCTYPE folder-listing SYSTEM \" obex-folder-listing.dtd\">\n");
> +  folderListingObejct.Append("<folder-listing version=\"1.0\">\n");

Do you want to put "<?xml version...<folder-listing version=\"1.0\">\n" into FOLDER_LIST_PREFIX? I guess that will be too long and not able to read easily.

::: dom/bluetooth/bluedroid/BluetoothMapSmsManager.cpp
@@ +231,5 @@
> +    const uint8_t* data = aMessage->GetData();
> +    uint8_t opCode = data[0];
> +
> +    // Check response code and send out system message as finished if the
> +    // response code is somehow incorrect.

Thanks! I forgot to remove this comment.

@@ +240,5 @@
> +          mLastCommand == ObexRequestCode::Put ||
> +          mLastCommand == ObexRequestCode::Abort ||
> +          mLastCommand == ObexRequestCode::PutFinal) {
> +        SendMnsClientDisconnectRequest();
> +      }

I actually reference this code from |BluetoothOppManager::ClientDataHandler|. And I think I'm wrong about |ObexRequestCode::Get|, MNS client doesn't support |Get| anyway.

@@ +680,5 @@
> +  }
> +
> +  // folder listing size
> +  // Section 3.3.4 "GetResponse", IrOBEX 1.2
> +  // [opcode:1][length:2][Headers:var] where

I don't understand this question.

@@ +744,5 @@
> +{
> +  MOZ_ASSERT(NS_IsMainThread());
> +
> +  uint8_t buf[64];
> +  if (aHeader.GetAppParameter(Map::AppParametersTagId::NotificationStatus,

Will it be good to keep |mNtfRegistered| so we can keep track that MCE already sent MAP-NotificationRegistration request? Maybe we shall handle one case that registered but fail to get connected.

@@ +790,5 @@
> +
> +void
> +BluetoothMapSmsManager::SendObexData(uint8_t* aData, uint8_t aOpcode, int aSize)
> +{
> +  mLastCommand = aOpcode;

MNS client shall support connect/disconnect/put/abort operations (see 6.2.2 Table 6.2). And I handle |MnsDataHandler| to disconnect obex if not success. Does it make sense? I think I will use |mMnsConnected| to check.

@@ +835,5 @@
> +
> +  if (aSocket == mMnsSocket) {
> +    mMnsConnected = false;
> +    mMnsSocket = nullptr;
> +    BT_LOGR("MSN socket disconnected");

I guess Schizophrenia is hunting on me.

::: dom/bluetooth/bluedroid/BluetoothMapSmsManager.h
@@ +20,5 @@
> +    MaxListCount                  = 0x1,
> +    StartOffset                   = 0x2,
> +    FilterMessageType             = 0x3,
> +    FilterPeriodBegin             = 0x4,
> +    EndFilterPeriodEnd            = 0x5,

I found the specification 6.3.1 Table 6.5 is wrong. 'EndFilterPeriodEnd' shall be 'FilterPeriodEnd'.

::: dom/bluetooth/bluedroid/BluetoothServiceBluedroid.cpp
@@ +1918,5 @@
>        BT_LOGR("Fail to start BluetoothPbapManager listening");
>      }
> +
> +    BluetoothMapSmsManager* map = BluetoothMapSmsManager::Get();
> +    BT_LOGR("Start BluetoothMapSmsManager listening");

I think I should remove this log.
Attachment #8644904 - Flags: feedback?(jaliu)
Attachment #8646940 - Flags: review?(btian)
Attachment #8646940 - Flags: feedback?(jaliu)
(In reply to Shawn Huang [:shawnjohnjr] from comment #11)
> ::: dom/bluetooth/bluedroid/BluetoothMapFolder.cpp
> @@ +51,5 @@
> > +  // Based on Element Specification, 9.1.1, IrObex 1.2
> > +  nsAutoCString folderListingObejct("<?xml version=\"1.0\"?>\n");
> > +  folderListingObejct.
> > +    Append("<!DOCTYPE folder-listing SYSTEM \" obex-folder-listing.dtd\">\n");
> > +  folderListingObejct.Append("<folder-listing version=\"1.0\">\n");
> 
> Do you want to put "<?xml version...<folder-listing version=\"1.0\">\n" into
> FOLDER_LIST_PREFIX? I guess that will be too long and not able to read
> easily.

You can format in string definition for better readability.

> @@ +680,5 @@
> > +  }
> > +
> > +  // folder listing size
> > +  // Section 3.3.4 "GetResponse", IrOBEX 1.2
> > +  // [opcode:1][length:2][Headers:var] where
> 
> I don't understand this question.

Please state in comment where [FolderListingSize:4] is.

> @@ +744,5 @@
> > +{
> > +  MOZ_ASSERT(NS_IsMainThread());
> > +
> > +  uint8_t buf[64];
> > +  if (aHeader.GetAppParameter(Map::AppParametersTagId::NotificationStatus,
> 
> Will it be good to keep |mNtfRegistered| so we can keep track that MCE
> already sent MAP-NotificationRegistration request? Maybe we shall handle one
> case that registered but fail to get connected.

Agree. But please add the corresponding code together in one patch.

> @@ +790,5 @@
> > +
> > +void
> > +BluetoothMapSmsManager::SendObexData(uint8_t* aData, uint8_t aOpcode, int aSize)
> > +{
> > +  mLastCommand = aOpcode;
> 
> MNS client shall support connect/disconnect/put/abort operations (see 6.2.2
> Table 6.2). And I handle |MnsDataHandler| to disconnect obex if not success.
> Does it make sense? I think I will use |mMnsConnected| to check.

So more command implementation will come in later patches. Please separate outgoing requests and replies to remember only requests.
Attachment #8646940 - Attachment is obsolete: true
Attachment #8646940 - Flags: review?(btian)
Attachment #8646940 - Flags: feedback?(jaliu)
Comment on attachment 8647341 [details] [diff] [review]
bug1166645-mc.patch (v3)

Review of attachment 8647341 [details] [diff] [review]:
-----------------------------------------------------------------

Please see my comment below.

::: dom/bluetooth/bluedroid/BluetoothMapFolder.cpp
@@ +10,5 @@
> +
> +#define FOLDER_LISTING_PREFIX                                                 \
> +    "<?xml version=\"1.0\"?>\n                                                \
> +    <!DOCTYPE folder-listing SYSTEM \" obex-folder-listing.dtd\">\n           \
> +    <folder-listing version=\"1.0\">\n"

nit: two-space indention

@@ +20,5 @@
> +BluetoothMapFolder::~BluetoothMapFolder()
> +{ }
> +
> +BluetoothMapFolder::BluetoothMapFolder(const nsAString& aFolderName,
> +                    BluetoothMapFolder* aParent)

nit: align with (

::: dom/bluetooth/bluedroid/BluetoothMapFolder.h
@@ +18,5 @@
> +class BluetoothMapFolder
> +{
> +public:
> +  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(BluetoothMapFolder)
> +  BluetoothMapFolder();

Can we remove the constructor with default parameter values?

@@ +23,5 @@
> +  ~BluetoothMapFolder();
> +
> +  BluetoothMapFolder(const nsAString& aFolderName, BluetoothMapFolder* aParent);
> +  // Add virtual folder to subfolders
> +  void AddFolder(const nsAString& aFolderName);

Rename to |AddSubFolder| to be clearer.

@@ +25,5 @@
> +  BluetoothMapFolder(const nsAString& aFolderName, BluetoothMapFolder* aParent);
> +  // Add virtual folder to subfolders
> +  void AddFolder(const nsAString& aFolderName);
> +  BluetoothMapFolder* GetSubFolder(const nsAString& aFolderName);
> +  int GetSubFolderSize();

Rename to |GetSubFolderCount| since folder size may be mis-regarded as sum of file sizes under the subfolder.

::: dom/bluetooth/bluedroid/BluetoothMapSmsManager.cpp
@@ +32,5 @@
> +      0x80, 0x00, 0x00, 0x80, 0x5F, 0x9B, 0x34, 0xFB
> +    }
> +  };
> +
> +  // UUID of Map Mas

nit: put Map Mas UUID before MNS one.

@@ +203,5 @@
> +   */
> +  sdpString.AppendLiteral("SMS/MMS Message Access");
> +  nsresult rv = mMasServerSocket->Listen(sdpString, kMapMas,
> +                                      BluetoothSocketType::RFCOMM, -1, false,
> +                                      true);

nit: alignment

@@ +225,5 @@
> +
> +  const uint8_t* data = aMessage->GetData();
> +  uint8_t opCode = data[0];
> +  if (opCode != ObexResponseCode::Success) {
> +    BT_LOGR("Not expectedOpCode: %x", opCode);

Revise to "Unexpected opCode" since "Not expectedOpCode: %x" is ambiguous that whether %x is expected or not.

@@ +411,5 @@
> +  return true;
> +}
> +
> +uint8_t
> +BluetoothMapSmsManager::SetPath(uint8_t flags,

Integrate |mCurrentPath| into |mCurrentFolder| in this function.

@@ +463,5 @@
> +
> +  mCurrentPath = newPath;
> +  BT_LOGR("MAS Current path [%s]", NS_ConvertUTF16toUTF8(mCurrentPath).get());
> +
> +  if (mCurrentPath.IsEmpty()) {

Where do you handle "go up 1 level"?

@@ +615,5 @@
> +  if (!mMnsSocket) {
> +    return;
> +  }
> +
> +  mMnsSocket->Close();

Reset |mMnsSocket| to nullptr.

@@ +682,5 @@
> +    startOffset = (startOffset >> 8) | (startOffset << 8);
> +  }
> +
> +  // folder listing size
> +  int foldersize = mCurrentFolder->GetSubFolderSize();

nit: revise as following to be clearer

  // Folder listing size
  int foldersize = mCurrentFolder->GetSubFolderSize();
  BT_LOGR("CurrentPath: %s, sub folder size: %d",
          NS_ConvertUTF16toUTF8(mCurrentPath).get(), foldersize);

  // Convert little endian to big endian
  uint8_t folderListingSizeValue[2];
  folderListingSizeValue[0] = (foldersize & 0xFF00) >> 8;
  folderListingSizeValue[1] = (foldersize & 0x00FF);

@@ +698,5 @@
> +  AppendAppParameter(appParameter, sizeof(appParameter),
> +                     (uint8_t)Map::AppParametersTagId::FolderListingSize,
> +                     folderListingSizeValue, sizeof(folderListingSizeValue));
> +
> +  index += AppendHeaderAppParameters(&resp[index], 255, appParameter,

Declare |resp| and |index| here, right before where they are first used.

@@ +712,5 @@
> +                                NS_ConvertUTF16toUTF8(output).get()),
> +                              NS_ConvertUTF16toUTF8(output).Length());
> +
> +    index += AppendHeaderEndOfBody(&resp[index]);
> +    SendMasObexData(resp, ObexResponseCode::Success, index);

Revise as following:

  /* MCE wants to query sub-folder size
   * FolderListingSize AppParameter shall be used in the response if the value
   * of MaxListCount in the request is 0. If MaxListCount = 0, the MSE shall
   * ignore all other applications parameters that may be presented in the
   * request. The response shall contain any Body header.
   */
  if (!maxListCount) {
    ...
  }

  SendObexData(resp, ObexResponseCode::Success, index);

@@ +745,5 @@
> +}
> +
> +void
> +BluetoothMapSmsManager::HandleNotificationRegistration(const ObexHeaderSet&
> +                                                       aHeader)

nit: align as following

BluetoothMapSmsManager::HandleNotificationRegistration(
  const ObexHeaderSet& aHeader)

@@ +756,5 @@
> +    return;
> +  }
> +
> +  bool ntfRegistered = static_cast<bool>(buf[0]);
> +

nit: remove this newline.

@@ +783,5 @@
> +
> +void
> +BluetoothMapSmsManager::HandleEventReport(const ObexHeaderSet& aHeader)
> +{
> +  // TODO: Handle Handle event report in Bug 1166666

nit: remove extra 'Handle'

@@ +826,5 @@
> +void
> +BluetoothMapSmsManager::OnSocketConnectSuccess(BluetoothSocket* aSocket)
> +{
> +  MOZ_ASSERT(aSocket);
> +  MOZ_ASSERT(!mMasSocket);

Remove this assertion since |mMasSocket| is non-nullptr when |mMnsSocket| connect success.

@@ +828,5 @@
> +{
> +  MOZ_ASSERT(aSocket);
> +  MOZ_ASSERT(!mMasSocket);
> +
> +  if (aSocket == mMnsSocket) {

Add following comment:

  // MNS socket is connected
  if (aSocket == mMnsSocket) {

@@ +834,5 @@
> +    SendMnsConnectRequest();
> +    return;
> +  }
> +
> +  // Close server socket as only one session is allowed at a time

Add following comment:

  // MAS socket is connected
  // Close server socket as only one session is allowed at a time

@@ +845,5 @@
> +
> +void
> +BluetoothMapSmsManager::OnSocketConnectError(BluetoothSocket* aSocket)
> +{
> +  if (aSocket == mMnsSocket) {

Add following comment:

  // MNS socket connection error
  if (aSocket == mMnsSocket) {

@@ +846,5 @@
> +void
> +BluetoothMapSmsManager::OnSocketConnectError(BluetoothSocket* aSocket)
> +{
> +  if (aSocket == mMnsSocket) {
> +    mMnsConnected = false;

Should we reset |mMnsSocket| as nullptr here?

@@ +859,5 @@
> +BluetoothMapSmsManager::OnSocketDisconnect(BluetoothSocket* aSocket)
> +{
> +  MOZ_ASSERT(aSocket);
> +
> +  if (aSocket == mMnsSocket) {

Add following comment:

  // MNS socket is disconnected
  if (aSocket == mMnsSocket) {

@@ +866,5 @@
> +    BT_LOGR("MNS socket disconnected");
> +    return;
> +  }
> +
> +  if (aSocket != mMasSocket) {

Add following comment:

  // MAS server socket is closed
  if (aSocket != mMasSocket) {

@@ +871,5 @@
> +    // Do nothing when a listening server socket is closed.
> +    return;
> +  }
> +
> +  AfterMapSmsDisconnected();

Add following comment:

  // MAS socket is disconnected
  AfterMapSmsDisconnected();

@@ +881,5 @@
> +
> +void
> +BluetoothMapSmsManager::Disconnect(BluetoothProfileController* aController)
> +{
> +  if (mMasSocket) {

Revise with guardian clause:

  if (!mMasSocket) {
    BT_WARNING("%s: No ongoing connection to disconnect", __FUNCTION__);
    return;
  }

  mMasSocket->Close();

::: dom/bluetooth/bluedroid/BluetoothMapSmsManager.h
@@ +116,5 @@
> +  void HandleSmsMmsFolderListing(const ObexHeaderSet& aHeader);
> +  /**
> +   * Current virtual folder path
> +   */
> +  nsString mCurrentPath;

Remove |mCurrentPath| and remember current folder in |mCurrentFolder|.

@@ +129,5 @@
> +   * Record the last command
> +   */
> +  int mLastCommand;
> +  bool mMnsConnected;
> +  bool mNtfRegistered;

Rename to |mNtfRequired|.
Attachment #8647341 - Flags: review?(btian)
Attached patch bug1166645-mc.patch (v4) (obsolete) — Splinter Review
I rebased to the latest m-c, and test with MCE equipment.
Attachment #8647934 - Attachment is obsolete: true
Attachment #8647934 - Flags: review?(btian)
Comment on attachment 8647953 [details] [diff] [review]
bug1166645-mc.patch (v4)

Review of attachment 8647953 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bluetooth/bluedroid/BluetoothMapFolder.cpp
@@ +41,5 @@
> +  return mParent;
> +}
> +
> +void
> +BluetoothMapFolder::GetFolderName(nsString& aFolderName)

Replace this function with |PrintFolderName()| to print out folder name directly.

::: dom/bluetooth/bluedroid/BluetoothMapSmsManager.cpp
@@ +432,5 @@
> +    // Go up 1 level
> +    BluetoothMapFolder* parent = mCurrentFolder->GetParentFolder();
> +    if (!parent) {
> +      BT_LOGR("MAS SetPath Go up 1 level");
> +      mCurrentFolder = parent;

nit: assign first and then print log to conform with below.

@@ +447,5 @@
> +      BT_LOGR("MAS SetPath Go back to root");
> +    } else {
> +      // Go down 1 level
> +      BluetoothMapFolder* child = mCurrentFolder->GetSubFolder(childFolderName);
> +

nit: remove this newline

@@ +449,5 @@
> +      // Go down 1 level
> +      BluetoothMapFolder* child = mCurrentFolder->GetSubFolder(childFolderName);
> +
> +      if (!child) {
> +        return ObexResponseCode::NotFound;

Print log

  BT_LOGR("Illegal subfolder name [%s]",
          NS_ConvertUTF16toUTF8(childFolderName));

@@ +667,5 @@
> +                     folderListingSizeValue, sizeof(folderListingSizeValue));
> +
> +  uint8_t resp[255];
> +  int index = 3;
> +

nit: remove this newline.

@@ +677,5 @@
> +   * If MaxListCount = 0, the MSE shall ignore all other applications
> +   * parameters that may be presented in the request. The response shall
> +   * contain any Body header.
> +   */
> +  if (maxListCount) {

The condition was |!maxListCount| in previous patch. Which one is correct?

@@ +740,5 @@
> +   * during MAP session. Only one MNS connection per device pair.
> +   * Section 6.4.2, MAP
> +   * If the Message Access connection is disconnected after Message Notification
> +   * connection establishment, this will automatically indicate a MAS
> +   * Notification-Deregistration for this MAS instance.

Where do we do auto de-registration?

@@ +821,5 @@
> +    mMnsSocket = nullptr;
> +    return;
> +  }
> +
> +  mMasServerSocket = nullptr;

Add comment // MAS socket connection error

@@ +856,5 @@
> +void
> +BluetoothMapSmsManager::Disconnect(BluetoothProfileController* aController)
> +{
> +  if (!mMasSocket) {
> +     BT_WARNING("%s: No ongoing connection to disconnect", __FUNCTION__);

nit: remove extra 1-space indention.

::: dom/bluetooth/bluedroid/BluetoothMapSmsManager.h
@@ +120,5 @@
> +  nsRefPtr<BluetoothMapFolder> mRootFolder;
> +  /**
> +   * OBEX session status. Set when OBEX session is established.
> +   */
> +  bool mConnected;

Rename to |mMasConnected| and move declaration close to |mMnsConnected|.
(In reply to Ben Tian [:btian] from comment #20)
> Comment on attachment 8647953 [details] [diff] [review]
> bug1166645-mc.patch (v4)
> 
> Review of attachment 8647953 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/bluetooth/bluedroid/BluetoothMapFolder.cpp
> @@ +41,5 @@
> > +  return mParent;
> > +}
> > +
> > +void
> > +BluetoothMapFolder::GetFolderName(nsString& aFolderName)
> 
> Replace this function with |PrintFolderName()| to print out folder name
> directly.
> 
> ::: dom/bluetooth/bluedroid/BluetoothMapSmsManager.cpp
> @@ +432,5 @@
> > +    // Go up 1 level
> > +    BluetoothMapFolder* parent = mCurrentFolder->GetParentFolder();
> > +    if (!parent) {
> > +      BT_LOGR("MAS SetPath Go up 1 level");
> > +      mCurrentFolder = parent;
> 
> nit: assign first and then print log to conform with below.
> 
> @@ +447,5 @@
> > +      BT_LOGR("MAS SetPath Go back to root");
> > +    } else {
> > +      // Go down 1 level
> > +      BluetoothMapFolder* child = mCurrentFolder->GetSubFolder(childFolderName);
> > +
> 
> nit: remove this newline
> 
> @@ +449,5 @@
> > +      // Go down 1 level
> > +      BluetoothMapFolder* child = mCurrentFolder->GetSubFolder(childFolderName);
> > +
> > +      if (!child) {
> > +        return ObexResponseCode::NotFound;
> 
> Print log
> 
>   BT_LOGR("Illegal subfolder name [%s]",
>           NS_ConvertUTF16toUTF8(childFolderName));
> 
> @@ +667,5 @@
> > +                     folderListingSizeValue, sizeof(folderListingSizeValue));
> > +
> > +  uint8_t resp[255];
> > +  int index = 3;
> > +
> 
> nit: remove this newline.
> 
> @@ +677,5 @@
> > +   * If MaxListCount = 0, the MSE shall ignore all other applications
> > +   * parameters that may be presented in the request. The response shall
> > +   * contain any Body header.
> > +   */
> > +  if (maxListCount) {
> 
> The condition was |!maxListCount| in previous patch. Which one is correct?
The previous v3 patch is wrong. I reversed the logic and use |if (!maxListCount)|.
I did the full test and found v3 is with this bug.

> 
> @@ +740,5 @@
> > +   * during MAP session. Only one MNS connection per device pair.
> > +   * Section 6.4.2, MAP
> > +   * If the Message Access connection is disconnected after Message Notification
> > +   * connection establishment, this will automatically indicate a MAS
> > +   * Notification-Deregistration for this MAS instance.
> 
> Where do we do auto de-registration?
It looks like Android indeed close MNS connection after receiving MAS message notification-registration=off.
http://androidxref.com/5.1.1_r6/xref/packages/apps/Bluetooth/src/com/android/bluetooth/map/BluetoothMnsObexClient.java#194

> 
> @@ +821,5 @@
> > +    mMnsSocket = nullptr;
> > +    return;
> > +  }
> > +
> > +  mMasServerSocket = nullptr;
> 
> Add comment // MAS socket connection error

OK.

> 
> @@ +856,5 @@
> > +void
> > +BluetoothMapSmsManager::Disconnect(BluetoothProfileController* aController)
> > +{
> > +  if (!mMasSocket) {
> > +     BT_WARNING("%s: No ongoing connection to disconnect", __FUNCTION__);
> 
> nit: remove extra 1-space indention.
> 
> ::: dom/bluetooth/bluedroid/BluetoothMapSmsManager.h
> @@ +120,5 @@
> > +  nsRefPtr<BluetoothMapFolder> mRootFolder;
> > +  /**
> > +   * OBEX session status. Set when OBEX session is established.
> > +   */
> > +  bool mConnected;
> 
> Rename to |mMasConnected| and move declaration close to |mMnsConnected|.

Ok.
(In reply to Ben Tian [:btian] from comment #20)
> Comment on attachment 8647953 [details] [diff] [review]
> bug1166645-mc.patch (v4)
> 
> Review of attachment 8647953 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/bluetooth/bluedroid/BluetoothMapFolder.cpp
> @@ +41,5 @@
> > +  return mParent;
> > +}
> > +
> > +void
> > +BluetoothMapFolder::GetFolderName(nsString& aFolderName)
> 
> Replace this function with |PrintFolderName()| to print out folder name
> directly.
I prefer to keep this getter, but provide |Dump()| for debugging purpose.
(In reply to Ben Tian [:btian] from comment #20)
> Comment on attachment 8647953 [details] [diff] [review]
> bug1166645-mc.patch (v4)
> 
> Review of attachment 8647953 [details] [diff] [review]:
> -----------------------------------------------------------------
> @@ +740,5 @@
> > +   * during MAP session. Only one MNS connection per device pair.
> > +   * Section 6.4.2, MAP
> > +   * If the Message Access connection is disconnected after Message Notification
> > +   * connection establishment, this will automatically indicate a MAS
> > +   * Notification-Deregistration for this MAS instance.
> 
> Where do we do auto de-registration?

I consider to drop MNS connection after MAS connection dropped to ensure MAS ntf-registration=off was not sent via MCE. But the current logic remains the same. When we received "x-bt/MAP-NotificationRegistration" type, we connect/disconnect MNS connection. If MAS connection already dropped but MNS is still connected, we disconnect anyway in |AfterMapSmsDisconnected|.
Attachment #8647953 - Attachment is obsolete: true
Attachment #8647953 - Flags: review?(btian)
Comment on attachment 8648056 [details] [diff] [review]
bug1166645-mc.patch (v5)

Review of attachment 8648056 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with comment addressed.

::: dom/bluetooth/bluedroid/BluetoothMapFolder.cpp
@@ +18,5 @@
> +{
> +}
> +
> +void
> +BluetoothMapFolder::AddSubFolder(const nsAString& aFolderName)

Return the added |folder| so you don't have to call |GetSubFolder| again.

@@ +41,5 @@
> +  return mParent;
> +}
> +
> +void
> +BluetoothMapFolder::GetFolderName(nsString& aFolderName)

Remove this function since it's unused.

::: dom/bluetooth/bluedroid/BluetoothMapSmsManager.cpp
@@ +690,5 @@
> +
> +void
> +BluetoothMapSmsManager::BuildDefaultFolderStructure()
> +{
> +  // MAP specification defines virtual folders structure

List mandatory folders in comment as following.

  /**
   * MAP spec defined virtual folders structure:
   *   /
   *   /telecom
   *   /telecom/msg
   *   /telecom/msg/inbox
   *   /telecom/msg/draft
   *   /telecom/msg/outbox
   *   /telecom/msg/sent
   *   /telecom/msg/deleted
   */

@@ +694,5 @@
> +  // MAP specification defines virtual folders structure
> +  mRootFolder = new BluetoothMapFolder(NS_LITERAL_STRING("root"), nullptr);
> +  mRootFolder->AddSubFolder(NS_LITERAL_STRING("telecom"));
> +  nsRefPtr<BluetoothMapFolder> folder;
> +  folder = mRootFolder->GetSubFolder(NS_LITERAL_STRING("telecom"));

Integrate |GetSubFolder| into |GetSubFolder|.

::: dom/bluetooth/bluedroid/BluetoothMapSmsManager.h
@@ +93,5 @@
> +
> +  void HandleNotificationRegistration(const ObexHeaderSet& aHeader);
> +  void HandleEventReport(const ObexHeaderSet& aHeader);
> +  void HandleMessageStatus(const ObexHeaderSet& aHeader);
> +  void ReplyError(uint8_t aError);

nit: move this function right after |ReplyToPut| to group together.

@@ +111,5 @@
> +  /*
> +   * Build mandatory folders
> +   */
> +  void BuildDefaultFolderStructure();
> +  void HandleSmsMmsFolderListing(const ObexHeaderSet& aHeader);

nit: move this function right after |HandleMessageStatus| to group together.
Attachment #8648056 - Flags: review?(btian) → review+
Hi Jamin,
This patch depends on bug1180556 Obex append Appparameters.

int AppendHeaderAppParameters(uint8_t* aRetBuf, int aBufferSize,               
                              const uint8_t* aAppParameters, int aLength);     
int AppendAppParameter(uint8_t* aRetBuf, int aBufferSize, const uint8_t aTagId,
                       const uint8_t* aValue, int aLength);

Can you separate OBEX change from bug1180556?
Flags: needinfo?(jaliu)
Clear the ni flag from #comment 28 since Bug 1180554 has been resolved.
Flags: needinfo?(6jamin)
(In reply to Shawn Huang [:shawnjohnjr] from comment #37)
> https://treeherder.mozilla.org/#/jobs?repo=b2g-inbound&revision=3545d8b3257c

All testfailed items are not related to this patch.
Due to the change for nsBaseHashtable, I need to rebase v2.2r and give up Iterator. :(
(In reply to Shawn Huang [:shawnjohnjr] from comment #41)
> Due to the change for nsBaseHashtable, I need to rebase v2.2r and give up
> Iterator. :(

See Bug 1181445.
https://hg.mozilla.org/mozilla-central/rev/3545d8b3257c
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S7 (18Sep)
After rebase to v2.2r, now I can see MAP socket listening failure.
It said rfc_channel: 1 already in use. I have no idea why it fails. ni myself to dig into the error before I uplift to v2.2r branch. This is sad.

09-10 03:39:40.040 D/bt-btif ( 1601): btsock_rfc_listen, service_name:SMS/MMS Message Access
09-10 03:39:40.040 D/bt-btif ( 1601): BTA_JvCreateRecordByUser:SMS/MMS Message Access
09-10 03:39:40.040 I/bt-btif ( 1601): BTA_JvCreateRecordByUser                  
09-10 03:39:40.040 D/bt-btif ( 1601): adding fd:32, flags:0x4                   
09-10 03:39:40.040 D/bt-btif ( 1601): cmd.id:3                                  
09-10 03:39:40.040 I/bt-btif ( 1601): BTA got event 0x1a0e                      
09-10 03:39:40.040 D/bt-btif ( 1601): jv_dm_cback: event:11, slot id:3          
09-10 03:39:40.040 E/bt-btif ( 1601): rfc channel:1 already in use              
09-10 03:39:40.040 E/bt-btif ( 1601): jv_dm_cback: cannot start server, slot found:0xb6e1a4f4
09-10 03:39:40.040 D/bt-btif ( 1601): cleanup slot:3, fd:32, scn:1, sdp_handle:0x0
09-10 03:39:40.040 D/bt-btm  ( 1601): BTM_FreeSCN                               
09-10 03:39:40.040 D/bt-btif ( 1601): print poll event:20                       
09-10 03:39:40.040 D/bt-btif ( 1601):    POLLNVAL                               
09-10 03:39:40.040 W/bt-btif ( 1601): invalid rfc slot id: 3
Flags: needinfo?(shuang)
Clear ni, i found rebase and mess up parameter.
Flags: needinfo?(shuang)
blocking-b2g: --- → 2.2r?
Flags: needinfo?(whuang)
Flags: needinfo?(shuang)
blocking-b2g: 2.2r? → 2.2r+
Flags: needinfo?(whuang)
(In reply to Shawn Huang [:shawnjohnjr] from comment #47)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=fa751b2f066a

I'm not sure why try server won't execute my request for v2.2r.
(In reply to Shawn Huang [:shawnjohnjr] from comment #48)
> (In reply to Shawn Huang [:shawnjohnjr] from comment #47)
> > https://treeherder.mozilla.org/#/jobs?repo=try&revision=fa751b2f066a
> 
> I'm not sure why try server won't execute my request for v2.2r.

Any idea or I can ask anyone? I still don't see any result.
Flags: needinfo?(cbook)
Flags: needinfo?(cbook)
Sadly, I don't know why my rebase v2.2r patch became to m-c version. I lost all my change locally for v2.2r. I need to rebase again :(
Hey Shawn, yeah this didn't applied to b2g-i , but will checkin after you rebased. Thanks!
Flags: needinfo?(shuang)
Keywords: checkin-needed
Keywords: checkin-needed
Whiteboard: needs-uplift
You need to log in before you can comment on or make changes to this bug.